Re: [PATCH 6.6.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 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The patch addresses a missing execution abortion on expectation failure in `damon_test_merge_regions_of()`, but fails to fix identical vulnerabilities in several other test functions where unmet expectations or failed operations lead to invalid memory accesses, out-of-bounds reads, and kernel heap corruption.
--

commit 6344f23ed0a644a799bb1ef7adb4c771a412bbdc
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.  Fix the issue
by catching the expectation failure and skipping the per-region tests to
avoid NULL pointer dereferences.

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/core-test.h b/mm/damon/core-test.h
> index a2aa410d6d0d0..2e191cc4a1b3d 100644
> --- a/mm/damon/core-test.h
> +++ b/mm/damon/core-test.h
> @@ -253,11 +253,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 is a pre-existing issue, but this patch addresses the missing execution
abort for damon_test_merge_regions_of() while leaving identical issues
in other test functions.

In mm/damon/vaddr-test.h:damon_do_test_apply_three_regions():

	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 the target holds fewer regions than nr_expected / 2, could
__nth_region_of() return a NULL pointer, leading to a NULL pointer
dereference?

In mm/damon/core-test.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, does r remain the last element?  If so, would
damon_next_region(r) return a pseudo-pointer derived from the list head,
causing garbage reads and potentially corrupting the kernel heap during
damon_destroy_region()?

There is also a second instance of this in 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 * 6);
	KUNIT_EXPECT_EQ(test, r2->ar.end, DAMON_MIN_REGION * 8);
	damon_destroy_region(r2, t);

Does this second unmet region split expectation also cause a pseudo-pointer
to be passed to damon_destroy_region(), crashing the system?

In mm/damon/core-test.h:damon_test_split_at():

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

If damon_split_region_at() fails silently, does damon_next_region(r) return a
pseudo-pointer that is immediately dereferenced, causing an out-of-bounds
read?

In mm/damon/core-test.h:damon_test_set_regions():

	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 damon_nr_regions(t) is unexpectedly greater than 3, will the loop execute
more times than the size of the 6-element expects array?  Could this cause
expect_idx to overrun and trigger an out-of-bounds stack read?

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