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

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

[SeongJae Park]

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/20260401012428.86694-1-sj@kernel.org

Fixes: 405f61996d9d ("mm/damon/stat: use damon_call() repeat mode instead of damon_callback")
Cc: <stable@vger.kernel.org> # 6.17.x
Signed-off-by: SeongJae Park <sj@kernel.org>
---
 mm/damon/stat.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/mm/damon/stat.c b/mm/damon/stat.c
index 5a742fc157e4..30aeb2c207ec 100644
--- a/mm/damon/stat.c
+++ b/mm/damon/stat.c
@@ -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
-- 
2.47.3

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

[SeongJae Park]

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.

> # review url: https://sashiko.dev/#/patchset/20260402010457.66860-1-sj@kernel.org
> # start of sashiko.dev inline review
> commit 1d53bb9291d3fde393f37b143e637f0d25b6634e
> Author: SeongJae Park <sj@kernel.org>
> 
> mm/damon/stat: deallocate damon_call() failure leaking damon_ctx
> 
> This commit addresses a memory leak by deallocating damon_stat_context if
> damon_call() fails during stat startup.
> 
> Link: https://lore.kernel.org/20260401012428.86694-1-sj@kernel.org
> 
> > diff --git a/mm/damon/stat.c b/mm/damon/stat.c
> > --- a/mm/damon/stat.c
> > +++ b/mm/damon/stat.c
> > @@ -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?
> 
> Earlier in damon_stat_start(), damon_start() is called, which creates
> and starts the kdamond_fn kernel thread. This thread actively uses the
> damon_stat_context.
> 
> If damon_call() fails, the kdamond_fn thread might still be running or
> in its teardown phase. If we free the context directly using
> damon_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.

> 
> Should we call damon_stop() here to wait for the thread to safely exit
> before destroying the context, similar to the teardown sequence in
> damon_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.

> 
> > +		damon_stat_context = NULL;
> > +	}
> > +	return err;
> >  }
> 
> 
> # end of sashiko.dev inline review
> # review url: https://sashiko.dev/#/patchset/20260402010457.66860-1-sj@kernel.org


Thanks,
SJ

[...]

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

[SeongJae Park]

On Wed,  1 Apr 2026 19:07:44 -0700 SeongJae Park <sj@kernel.org> wrote:
[...]
> > > --- a/mm/damon/stat.c
> > > +++ b/mm/damon/stat.c
> > > @@ -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?
> > 
> > Earlier in damon_stat_start(), damon_start() is called, which creates
> > and starts the kdamond_fn kernel thread. This thread actively uses the
> > damon_stat_context.
> > 
> > If damon_call() fails, the kdamond_fn thread might still be running or
> > in its teardown phase. If we free the context directly using
> > damon_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.
> 
> > 
> > Should we call damon_stop() here to wait for the thread to safely exit
> > before destroying the context, similar to the teardown sequence in
> > damon_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/20260402045928.71170-1-sj@kernel.org


Thanks,
SJ

[...]