Re: [PATCH v1.1 2/6] mm/damon/tests/core-kunit: catch test failure in test_merge_regions_of()

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

Pre-existing issues:
- [Medium] The patch correctly identifies that `KUNIT_EXPECT_EQ()` does not abort test execution on failure, which causes undefined behavior in subsequent code if the test expectation is not met. However, it leaves the exact same anti-pattern unfixed in several other test helper functions within the DAMON test suite. This can lead to out-of-bounds (OOB) array reads, NULL pointer dereferences, and slab corruption when test failures occur.
--

--- Patch [2]: [PATCH v1.1 2/6] mm/damon/tests/core-kunit: catch test failure in test_merge_regions_of() ---
commit ce2690fbbc97573d0a06744653f652063e18c91b
Author: SJ Park <[email protected]>
Date:   Wed Jul 15 03:09:57 2026 +0000

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

This commit fixes an issue where KUNIT_EXPECT_EQ() failures do not abort
the test, leading to potential NULL pointer dereferences in subsequent
code. It addresses this in damon_test_merge_regions_of() by explicitly
checking the expectation and skipping the rest of the test if it fails.

Link: https://lore.kernel.org/[email protected]

> diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h
> index 485472ddebd19..eba643762132f 100644
> --- a/mm/damon/tests/core-kunit.h
> +++ b/mm/damon/tests/core-kunit.h
> @@ -260,11 +260,14 @@ static void damon_test_merge_regions_of(struct kunit *test)
>  	damon_merge_regions_of(t, 9, 9999, ctx, true);
>  	/* 0-112, 114-130, 130-156, 156-170, 170-230, 230-10170 */
>  	KUNIT_EXPECT_EQ(test, damon_nr_regions(t), 6u);
> +	if (damon_nr_regions(t) != 6)
> +		goto out;
>  	for (i = 0; i < 6; 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);
>  	damon_destroy_ctx(ctx);
>  }

[Severity: Medium]
This is a pre-existing issue, but while you are fixing this anti-pattern
here, the exact same missing abort condition exists in several other
functions within the DAMON test suite.

For example, in mm/damon/tests/core-kunit.h:damos_test_commit_dests_for():

    KUNIT_EXPECT_EQ(test, dst.nr_dests, src_nr_dests);
    for (i = 0; i < dst.nr_dests; i++) {
        KUNIT_EXPECT_EQ(test, dst.node_id_arr[i], src_node_id_arr[i]);

If the expectation fails, could this loop read out-of-bounds on
src_node_id_arr since it iterates based on dst.nr_dests?

Similarly, in mm/damon/tests/core-kunit.h:damon_test_split_at():

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

Could damon_next_region() yield an invalid pointer if the split fails,
causing a crash when r_new->ar.start is accessed unconditionally?

The same unbounded array access or unsafe pointer dereference pattern
appears to exist in:
- mm/damon/tests/core-kunit.h:damos_test_commit_quota_goals_for()
- mm/damon/tests/core-kunit.h:damon_test_commit_target_regions_for()
- mm/damon/tests/core-kunit.h:damos_test_filter_out()
- mm/damon/tests/vaddr-kunit.h:damon_do_test_apply_three_regions()

Would it make sense to update these remaining occurrences in a follow-up or
v2 of this patch to prevent slab corruption or NULL pointer dereferences
during test failures?

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