Skip to contentSkip to conversation

damon/sysfs: Fix possible memleak on damon_sysfs_add_target.

PATCH In review Levi Yun 2 participants 2 messages Sep 25, 2022 – Sep 26, 2022

Subsystems (3)
lore
Share & tools
lore

Levi Yun

author
#0 lore raw

PATCH v3 1/1 damon/sysfs: Fix possible memleak on damon_sysfs_add_target.

When damon_sysfs_add_target couldn't find proper task, New allocated damon_target structure isn't registered yet, So, it's impossible to free new allocated one by damon_sysfs_destroy_targets.

By calling daemon_add_target as soon as allocating new target, Fix this possible memory leak.

Fixes 74bd8b7d2f8e7
---
 mm/damon/sysfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
mm/damon/sysfs.c +1 -1
index 7488e27c87c3..bdef9682d0a0 100644
@@ -2182,12 +2182,12 @@ static int damon_sysfs_add_target(struct damon_sysfs_target *sys_target,
 
 	if (!t)
 		return -ENOMEM;
+	damon_add_target(ctx, t);
 	if (damon_target_has_pid(ctx)) {
 		t->pid = find_get_pid(sys_target->pid);
 		if (!t->pid)
 			goto destroy_targets_out;
 	}
-	damon_add_target(ctx, t);
 	err = damon_sysfs_set_regions(t, sys_target->regions);
 	if (err)
 		goto destroy_targets_out;
Signature · 1 line
2.35.1
maintainer Reviewed-by on v3 1/1
#1 lore raw

Hi Levi,

Quoted 10 lines · On Mon, 26 Sep 2022 08:43:27 +0900 Levi Yun <[email protected]> wrote:

When damon_sysfs_add_target couldn't find proper task, New allocated damon_target structure isn't registered yet, So, it's impossible to free new allocated one by damon_sysfs_destroy_targets.

By calling daemon_add_target as soon as allocating new target, Fix this possible memory leak.

Fixes 74bd8b7d2f8e7

The commit is indeed the last one which touched the code, but the bug was introduced earlier than that, by commit a61ea561c871 ("mm/damon/sysfs: link DAMON for virtual address spaces monitoring").

Also, let's add one space before the commit hash, then only 12 characters for the hash, and then one line summary of the commit[1], like below.

Fixes a61ea561c871 ("mm/damon/sysfs: link DAMON for virtual address spaces monitoring")

[1] https://www.kernel.org/doc/html/v4.15/process/submitting-patches.html#describe-your-changes

Quoted 2 lines

---

Also, putting the change log of the patch here would be a good practice.

Other than the very minor things,

As the changes I'm requesting are very minor, I will revise and post a new version of this patch on my own.

Thanks,
SJ

Quoted 24 lines
 mm/damon/sysfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
mm/damon/sysfs.c +1 -1
index 7488e27c87c3..bdef9682d0a0 100644
@@ -2182,12 +2182,12 @@ static int damon_sysfs_add_target(struct damon_sysfs_target *sys_target,
 
 	if (!t)
 		return -ENOMEM;
+	damon_add_target(ctx, t);
 	if (damon_target_has_pid(ctx)) {
 		t->pid = find_get_pid(sys_target->pid);
 		if (!t->pid)
 			goto destroy_targets_out;
 	}
-	damon_add_target(ctx, t);
 	err = damon_sysfs_set_regions(t, sys_target->regions);
 	if (err)
 		goto destroy_targets_out;
Signature · 1 line
2.35.1