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