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

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

[SJ Park]

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.

The user impact on realistic setups should be negligible, as it is a unit
test.

The issue was discovered [1] by Sashiko.

Link: https://lore.kernel.org/20260715031002.108504-3-sj@kernel.org
Link: https://lore.kernel.org/20260710144937.26981-1-sj@kernel.org [1]
Fixes: 17ccae8bb5c9 ("mm/damon: add kunit tests")
Signed-off-by: SJ Park <sj@kernel.org>
Cc: <stable@vger.kernel.org> # 5.15.x
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
(cherry picked from commit b640708929aa956235866bf7717d89018c661083)
Signed-off-by: SJ Park <sj@kernel.org>
---
 mm/damon/tests/core-kunit.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h
index d7737f5df6688..1053d7acd8e45 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);
 }
 
-- 
2.47.3

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

[sashiko-bot]

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 the patch is an incomplete fix. It addresses an unhandled KUNIT_EXPECT_EQ failure in `damon_test_merge_regions_of()` that causes a NULL pointer dereference, but identical unhandled test failures exist in four other test functions across the DAMON kunit test suite, leading to kernel panics and out-of-bounds memory accesses.
--

commit a4abe0e36a0fa52342ceb358e02a1259ba2ec1d5
Author: SJ Park <sj@kernel.org>
Subject: mm/damon/tests/core-kunit: catch test failure in test_merge_regions_of()

This commit fixes a potential NULL pointer dereference in the DAMON kunit
tests by aborting the test execution in damon_test_merge_regions_of() if
the initial KUNIT_EXPECT_EQ() expectation is not met.

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 d7737f5df6688..1053d7acd8e45 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 this fix appears incomplete.
While it addresses the unhandled test failure in damon_test_merge_regions_of(),
would it make sense to apply similar bailouts to other test functions in the
suite that have identical unhandled expectations?

For example, in 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 fails to split, damon_nr_regions(t) remains 1, and
damon_next_region(r) evaluates to a bogus pointer. Calling 
damon_destroy_region() will attempt to free this invalid address, causing
a kernel panic.

Similarly, in 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);

If damon_split_region_at() fails, damon_next_region() returns an
out-of-bounds pointer, leading to an out-of-bounds read when accessing
r_new->ar.start.

In mm/damon/tests/core-kunit.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 the region count exceeds expectations (>3), expect_idx overflows the
6-element expects stack array, leading to a stack buffer out-of-bounds
read.

Finally, in mm/damon/tests/vaddr-kunit.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 resulting regions are fewer than expected, __nth_region_of() returns
NULL, leading to a direct NULL pointer dereference.

Are there plans to catch these other unhandled test failures in a follow-up
patch?

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

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

[Sasha Levin]

> 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.

Queued for 6.12, thanks.

-- 
Thanks,
Sasha