mm/damon/stat: deallocate damon_call() failure leaking damon_ctx
Subsystems (3)
SeongJae Park
maintainerPATCH 1/1 mm/damon/stat: deallocate damon_call() failure leaking damon_ctx
DAMON_STAT does not deallocate its dynamically allocated damon_ctx object when damon_call() is failed. As a result, the memory is leaked. Check the failure and deallocate the damon_ctx object.
The issue was discovered [1] by sashiko.
[1] https://lore.kernel.org/[email protected]
--- mm/damon/stat.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
mm/damon/stat.c +6 -1
@@ -257,7 +257,12 @@ static int damon_stat_start(void) damon_stat_last_refresh_jiffies = jiffies; call_control.data = damon_stat_context; -return damon_call (damon_stat_context, &call_control); + err =damon_call (damon_stat_context, &call_control); +if (err) { +damon_destroy_ctx (damon_stat_context); + damon_stat_context =NULL ; + } +return err; }static void damon_stat_stop (void )
base-commit: 4fd04f750d79667937931314ed64c9d79b0d82ef
Signature · 1 line
2.47.3
SeongJae Park
maintainerRe: (sashiko review) [PATCH] mm/damon/stat: deallocate damon_call() failure leaking damon_ctx
Andrew, please drop this patch. sashiko found an issue, so I will respin.
More details below.
Forwarding Sashiko review in the usual reply format with my inline comments, for doing discussions via mails.
Quoted 26 lines
# review url: https://sashiko.dev/#/patchset/[email protected] # start of sashiko.dev inline review commit 1d53bb9291d3fde393f37b143e637f0d25b6634e Author: SeongJae Park <[email protected]>
mm/damon/stat: deallocatedamon_call()failure leakingdamon_ctxThis commit addresses a memory leak by deallocating
damon_stat_contextifdamon_call()fails during stat startup.mm/damon/stat.c static int damon_stat_start(void) · line 262
Quoted 11 lines
mm/damon/stat.c +3
-1@@ -257,7 +257,12 @@ static int damon_stat_start(void) damon_stat_last_refresh_jiffies = jiffies; call_control.data = damon_stat_context; -return damon_call (damon_stat_context, &call_control); + err =damon_call (damon_stat_context, &call_control); +if (err) { +damon_destroy_ctx (damon_stat_context);Can this cause a use-after-free?
Quoted 8 lines
Earlier in
damon_stat_start(),damon_start()is called, which creates and starts thekdamond_fnkernel thread. This thread actively uses thedamon_stat_context.If
damon_call()fails, thekdamond_fnthread might still be running or in its teardown phase. If we free the context directly usingdamon_destroy_ctx()before the kthread has fully exited, the kthread might access freed memory.
Nice catch.
FYI, I initially thought damon_call() of DAMON_STAT cannot fail, because it synchronizes its damon_start()/damon_stop() calls with module parameter handling function, and it doesn't update the context internal state, which means the damon_ctx->maybe_corrupted cannot be set. If that's true, this patch itself is not needed since the memory leak cannot exist.
But, kdamond can fail for its internal memory allocation failures. Specifically, if ctx->region_score_histogram allocation is failed, it will be terminated. So, yes, sashiko is right. There is a chance.
Quoted 4 lines
Should we call
damon_stop()here to wait for the thread to safely exit before destroying the context, similar to the teardown sequence indamon_stat_stop()?
Seems that is a workable option. But given the fact that kdamond is already in its termination step, it feels odd to me. I'll take more time to think about.
Quoted 9 lines
mm/damon/stat.c static int damon_stat_start(void) · line 266
Quoted 5 lines
mm/damon/stat.c +3
-0+ damon_stat_context =NULL ; + } +return err; }# end of sashiko.dev inline review # review url: https://sashiko.dev/#/patchset/[email protected]
Thanks,
SJ
[...]
SeongJae Park
maintainerRe: (sashiko review) [PATCH] mm/damon/stat: deallocate damon_call() failure leaking damon_ctx
On Wed, 1 Apr 2026 19:07:44 -0700 SeongJae Park <[email protected]> wrote:
[...]
Quoted 35 lines
Quoted 13 lines
mm/damon/stat.c static int damon_stat_start(void) · line 262
Quoted 10 lines
mm/damon/stat.c +3
-1@@ -257,7 +257,12 @@ static int damon_stat_start(void) damon_stat_last_refresh_jiffies = jiffies; call_control.data = damon_stat_context; -return damon_call (damon_stat_context, &call_control); + err =damon_call (damon_stat_context, &call_control); +if (err) { +damon_destroy_ctx (damon_stat_context);Can this cause a use-after-free?
Quoted 8 lines
Earlier in
damon_stat_start(),damon_start()is called, which creates and starts thekdamond_fnkernel thread. This thread actively uses thedamon_stat_context.If
damon_call()fails, thekdamond_fnthread might still be running or in its teardown phase. If we free the context directly usingdamon_destroy_ctx()before the kthread has fully exited, the kthread might access freed memory.Nice catch.
FYI, I initially thought
damon_call()ofDAMON_STATcannot fail, because it synchronizes its damon_start()/damon_stop() calls with module parameter handling function, and it doesn't update the context internal state, which means thedamon_ctx->maybe_corruptedcannot be set. If that's true, this patch itself is not needed since the memory leak cannot exist.But, kdamond can fail for its internal memory allocation failures. Specifically, if
ctx->region_score_histogramallocation is failed, it will be terminated. So, yes, sashiko is right. There is a chance.Quoted 1 line
Quoted 6 lines
Quoted 3 lines
Should we call
damon_stop()here to wait for the thread to safely exit before destroying the context, similar to the teardown sequence indamon_stat_stop()?Seems that is a workable option. But given the fact that kdamond is already in its termination step, it feels odd to me. I'll take more time to think about.
I just posted another approach [1] that can avoid the use-after-free, with RFC tag for getting sashiko review before being merged.
[1] https://lore.kernel.org/[email protected]
Thanks,
SJ
[...]