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

[PATCH 6.1.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/vaddr-test.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/mm/damon/vaddr-test.h b/mm/damon/vaddr-test.h
index dda3c186ffefe..bed92599e9782 100644
--- a/mm/damon/vaddr-test.h
+++ b/mm/damon/vaddr-test.h
@@ -141,12 +141,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.1.y] mm/damon/vaddr-kunit: check region count in three_regions test

[sashiko-bot]

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

Pre-existing issues:
- [High] Missing maple tree preallocation before calling `vma_mas_store()` in `__link_vmas()` can trigger a `BUG_ON()`.
- [High] Out-of-bounds read in damon_test_split_at on memory allocation failure.
--

commit 2d4b725be5cfa292d091f09d09b2d8ec22b7965e
Author: SJ Park <sj@kernel.org>

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

This commit fixes a potential NULL pointer dereference in the DAMON kunit
tests. It ensures that damon_do_test_apply_three_regions() checks if the
expected number of regions was successfully set before iterating over them,
exiting early if the allocation failed.

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 dda3c186ffefe..bed92599e9782 100644
> --- a/mm/damon/vaddr-test.h
> +++ b/mm/damon/vaddr-test.h
> @@ -141,12 +141,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);
>  }

As a side note, looking at __link_vmas() earlier in the same file:

mm/damon/vaddr-test.h:__link_vmas() {
...
	mas_lock(&mas);
	for (i = 0; i < nr_vmas; i++)
		vma_mas_store(&vmas[i], &mas);
	mas_unlock(&mas);
...
}

[Severity: High]
This isn't a bug introduced by this patch, but is it safe to call
vma_mas_store() repeatedly here without any preceding call to
mas_preallocate()?

If dynamic node allocation fails under memory pressure during
mas_store_prealloc(), mas_wr_store_entry() sets an error state on the
maple state.

mas_store_prealloc() concludes with a BUG_ON(mas_is_err(mas)), which
will panic the system when damon_test_three_regions_in_vmas() runs.

Also, looking at damon_test_split_at() in mm/damon/core-test.h:

mm/damon/core-test.h:damon_test_split_at() {
...
	damon_split_region_at(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);
...
}

[Severity: High]
This is a pre-existing issue, but could this cause an out-of-bounds read if
damon_split_region_at() fails to allocate memory for the new region?

If damon_split_region_at() fails, it returns silently without splitting the
region. When damon_next_region(r) is called, list.next points to the list head
(&t->regions_list) since r is the only item in the list.

The container_of macro then yields an out-of-bounds pointer located before
the start of the damon_target struct. Accessing r->ar.start will read adjacent
slab memory.

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

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

-- 
Thanks,
Sasha