mm/damon/stat: deallocate damon_call() failure leaking damon_ctx

RFC PATCH Superseded SeongJae Park 1 participant 3 messages Apr 2

Subsystems (3)

Stable branch Memory management DAMON

lore

Share & tools

lore

SeongJae Park

author maintainer

2026-04-02 04:59 UTC #0 lore raw

RFC PATCH 1/1 mm/damon/stat: deallocate damon_call() failure leaking damon_ctx

damon_stat_start() always allocates the module's damon_ctx object (damon_stat_context). Meanwhile, if damon_call() in the function fails, the damon_ctx object is not deallocated. Hence, if the damon_call() is failed, and the user writes Y to “enabled” again, the previously allocated damon_ctx object is leaked.

This cannot simply be fixed by deallocating the damon_ctx object when damon_call() fails. That's because damon_call() failure doesn't guarantee the kdamond main function, which accesses the damon_ctx object, is completely finished. In other words, if damon_stat_start() deallocates the damon_ctx object after damon_call() failure, the not-yet-terminated kdamond could access the freed memory (use-after-free).

Fix the leak while avoiding the use-after-free by keeping returning damon_stat_start() without deallocating the damon_ctx object after damon_call() failure, but deallocating it when the function is invoked again and the kdamond is completely terminated. If the kdamond is not yet terminated, simply return -EAGAIN, as the kdamond will soon be terminated.

The issue was discovered [1] by sashiko.

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

Fixes 405f61996d9d ("mm/damon/stat: use damon_call() repeat mode instead of damon_callback")

Cc <[email protected]> # 6.17.x

Signed-off-by SeongJae Park <[email protected]>

---
Changes from v1
(https://lore.kernel.org/[email protected])
- Avoid use-after-free.
- Add RFC tag.

 mm/damon/stat.c | 7 +++++++
 1 file changed, 7 insertions(+)

mm/damon/stat.c +7 -0

index 5a742fc157e4..99ba346f9e32 100644
@@ -245,6 +245,12 @@ static int damon_stat_start(void)
 {
 	int err;
 
+	if (damon_stat_context) {
+		if (damon_is_running(damon_stat_context))
+			return -EAGAIN;
+		damon_destroy_ctx(damon_stat_context);
+	}
+
 	damon_stat_context = damon_stat_build_ctx();
 	if (!damon_stat_context)
 		return -ENOMEM;

@@ -264,6 +270,7 @@ static void damon_stat_stop(void)
 {
 	damon_stop(&damon_stat_context, 1);
 	damon_destroy_ctx(damon_stat_context);
+	damon_stat_context = NULL;
 }
 
 static int damon_stat_enabled_store(

base-commit: 4fd04f750d79667937931314ed64c9d79b0d82ef

Signature · 1 line

2.47.3

SeongJae Park

author maintainer

2026-04-02 13:39 UTC #1 lore raw

Re: (sashiko review) [RFC PATCH] mm/damon/stat: deallocate damon_call() failure leaking damon_ctx

TL; DR: sashiko didn't find a real issue here. I will post non-RFC patch soon.

Forwarding Sashiko review in a usual reply format, with my inline comment for doing discussions via mails.

Quoted 20 lines

# review url: https://sashiko.dev/#/patchset/[email protected]
# start of sashiko.dev inline review
commit 8adb3c95d68d8448c8f57865d9874a5277cc1540
Author: SeongJae Park <[email protected]>
mm/damon/stat: deallocate damon_call() failure leaking damon_ctx
This commit attempts to fix a memory leak of the damon_ctx object when damon_call() fails, by deferring deallocation to a subsequent start call to avoid a use-after-free with the terminating kdamond thread.
Link https://lore.kernel.org/[email protected] open here
Quoted 7 lines
mm/damon/stat.c +0 -0
@@ -245,6 +245,12 @@ static int damon_stat_start(void)
 {
 	int err;
 

Quoted 7 lines

mm/damon/stat.c static int damon_stat_start(void) · line 250
Quoted 3 lines
mm/damon/stat.c +3 -0
+	if (damon_stat_context) {
+		if (damon_is_running(damon_stat_context))
+			return -EAGAIN;
If the user retries while the background thread is still terminating, this will return -EAGAIN. Does this break standard sysfs boolean expectations, forcing userspace scripts to implement arbitrary retry loops?

I don't think that's a problem.

Quoted 3 lines

Also, if the user never writes "Y" to the "enabled" sysfs parameter again, does the memory remain permanently leaked?

In a sense someone can argue so. Someone else may also argue it as "cached". Anyway the total amount of "leaked" or "cached" memory is just single damon_ctx object. No problem in my opinion.

Quoted 5 lines

Could this be fixed synchronously by calling damon_stat_stop() in the error path of damon_stat_start() instead? Since damon_stop() safely blocks via kthread_stop_put() until the thread has fully exited, it seems it would allow immediate and safe deallocation without returning -EAGAIN.

damon_stop() was not designed for the purpose. Calling damon_stop() for terminating kdamond also feels odd. For a hotfix, I think the current patch is good.

Quoted 9 lines

mm/damon/stat.c static int damon_stat_start(void) · line 254
Quoted 5 lines
mm/damon/stat.c +3 -0
+		damon_destroy_ctx(damon_stat_context);
+	}
+
 	damon_stat_context = damon_stat_build_ctx();
# end of sashiko.dev inline review
# review url: https://sashiko.dev/#/patchset/[email protected]

Thanks,
SJ

[...]

SeongJae Park

author maintainer

2026-04-02 15:27 UTC #2 lore raw

Re: (sashiko review) [RFC PATCH] mm/damon/stat: deallocate damon_call() failure leaking damon_ctx

Quoted 1 line · On Thu, 2 Apr 2026 06:39:51 -0700 SeongJae Park <[email protected]> wrote:

TL; DR: sashiko didn't find a real issue here. I will post non-RFC patch soon.

Posted it: https://lore.kernel.org/[email protected]

[...]

Quoted 35 lines

Forwarding Sashiko review in a usual reply format, with my inline comment for doing discussions via mails.
Quoted 20 lines
# review url: https://sashiko.dev/#/patchset/[email protected]
# start of sashiko.dev inline review
commit 8adb3c95d68d8448c8f57865d9874a5277cc1540
Author: SeongJae Park <[email protected]>
mm/damon/stat: deallocate damon_call() failure leaking damon_ctx
This commit attempts to fix a memory leak of the damon_ctx object when damon_call() fails, by deferring deallocation to a subsequent start call to avoid a use-after-free with the terminating kdamond thread.
Link https://lore.kernel.org/[email protected] open here
Quoted 7 lines
mm/damon/stat.c +0 -0
@@ -245,6 +245,12 @@ static int damon_stat_start(void)
 {
 	int err;
 
Quoted 7 lines
mm/damon/stat.c static int damon_stat_start(void) · line 250
Quoted 3 lines
mm/damon/stat.c +3 -0
+	if (damon_stat_context) {
+		if (damon_is_running(damon_stat_context))
+			return -EAGAIN;
If the user retries while the background thread is still terminating, this will return -EAGAIN. Does this break standard sysfs boolean expectations, forcing userspace scripts to implement arbitrary retry loops?
I don't think that's a problem.
Quoted 1 line

Quoted 6 lines

Quoted 2 lines
Also, if the user never writes "Y" to the "enabled" sysfs parameter again, does the memory remain permanently leaked?
In a sense someone can argue so. Someone else may also argue it as "cached". Anyway the total amount of "leaked" or "cached" memory is just single damon_ctx object. No problem in my opinion.

Because the reference is still there, I think it cannot technically be called "leaked".

Thanks,
SJ

[...]