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

SeongJae Park
2026-04-02T05:06:52Z

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 …