Skip to contentSkip to conversation

damon/sysfs: Fix possible memleak on damon_sysfs_add_target.

PATCH Applied SeongJae Park 2 participants 3 messages Sep 26, 2022 – Sep 27, 2022

Subsystems (3)
Stable branch Memory management DAMON

v4 of 5 Applied 1/1 Latest is v5 →

lore
Share & tools
lore

SeongJae Park

author maintainer
#0 lore raw

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

From: Levi Yun <[email protected]>

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 a61ea561c871 ("mm/damon/sysfs: link DAMON for virtual address spaces monitoring")
Cc <[email protected]> # 5.17.x
Signed-off-by Levi Yun <[email protected]>
Reviewed-by SeongJae Park <[email protected]>
Signed-off-by SeongJae Park <[email protected]>

---

Changes from v3
(https://lore.kernel.org/damon/[email protected]/)
- Fix Fixes: tag
- Add patch changelog

Changes from v2
(https://lore.kernel.org/damon/[email protected]/)
- Add Fixes: and Cc: stable

Changes from v1
(https://lore.kernel.org/damon/[email protected]/)
- Do damon_add_target() earlier instead of explicitly freeing the object

 mm/damon/sysfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
mm/damon/sysfs.c +1 -1
index 455215a5c059..9f1219a67e3f 100644
@@ -2172,12 +2172,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.25.1
author maintainer
#1 lore raw

I forgot removing the closing dot of the subject and making the subject lower-case.

Quoted 8 lines · On Mon, 26 Sep 2022 16:06:11 +0000 SeongJae Park <[email protected]> wrote:

From: Levi Yun <[email protected]>

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

Also we should s/daemon/damon/

I will revise and send v5.

Quoted 45 lines

possible memory leak.

Fixes a61ea561c871 ("mm/damon/sysfs: link DAMON for virtual address spaces monitoring")
Cc <[email protected]> # 5.17.x
Signed-off-by Levi Yun <[email protected]>
Reviewed-by SeongJae Park <[email protected]>
Signed-off-by SeongJae Park <[email protected]>

---

Changes from v3
(https://lore.kernel.org/damon/[email protected]/)
- Fix Fixes: tag
- Add patch changelog

Changes from v2
(https://lore.kernel.org/damon/[email protected]/)
- Add Fixes: and Cc: stable

Changes from v1
(https://lore.kernel.org/damon/[email protected]/)
- Do damon_add_target() earlier instead of explicitly freeing the object

 mm/damon/sysfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
mm/damon/sysfs.c +1 -1
index 455215a5c059..9f1219a67e3f 100644
@@ -2172,12 +2172,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.25.1
#2 lore raw

Thanks :)

And Sorry to my mistake.

Quoted 64 lines · SeongJae Park

I forgot removing the closing dot of the subject and making the subject lower-case.

Quoted 8 lines · On Mon, 26 Sep 2022 16:06:11 +0000 SeongJae Park <[email protected]> wrote:

From: Levi Yun <[email protected]>

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

Also we should s/daemon/damon/

I will revise and send v5.

Quoted 45 lines

possible memory leak.

Fixes a61ea561c871 ("mm/damon/sysfs: link DAMON for virtual address spaces monitoring")
Cc <[email protected]> # 5.17.x
Signed-off-by Levi Yun <[email protected]>
Reviewed-by SeongJae Park <[email protected]>
Signed-off-by SeongJae Park <[email protected]>

---

Changes from v3
(https://lore.kernel.org/damon/[email protected]/)
- Fix Fixes: tag
- Add patch changelog

Changes from v2
(https://lore.kernel.org/damon/[email protected]/)
- Add Fixes: and Cc: stable

Changes from v1
(https://lore.kernel.org/damon/[email protected]/)
- Do damon_add_target() earlier instead of explicitly freeing the object

 mm/damon/sysfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
mm/damon/sysfs.c +1 -2
index 455215a5c059..9f1219a67e3f 100644
@@ -2172,12 +2172,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;
--

2.25.1