* [PATCH] damon/sysfs: Fix possible memleak on damon_sysfs_add_target.
@ 2022-09-25 14:02 Levi Yun
2022-09-25 17:40 ` SeongJae Park
0 siblings, 1 reply; 2+ messages in thread
From: Levi Yun @ 2022-09-25 14:02 UTC (permalink / raw)
To: sj; +Cc: akpm, damon, linux-mm, Levi Yun
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 additional damon_free_target when find_get_pid function,
Fix possible memory leak.
Signed-off-by: Levi Yun <ppbuk5246@gmail.com>
---
mm/damon/sysfs.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/mm/damon/sysfs.c b/mm/damon/sysfs.c
index 7488e27c87c3..28ed07d26d55 100644
--- a/mm/damon/sysfs.c
+++ b/mm/damon/sysfs.c
@@ -2184,8 +2184,11 @@ static int damon_sysfs_add_target(struct damon_sysfs_target *sys_target,
return -ENOMEM;
if (damon_target_has_pid(ctx)) {
t->pid = find_get_pid(sys_target->pid);
- if (!t->pid)
+ if (!t->pid) {
+ damon_free_target(t);
+
goto destroy_targets_out;
+ }
}
damon_add_target(ctx, t);
err = damon_sysfs_set_regions(t, sys_target->regions);
--
2.35.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] damon/sysfs: Fix possible memleak on damon_sysfs_add_target.
2022-09-25 14:02 [PATCH] damon/sysfs: Fix possible memleak on damon_sysfs_add_target Levi Yun
@ 2022-09-25 17:40 ` SeongJae Park
0 siblings, 0 replies; 2+ messages in thread
From: SeongJae Park @ 2022-09-25 17:40 UTC (permalink / raw)
To: Levi Yun; +Cc: sj, akpm, damon, linux-mm
On Sun, 25 Sep 2022 23:02:57 +0900 Levi Yun <ppbuk5246@gmail.com> 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.
Good finding, thanks!
>
> By calling additional damon_free_target when find_get_pid function,
> Fix possible memory leak.
>
> Signed-off-by: Levi Yun <ppbuk5246@gmail.com>
Could we add relevant 'Fixes: ' and 'Cc: <stable@vger.kernel.org>' tags?
> ---
> mm/damon/sysfs.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/mm/damon/sysfs.c b/mm/damon/sysfs.c
> index 7488e27c87c3..28ed07d26d55 100644
> --- a/mm/damon/sysfs.c
> +++ b/mm/damon/sysfs.c
> @@ -2184,8 +2184,11 @@ static int damon_sysfs_add_target(struct damon_sysfs_target *sys_target,
> return -ENOMEM;
> if (damon_target_has_pid(ctx)) {
> t->pid = find_get_pid(sys_target->pid);
> - if (!t->pid)
> + if (!t->pid) {
> + damon_free_target(t);
> +
Seems unnecessary new line?
> goto destroy_targets_out;
> + }
> }
Looks good to me, but... How about simply doing 'damon_add_target()' before
'if (damon_target_has_pid())', like below?
```
diff --git a/mm/damon/sysfs.c b/mm/damon/sysfs.c
index 455215a5c059..9f1219a67e3f 100644
--- a/mm/damon/sysfs.c
+++ b/mm/damon/sysfs.c
@@ -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;
```
Thanks,
SJ
> damon_add_target(ctx, t);
> err = damon_sysfs_set_regions(t, sys_target->regions);
> --
> 2.35.1
>
>
^ permalink raw reply related [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-09-25 17:40 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-25 14:02 [PATCH] damon/sysfs: Fix possible memleak on damon_sysfs_add_target Levi Yun
2022-09-25 17:40 ` SeongJae Park
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.