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

[PATCH 5.15.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/core-test.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/mm/damon/core-test.h b/mm/damon/core-test.h
index c19da2e3d4936..3350293761c4c 100644
--- a/mm/damon/core-test.h
+++ b/mm/damon/core-test.h
@@ -251,11 +251,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 5.15.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 2 potential issue(s) to consider:

Pre-existing issues:
- [High] The identical NULL pointer dereference bug fixed by this patch in `damon_test_merge_regions_of()` is also present in `damon_do_test_apply_three_regions()`, making this an incomplete fix.
- [High] Out-of-bounds pointer dereference in `damon_test_split_at()` due to unhandled failure of `damon_split_region_at()`.
--

commit 8d430ac7e5b2f68aa2392c6b505d7132a6bdebdb
Author: SJ Park <sj@kernel.org>
Date:   Wed Sep 9 03:40:38 2026 +0000

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

This commit fixes a NULL pointer dereference in damon_test_merge_regions_of()
caused by assuming KUNIT_EXPECT_EQ() aborts on failure. It catches the
expectation failure to skip the subsequent per-region tests that would otherwise
crash.

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 c19da2e3d4936..3350293761c4c 100644
> --- a/mm/damon/core-test.h
> +++ b/mm/damon/core-test.h
> @@ -251,11 +251,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);

[Severity: High]
This isn't a bug introduced by this patch, but the identical NULL pointer
dereference bug fixed here in damon_test_merge_regions_of() is also present
in damon_do_test_apply_three_regions() within mm/damon/vaddr-test.h:

	r = __nth_region_of(t, i);
	KUNIT_EXPECT_EQ(test, r->ar.start, expected[i * 2]);

If damon_va_apply_three_regions() fails to create the expected number of
regions, the target t will contain fewer regions than expected. The test loop
unconditionally calls __nth_region_of(), which returns NULL for the missing
regions. Because KUNIT_EXPECT_EQ() does not abort the test, r->ar.start
immediately dereferences the NULL pointer.

Does it make sense to fix this occurrence in
damon_do_test_apply_three_regions() as well?

[Severity: High]
This is a pre-existing issue, but there is an out-of-bounds pointer
dereference in damon_test_split_at() in this same file (mm/damon/core-test.h)
due to an unhandled failure:

	damon_split_region_at(c, t, r, 25);
	KUNIT_EXPECT_EQ(test, r->ar.start, 0ul);
	KUNIT_EXPECT_EQ(test, r->ar.end, 25ul);

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

If memory allocation for the new region fails inside damon_split_region_at(),
it silently returns and leaves r as the only element in the list. When the
test executes r = damon_next_region(r), its list.next points to the list head
(&t->regions_list).

The damon_next_region() macro uses container_of() which subtracts the offset
of the list field, yielding a bogus pointer 16 bytes before the allocated
struct damon_target memory. The subsequent KUNIT_EXPECT_EQ() then dereferences
this bogus pointer.

Should this unhandled error path be addressed similarly?

>  		KUNIT_EXPECT_EQ(test, r->ar.start, saddrs[i]);
>  		KUNIT_EXPECT_EQ(test, r->ar.end, eaddrs[i]);
>  	}
> +out:
>  	damon_free_target(t);
>  }
>

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

Re: [PATCH 5.15.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 5.15, thanks.

-- 
Thanks,
Sasha