Skip to contentSkip to conversation

mm/damon/sysfs: fix unhandled return values

PATCH Applied SeongJae Park 1 participant 5 messages Nov 6, 2023

Subsystems (3)
lore
Share & tools
lore

SeongJae Park

author maintainer
#0 lore raw

cover mm/damon/sysfs: fix unhandled return values

Some of DAMON sysfs interface code is not handling return values from some functions. As a result, confusing user input handling or NULL-dereference is possible. Check those properly.

Please note that these patches are not cleanly applicable on mm-unstable since mm-unstable has dropped the mainline-merged patches and rebased on v6.6, while some DAMON patches that these patches are depend on are merged in the mainline after v6.6. I confirmed these patches can cleanly applied on latest mainline, or mm-stable-2023-11-01-14-33.

SeongJae Park (3):
mm/damon/sysfs: check error from damon_sysfs_update_target() mm/damon/sysfs-schemes: handle tried regions sysfs directory
allocation failure
mm/damon/sysfs-schemes: handle tried region directory allocation
failure

 mm/damon/sysfs-schemes.c | 5 +++++
 mm/damon/sysfs.c         | 4 +++-
 2 files changed, 8 insertions(+), 1 deletion(-)

base-commit: 3496e8e0a1eeabb738105c09e575495fa78914bb

Signature · 1 line
2.34.1
author maintainer
#1 lore raw

PATCH 1/3 mm/damon/sysfs: check error from damon_sysfs_update_target()

damon_sysfs_update_target() returns error code for failures, but its caller, damon_sysfs_set_targets() is ignoring that. The update function seems making no critical change in case of such failures, but the behavior will look like DAMON sysfs is silently ignoring or only partially accepting the user input. Fix it.

Fixes 19467a950b49 ("mm/damon/sysfs: remove requested targets when online-commit inputs")
Cc <[email protected]> # 5.19.x

---

Note that yet another fix[1] should be applied before this.

[1] https://lore.kernel.org/all/[email protected]/

 mm/damon/sysfs.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
mm/damon/sysfs.c +3 -1
index 1dfa96d4de99..7472404456aa 100644
@@ -1203,8 +1203,10 @@ static int damon_sysfs_set_targets(struct damon_ctx *ctx,
 
 	damon_for_each_target_safe(t, next, ctx) {
 		if (i < sysfs_targets->nr) {
-			damon_sysfs_update_target(t, ctx,
+			err = damon_sysfs_update_target(t, ctx,
 					sysfs_targets->targets_arr[i]);
+			if (err)
+				return err;
 		} else {
 			if (damon_target_has_pid(ctx))
 				put_pid(t->pid);
Signature · 1 line
2.34.1
author maintainer
#2 lore raw

PATCH 2/3 mm/damon/sysfs-schemes: handle tried regions sysfs directory allocation failure

In reply to SeongJae Park

DAMOS tried regions sysfs directory allocation function (damon_sysfs_scheme_regions_alloc()) is not handling the memory allocation failure. In the case, the code will dereference NULL pointer. Handle the failure to avoid such invalid access.

Fixes 9277d0367ba1 ("mm/damon/sysfs-schemes: implement scheme region directory")
Cc <[email protected]> # 6.2.x
---
 mm/damon/sysfs-schemes.c | 3 +++
 1 file changed, 3 insertions(+)
mm/damon/sysfs-schemes.c +3 -0
index 45bd0fd4a8b1..7413cb35c5a9 100644
@@ -162,6 +162,9 @@ damon_sysfs_scheme_regions_alloc(void)
 	struct damon_sysfs_scheme_regions *regions = kmalloc(sizeof(*regions),
 			GFP_KERNEL);
 
+	if (!regions)
+		return NULL;
+
 	regions->kobj = (struct kobject){};
 	INIT_LIST_HEAD(&regions->regions_list);
 	regions->nr_regions = 0;
Signature · 1 line
2.34.1
author maintainer
#3 lore raw

PATCH 3/3 mm/damon/sysfs-schemes: handle tried region directory allocation failure

In reply to SeongJae Park

DAMON sysfs interface's before_damos_apply callback (damon_sysfs_before_damos_apply()), which creates the DAMOS tried regions for each DAMOS action applied region, is not handling the allocation failure for the sysfs directory data. As a result, NULL pointer derefeence is possible. Fix it by handling the case.

Fixes f1d13cacabe1 ("mm/damon/sysfs: implement DAMOS tried regions update command")
Cc <[email protected]> # 6.2.x
---
 mm/damon/sysfs-schemes.c | 2 ++
 1 file changed, 2 insertions(+)
mm/damon/sysfs-schemes.c +2 -0
index 7413cb35c5a9..be667236b8e6 100644
@@ -1826,6 +1826,8 @@ static int damon_sysfs_before_damos_apply(struct damon_ctx *ctx,
 		return 0;
 
 	region = damon_sysfs_scheme_region_alloc(r);
+	if (!region)
+		return 0;
 	list_add_tail(&region->list, &sysfs_regions->regions_list);
 	sysfs_regions->nr_regions++;
 	if (kobject_init_and_add(&region->kobj,
Signature · 1 line
2.34.1
author maintainer
#4 lore raw
In reply to SeongJae Park
Quoted 9 lines · On Mon, 6 Nov 2023 23:34:05 +0000 SeongJae Park <[email protected]> wrote:

Some of DAMON sysfs interface code is not handling return values from some functions. As a result, confusing user input handling or NULL-dereference is possible. Check those properly.

Please note that these patches are not cleanly applicable on mm-unstable since mm-unstable has dropped the mainline-merged patches and rebased on v6.6, while some DAMON patches that these patches are depend on are merged in the mainline after v6.6. I confirmed these patches can cleanly applied on latest mainline, or mm-stable-2023-11-01-14-33.

I just checked mm-unstable has just updated, and confirmed these patches can cleanly applied.

Thanks,
SJ

[...]