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

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Missing early exit on region count mismatch can lead to a NULL pointer dereference in damon_test_merge_regions_of().
- [High] Missing early exit on region count mismatch can lead to out-of-bounds array read in damon_test_set_regions().
--

commit 3ac2a42fcb13001506625ee82bd4439b241b1513
Author: SJ Park <sj@kernel.org>

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

damon_do_test_apply_three_regions() iterates regions after a 
damon_set_regions() call assuming the region count matches the expected 
number. If it fails, __nth_region_of() can return NULL, leading to a NULL 
dereference during the test. This commit fixes the issue by explicitly 
testing if the number of regions changed as expected and exiting early on 
failure.

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/vaddr-test.h b/mm/damon/vaddr-test.h
> index 292736d3f3954..c0546447b21e6 100644
> --- a/mm/damon/vaddr-test.h
> +++ b/mm/damon/vaddr-test.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;
> +

[Severity: High]
This isn't a bug introduced by this patch, but since we're fixing missing 
early exits on region count mismatches here, there are similar vulnerabilities 
in mm/damon/core-test.h that might need the same treatment.

In mm/damon/core-test.h:damon_test_merge_regions_of():

	damon_merge_regions_of(t, 9, 9999);
	/* 0-112, 114-130, 130-156, 156-170 */
	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]);

Can this lead to a NULL pointer dereference? 

If damon_merge_regions_of() produces fewer than 5 regions due to a test 
failure, KUNIT_EXPECT_EQ logs the failure but continues execution. The loop 
attempts to fetch the 5th region using __nth_region_of(), which returns NULL. 
Dereferencing r->ar.start then causes a panic.

[Severity: High]
This is a pre-existing issue, but there is also a potential out-of-bounds 
read in mm/damon/core-test.h:damon_test_set_regions():

	unsigned long expects[] = {8, 16, 16, 24, 24, 28};
	int expect_idx = 0;
...
	damon_set_regions(t, &range, 1);

	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++]);
	}

Can this lead to an out-of-bounds array read? 

If damon_set_regions() produces more than 3 regions due to a bug in the 
tested logic, KUNIT_EXPECT_EQ logs a failure but continues. The loop 
damon_for_each_region(r, t) then iterates over all regions, causing expect_idx 
to exceed the bounds of the 6-element expects[] array when accessing 
expects[expect_idx++].

>  	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);
>  }

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