Re: [PATCH 6.18.y] mm/damon/tests/core-kunit: catch test failure in test_merge_regions_of()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Unchecked expectation failure leads to NULL pointer dereference in `damon_do_test_apply_three_regions()`.
- [High] Unchecked expectation failures when splitting regions lead to out-of-bounds reads and invalid frees (slab corruption).
- [High] Unchecked expectation failures allow an out-of-bounds read on the stack-allocated `expects` array in `damon_test_set_regions()`.
--

commit 3412543f0a4dba58f4602d79dd62e10b9925d6a5
Author: SJ Park <sj@kernel.org>

mm/damon/tests/core-kunit: catch test failure in test_merge_regions_of()

KUNIT_EXPECT_EQ() does not abort the execution of test code when the
expectation is not met.  But damon_test_merge_regions_of() code after its
initial KUNIT_EXPECT_EQ() call assumes the expectation is met.  It does a
per-region test with a hard-coded number of regions that is correct only
if the expectation was met.  As a result, __nth_region_of() could return
NULL, and the test code can dereference NULL pointers.  Fix the issue by
catching the expectation failure and skip the per-region tests.

Link: https://lore.kernel.org/20260715031002.108504-3-sj@kernel.org
Link: https://lore.kernel.org/20260710144937.26981-1-sj@kernel.org [1]

> diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h
> index 6687778752728..7a3a923dec7cf 100644
> --- a/mm/damon/tests/core-kunit.h
> +++ b/mm/damon/tests/core-kunit.h
> @@ -264,11 +264,14 @@ static void damon_test_merge_regions_of(struct kunit *test)
>  	damon_merge_regions_of(t, 9, 9999);
>  	/* 0-112, 114-130, 130-156, 156-170 */
>  	KUNIT_EXPECT_EQ(test, damon_nr_regions(t), 5u);
> +	if (damon_nr_regions(t) != 5)
> +		goto out;
>  	for (i = 0; i < 5; i++) {
>  		r = __nth_region_of(t, i);
>  		KUNIT_EXPECT_EQ(test, r->ar.start, saddrs[i]);
>  		KUNIT_EXPECT_EQ(test, r->ar.end, eaddrs[i]);
>  	}
> +out:
>  	damon_free_target(t);
>  }

[Severity: High]
This isn't a bug introduced by this patch, but since this patch fixes an
unchecked KUNIT_EXPECT_EQ() failure, does the same issue exist in
damon_do_test_apply_three_regions() in mm/damon/tests/vaddr-kunit.h?

mm/damon/tests/vaddr-kunit.h:damon_do_test_apply_three_regions() {
	...
	damon_set_regions(t, three_regions, 3, DAMON_MIN_REGION);

	for (i = 0; i < nr_expected / 2; i++) {
		r = __nth_region_of(t, i);
		KUNIT_EXPECT_EQ(test, r->ar.start, expected[i * 2]);
		KUNIT_EXPECT_EQ(test, r->ar.end, expected[i * 2 + 1]);
	}
	...
}

If damon_set_regions() fails to create the expected number of regions,
__nth_region_of() returns NULL, leading to a NULL pointer dereference
when accessing r->ar.start.

[Severity: High]
This is a pre-existing issue, but can unchecked expectation failures when
splitting regions lead to out-of-bounds reads and slab corruption?

mm/damon/tests/core-kunit.h:damon_test_split_at() {
	...
	damon_split_region_at(t, r, 25);
	KUNIT_EXPECT_EQ(test, r->ar.start, 0ul);
	KUNIT_EXPECT_EQ(test, r->ar.end, 25ul);

	r_new = damon_next_region(r);
	KUNIT_EXPECT_EQ(test, r_new->ar.start, 25ul);
	...
}

A similar pattern exists in damos_test_filter_out():

mm/damon/tests/core-kunit.h:damos_test_filter_out() {
	...
	KUNIT_EXPECT_EQ(test, damon_nr_regions(t), 2);
	r2 = damon_next_region(r);
	KUNIT_EXPECT_EQ(test, r2->ar.start, DAMON_MIN_REGION * 2);
	KUNIT_EXPECT_EQ(test, r2->ar.end, DAMON_MIN_REGION * 4);
	damon_destroy_region(r2, t);
	...
}

If the region split fails, damon_next_region() returns an invalid pointer
derived from the list head. Dereferencing it reads out-of-bounds, and
passing it to damon_destroy_region() causes a SLUB BUG.

[Severity: High]
This is a pre-existing issue, but does damon_test_set_regions() have an
out-of-bounds read on the expects[] array if the expectation fails?

mm/damon/tests/core-kunit.h:damon_test_set_regions() {
	...
	unsigned long expects[] = {8, 16, 16, 24, 24, 28};
	...
	KUNIT_EXPECT_EQ(test, damon_nr_regions(t), 3);
	damon_for_each_region(r, t) {
		KUNIT_EXPECT_EQ(test, r->ar.start, expects[expect_idx++]);
		KUNIT_EXPECT_EQ(test, r->ar.end, expects[expect_idx++]);
	}
	...
}

If there are more than 3 regions, the loop increments expect_idx past the
bounds of the stack-allocated expects[] array.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260909063159.63420-1-sj@kernel.org?part=1