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

[SeongJae Park <sj@kernel.org>]

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

[...]