Re: [PATCH 4/7] mm/damon/core-kunit: handle region split failure in filter_out()

On Sat, 18 Jul 2026 00:29:09 +0000 [email protected] wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] In `damon_apply_min_nr_regions()`, `damon_split_regions_of()`, and `kdamond_split_regions()`, if `damon_split_region_at()` fails to allocate memory for a new region, the loop variable `r` is still unconditionally advanced to the next region. This leads to iterating past the end of the region list, reading from out-of-bounds memory, and potentially causing memory corruption.
> --
> 
> --- Patch [4]: [PATCH 4/7] mm/damon/core-kunit: handle region split failure in filter_out() ---
> commit 8274c3713b297d606d32992612a8cbd24ac04759
> Author: SJ Park <[email protected]>
> 
> mm/damon/core-kunit: handle region split failure in filter_out()
> 
> This commit modifies the kunit test damos_test_filter_out() to add early
> exit checks when damon_nr_regions() does not return the expected value. This
> prevents the test from dereferencing invalid pointers if
> damon_split_region_at() fails. It fixes memory corruption that could occur
> during test runs.
> 
> Link: https://lore.kernel.org/[email protected]
> 
> > diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h
> > index 322a295ac2cec..eddf3a0484583 100644
> > --- a/mm/damon/tests/core-kunit.h
> > +++ b/mm/damon/tests/core-kunit.h
> > @@ -1365,6 +1365,8 @@ static void damos_test_filter_out(struct kunit *test)
> >  	KUNIT_EXPECT_EQ(test, r->ar.start, 1);
> >  	KUNIT_EXPECT_EQ(test, r->ar.end, 2);
> >  	KUNIT_EXPECT_EQ(test, damon_nr_regions(t), 2);
> > +	if (damon_nr_regions(t) != 2)
> > +		goto out;
> >  	r2 = damon_next_region(r);
> 
> [ ... ]
> 
> [Severity: High]
> This is a pre-existing issue, but I noticed a similar pattern in mm/damon/core.c
> where damon_split_region_at() failures might not be handled safely.
> 
> In damon_apply_min_nr_regions():
> 
> mm/damon/core.c:damon_apply_min_nr_regions() {
>     ...
> 			while (damon_sz_region(r) > max_region_sz) {
> 				damon_split_region_at(t, r, max_region_sz);
> 				r = damon_next_region(r);
> 			}
>     ...
> }
> 
> If damon_split_region_at() fails to allocate memory due to memory pressure and
> silently returns, does the unconditional call to damon_next_region(r) advance
> past the end of the list?
> 
> If `r` was the last region in the target's list, could `r` become the list head
> and point to memory before the struct damon_target?
> 
> If so, the condition while (damon_sz_region(r) > max_region_sz) could read
> from out-of-bounds memory.
> 
> If the resulting garbage size exceeds max_region_sz, it might call
> damon_split_region_at() on the invalid pointer, which could then corrupt the
> heap by writing to r->ar.end.

Yes, I'm separately working [1] on it.

> 
> The same pattern appears in damon_split_regions_of():
> 
> mm/damon/core.c:damon_split_regions_of() {
>     ...
> 			damon_split_region_at(t, r, sz_sub);
> 			sz_region = sz_region - sz_sub;
> 			r = damon_next_region(r);
>     ...
> }
> 
> And also in kdamond_split_regions():
> 
> mm/damon/core.c:kdamond_split_regions() {
>     ...
> 				damon_split_region_at(t, r, sz_sub);
> 				sz_region = sz_region - sz_sub;
> 				r = damon_next_region(r);
>     ...
> }
> 
> Can these paths also wrap around the list and cause out-of-bounds accesses
> during memory pressure in the kdamond thread?

No, the above two cases are hallucinations.  The example code snippet is
different from the real ones.

> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4

[1] https://lore.kernel.org/[email protected]


Thanks,
SJ