Re: [PATCH v2 0/2] mm/damon: reset thread status parameters upon kdamond termination

SeongJae Park
2026-04-14T00:29:05Z

On Tue, 14 Apr 2026 06:05:11 +0800 Liew Rui Yan <aethernet65535@gmail.com> wrote:

> Hi SeongJae,
>
> I've reviewed the Sashiko report on [PATCH v2 1/2] and [2/2]. Since the
> issues are essentially the same, I want to reply to them all in one
> email.
[...]
> # PATCH v2 2/2
> > > diff --git a/mm/damon/reclaim.c b/mm/damon/reclaim.c
> > > index 86da147786583..e3e148fd80f97 100644
> > > --- a/mm/damon/reclaim.c
> > > +++ b/mm/damon/reclaim.c
> > [ ... ]
> > > @@ -250,6 +250,10 @@ static int damon_reclaim_apply_parameters(void)
> > > if (err)
> > > goto out;
> > > err = damon_commit_ctx(ctx, param_ctx);
> > > + if (err) {
> > > + enabled = false;
> > > + kdamond_pid = -1;
> > > + }
> > > out:
> > > damon_destroy_ctx(param_ctx);
> > > return err;
> >
> > Can prematurely resetting enabled to false here introduce a race condition
> > leading to a use-after-free of the DAMON context structures?
> >
> > If damon_reclaim_apply_parameters() is invoked from the kdamond worker thread
> > (for example, when applying commit_inputs) and damon_commit_ctx() fails,
> > ctx->maybe_corrupted is set to true. This signals the kdamond thread to
> > break its main loop and begin its teardown phase, such as executing
> > damon_destroy_targets() to free lists.
> >
> > Because enabled is set to false asynchronously here, a concurrent sysfs write
> > of 'Y' to enabled will succeed and immediately trigger
> > damon_reclaim_turn(true). This unconditionally calls
> > damon_reclaim_apply_parameters() and executes damon_commit_ctx() from the
> > sysfs thread.
> >
> > Since damon_commit_ctx() locklessly mutates and frees items in
> > ctx->adaptive_targets and ctx->schemes, would this race directly with the
> > still-exiting kdamond thread traversing and freeing those exact same lists,
> > resulting in list corruption and a use-after-free?
>
> The core issue is - modifying 'enabled' and 'kdamond_pid' in the error
> path of damon_commit_ctx() is racy.

We simply made such racy user behaviors be prohibited [1]. So this should be
fine. But, I'd prefer simpler fix, as I replied to the cover letter.

>
> My plan for v3:
> - Remove the reset code in damon_*_apply_parameters()
> - Keep only the fix in damon_*_turn(false)
>
> This resolves the restart issue without introducing new races.
> Please let me know if this direction looks good.
>
> Small changes for v3:
> - Delete a "=" at the bottom of "Problem" (commit message):
>
> Problem
> - ========
> + =======
>

I also added comments about above as a reply to the cover letter. Please reply
there.

[1] https://lkml.kernel.org/r/20260329153052.46657-2-sj@kernel.org

Thanks,
SJ …