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@xxxxxxxxxx> 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@xxxxxxxxxx


Thanks,
SJ

[...]