* Re: [PATCH 6.6.y] mm/damon/vaddr-kunit: check region count in three_regions test
2026-09-09 3:49 ` [PATCH 6.6.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 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
^ permalink raw reply [flat|nested] 3+ messages in thread