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

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but it shares the exact same logic flaw fixed here. In `damon_apply_min_nr_regions()`, if `damon_split_region_at()` fails to allocate a new region (e.g., due to memory pressure), it leaves `r` unmodified. However, the inner `while` loop unconditionally executes `r = damon_next_region(r);`. If `r` was the last region in the list, `damon_next_region(r)` will return a pointer derived from the list head (`&t->regions_list`), resulting in a fake `damon_region` pointer pointing into the middle of the `damon_target` struct or preceding memory. The loop then evaluates `damon_sz_region(r)` on this fake region, reading garbage memory. If the garbage evaluates to a size greater than `max_region_sz`, it will invoke `damon_split_region_at()` on the fake region, causing an out-of-bounds write and list corruption (overwriting `r_fake->ar.end` and `t->regions_list`), or an infinite loop lockup if allocations keep failing.
--

--- Patch [4]: [RFC PATCH v1.1 4/7] mm/damon/core-kunit: handle region split failure in filter_out() ---
commit 571c1400b8cb843c3acdb1ded0915fd9807f302f
Author: SJ Park <[email protected]>

mm/damon/core-kunit: handle region split failure in filter_out()

This commit fixes a bug in the DAMON kunit tests where test execution
continued even if a region failed to split as expected. By exiting early
when the number of regions does not match expectations, it prevents
dereferencing invalid pointers and potential memory corruption.

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;

[Severity: High]
This isn't a bug introduced by this patch, but it shares the exact same logic
flaw fixed here.

In mm/damon/core.c:damon_apply_min_nr_regions():

		damon_for_each_region_safe(r, next, t) {
			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 new region (e.g., due to memory
pressure), it leaves r unmodified. However, the inner while loop
unconditionally executes r = damon_next_region(r).

If r was the last region in the list, damon_next_region(r) will return a
pointer derived from the list head (&t->regions_list), resulting in a fake
damon_region pointer. The loop then evaluates damon_sz_region(r) on this fake
region, reading garbage memory.

If the garbage evaluates to a size greater than max_region_sz, it will invoke
damon_split_region_at() on the fake region.

Can this lead to an out-of-bounds write and list corruption, or an infinite
loop lockup if allocations keep failing?

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