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

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?

>  	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