[PATCH 6.12.y] mm/damon/vaddr-kunit: check region count in three_regions test

[PATCH 6.12.y] mm/damon/vaddr-kunit: check region count in three_regions test

[SJ Park]

damon_do_test_apply_three_regions() iterates regions after
damon_set_regions() call assuming the function would succeed at setting
the number of regions the same to the expected one.  It might have failed.
In this case, __nth_region_of() in the iteration could return NULL and
NULL dereference can happen in the test.

The consequent user impact (NULL dereference) is quite bad.  The realistic
user impact would be limited, though.  It would affect only test run
setups.

Fix it by testing if the number of regions was also changed as expected
and exit early for the failure.

The issue was discovered [1] by Sashiko.

Link: https://lore.kernel.org/20260718001442.87129-4-sj@kernel.org
Link: https://lore.kernel.org/20260713144757.39740-1-sj@kernel.org [1]
Fixes: 17ccae8bb5c9 ("mm/damon: add kunit tests")
Signed-off-by: SJ Park <sj@kernel.org>
Cc: Brendan Higgins <brendan.higgins@linux.dev>
Cc: <stable@vger.kernel.org> # 5.15.x
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
(cherry picked from commit 5fea07e460874c8c7cf00f728efbe22abc62c8d8)
Signed-off-by: SJ Park <sj@kernel.org>
---
 mm/damon/tests/vaddr-kunit.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/mm/damon/tests/vaddr-kunit.h b/mm/damon/tests/vaddr-kunit.h
index ed2845d9f9a95..9eead9882dcce 100644
--- a/mm/damon/tests/vaddr-kunit.h
+++ b/mm/damon/tests/vaddr-kunit.h
@@ -149,12 +149,17 @@ static void damon_do_test_apply_three_regions(struct kunit *test,
 
 	damon_set_regions(t, three_regions, 3);
 
+	KUNIT_EXPECT_EQ(test, damon_nr_regions(t), nr_expected / 2);
+	if (damon_nr_regions(t) != nr_expected / 2)
+		goto out;
+
 	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]);
 	}
 
+out:
 	damon_destroy_target(t);
 }
 
-- 
2.47.3

Re: [PATCH 6.12.y] mm/damon/vaddr-kunit: check region count in three_regions test

[sashiko-bot]

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

Pre-existing issues:
- [High] Incomplete fix: The same vulnerable test pattern fixed by this patch in `vaddr-kunit.h` also exists in `mm/damon/tests/core-kunit.h` within `damon_test_merge_regions_of()`, where a hardcoded loop accesses regions via `__nth_region_of()` after an unchecked operation.
--

commit 95900378d75f76847738ad9f2040d727e3455432
Author: SJ Park <sj@kernel.org>

    mm/damon/vaddr-kunit: check region count in three_regions test

This commit adds a check in damon_do_test_apply_three_regions() to verify
that damon_set_regions() updated the region count as expected. If the count
is wrong, it exits early to prevent a potential NULL pointer dereference
when accessing the regions.

Link: https://lore.kernel.org/20260718001442.87129-4-sj@kernel.org
Link: https://lore.kernel.org/20260713144757.39740-1-sj@kernel.org [1]

> diff --git a/mm/damon/tests/vaddr-kunit.h b/mm/damon/tests/vaddr-kunit.h
> index ed2845d9f9a95..9eead9882dcce 100644
> --- a/mm/damon/tests/vaddr-kunit.h
> +++ b/mm/damon/tests/vaddr-kunit.h
> @@ -149,12 +149,17 @@ static void damon_do_test_apply_three_regions(struct kunit *test,
>  
>  	damon_set_regions(t, three_regions, 3);
>  
> +	KUNIT_EXPECT_EQ(test, damon_nr_regions(t), nr_expected / 2);
> +	if (damon_nr_regions(t) != nr_expected / 2)
> +		goto out;
> +
>  	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]);
>  	}

[Severity: High]
This is a pre-existing issue, but should a similar fix also be applied to
mm/damon/tests/core-kunit.h?

Looking at damon_test_merge_regions_of() in mm/damon/tests/core-kunit.h,
there is an identical vulnerable pattern:

	KUNIT_EXPECT_EQ(test, damon_nr_regions(t), 5u);
	for (i = 0; i < 5; i++) {
		r = __nth_region_of(t, i);
		KUNIT_EXPECT_EQ(test, r->ar.start, saddrs[i]);

If a regression causes damon_merge_regions_of() to return fewer than 5
regions, the KUNIT_EXPECT_EQ() check will log a failure but will not abort
execution.

Will the subsequent loop continue executing past the actual number of
regions, causing __nth_region_of(t, i) to return NULL? Does this result
in a NULL pointer dereference when accessing r->ar.start that crashes the
test execution environment?

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

Re: [PATCH 6.12.y] mm/damon/vaddr-kunit: check region count in three_regions test

[Sasha Levin]

> damon_do_test_apply_three_regions() iterates regions after
> damon_set_regions() call assuming the function would succeed at setting
> the number of regions the same to the expected one.  It might have failed.

Queued for 6.12, thanks.

-- 
Thanks,
Sasha