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