* Re: [PATCH 6.1.y] mm/damon/vaddr-kunit: check region count in three_regions test
2026-09-09 3:47 ` [PATCH 6.1.y] mm/damon/vaddr-kunit: check region count in three_regions test SJ Park
@ 2026-09-09 4:00 ` sashiko-bot
2026-09-09 20:26 ` Sasha Levin
1 sibling, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-09-09 4:00 UTC (permalink / raw)
To: SJ Park; +Cc: damon
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
^ permalink raw reply [flat|nested] 3+ messages in thread