Re: [RFC PATCH 1/8] mm/damon/core: safely validate src on damon_commit_ctx()

[SJ Park <sj@kernel.org>]

On Wed, 01 Jul 2026 15:01:10 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Validation step incorrectly tests the existing context (`dst`) instead of the new parameters (`src`), completely neutralizing the patch's safety guarantees.
> --
> 
> commit 4ac81fdba8a8c8576fad127fdd4347015f4dac51
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/core: safely validate src on damon_commit_ctx()
> 
> This commit updates damon_commit_ctx() to validate parameters safely by
> applying them to a test context before applying them to the running context.
> If the test context validation succeeds, the real update proceeds.
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index 5febe58a07028..5ca176eb27e1d 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> 
> [ ... ]
> 
> > @@ -1635,6 +1622,52 @@ int damon_commit_ctx(struct damon_ctx *dst, struct damon_ctx *src)
> 
> [ ... ]
> 
> > +int damon_commit_ctx(struct damon_ctx *dst, struct damon_ctx *src)
> > +{
> > +	struct damon_ctx *test_ctx;
> > +	int err;
> > +
> > +	test_ctx = damon_new_test_ctx(dst);
> > +	if (!test_ctx)
> > +		return -ENOMEM;
> > +	err = __damon_commit_ctx(test_ctx, dst);
> 
> [Severity: High]
> Does this code incorrectly validate the existing context instead of the new
> parameters?
> 
> test_ctx is initialized as a clone of dst via damon_new_test_ctx(dst). If
> __damon_commit_ctx() applies dst to test_ctx again, it avoids testing the
> new parameters in src entirely.
> 
> If src contains invalid parameters (like a non-power-of-two min_region_sz),
> it will bypass this check. When the unvalidated src is subsequently applied
> to the running context:
> 
>     err = __damon_commit_ctx(dst, src);
>     
> it can fail partway through. This leaves dst->maybe_corrupted = true and
> causes the running kdamond monitoring thread to terminate unexpectedly.
> 
> Should the validation step be checking src instead?
> 
>     err = __damon_commit_ctx(test_ctx, src);

Correct.  Maybe I made a copy-pasta.  I will fix this in the next revisssion.


Thanks,
SJ

[...]