Re: [RFC PATCH v1.1 2/7] mm/damon/core-kunit: check region count before testing in split_at()

On Fri, 17 Jul 2026 14:51:56 +0000 [email protected] wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [Critical] Severe out-of-bounds slab write and infinite kernel loop in `damon_apply_min_nr_regions()` due to unchecked `damon_split_region_at()` failures.
> --
> 
> --- Patch [2]: [RFC PATCH v1.1 2/7] mm/damon/core-kunit: check region count before testing in split_at() ---
> commit d2fdc98cf0d62ffe11b7dca50c29ff7c72e1fcd6
> Author: SJ Park <[email protected]>
> 
> mm/damon/core-kunit: check region count before testing in split_at()
> 
> This commit fixes a potential invalid pointer dereference in the kunit tests
> by checking if the region count changed as expected after a split attempt.
> Since region splits can fail, it ensures the test exits early rather than
> accessing an invalid next region.
> 
> Link: https://lore.kernel.org/[email protected]
> 
> > diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h
> > index eba643762132f..322a295ac2cec 100644
> > --- a/mm/damon/tests/core-kunit.h
> > +++ b/mm/damon/tests/core-kunit.h
> > @@ -155,6 +155,10 @@ static void damon_test_split_at(struct kunit *test)
> >  	r->age = 10;
> >  	damon_add_region(r, t);
> >  	damon_split_region_at(t, r, 25);
> > +	KUNIT_EXPECT_EQ(test, damon_nr_regions(t), 2);
> > +	if (damon_nr_regions(t) != 2)
> > +		goto out;
> > +
> 
> [Severity: Critical]
> This is a pre-existing issue, but since this patch correctly notes that
> damon_split_region_at() can fail, does the same unhandled failure cause an
> out-of-bounds access in the core code?
> 
> Looking at 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 a sub-region under memory
> pressure, it silently returns. The loop then unconditionally executes
> r = damon_next_region(r). Since no new sub-region was created, it skips the
> remainder of the original region and advances to the next one.
> 
> If this happens on the last region in the list, r evaluates to the list head
> at &t->regions_list, pushing the region pointer out-of-bounds to
> (char *)t - 24. The loop would then read garbage bytes for the region
> boundaries.
> 
> If those garbage bytes evaluate to a size greater than max_region_sz, could
> it call damon_split_region_at() again on this invalid pointer? If a
> subsequent allocation succeeds, r->ar.end = new->ar.start could write a
> user-monitored address into the out-of-bounds slab memory, potentially
> corrupting memory 16 bytes before the damon_target object.
> 
> Additionally, if allocations consistently fail, could the fake region's next
> pointer wrap back to the first valid region and create an unbreakable infinite
> loop in the kdamond thread?

This is a known issue that I'm separately working on.

> 
> >  	KUNIT_EXPECT_EQ(test, r->ar.start, 0ul);
> >  	KUNIT_EXPECT_EQ(test, r->ar.end, 25ul);
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2


Thanks,
SJ