* [PATCH 6.18.y] mm/damon/tests/core-kunit: catch test failure in test_merge_regions_of()
[not found] <2026090808-phoniness-repaying-46f0@gregkh>
@ 2026-09-09 3:38 ` SJ Park
2026-09-09 3:45 ` sashiko-bot
2026-09-09 6:28 ` SJ Park
2026-09-09 6:31 ` SJ Park
1 sibling, 2 replies; 6+ messages in thread
From: SJ Park @ 2026-09-09 3:38 UTC (permalink / raw)
To: stable; +Cc: damon, SJ Park, Andrew Morton
KUNIT_EXPECT_EQ() does not abort the execution of test code when the
expectation is not met. But damon_test_merge_regions_of() code after its
initial KUNIT_EXPECT_EQ() call assumes the expectation is met. It does a
per-region test with a hard-coded number of regions that is correct only
if the expectation was met. As a result, __nth_region_of() could return
NULL, and the test code can dereference NULL pointers. Fix the issue by
catching the expectation failure and skip the per-region tests.
The user impact on realistic setups should be negligible, as it is a unit
test.
The issue was discovered [1] by Sashiko.
Link: https://lore.kernel.org/20260715031002.108504-3-sj@kernel.org
Link: https://lore.kernel.org/20260710144937.26981-1-sj@kernel.org [1]
Fixes: 17ccae8bb5c9 ("mm/damon: add kunit tests")
Signed-off-by: SJ Park <sj@kernel.org>
Cc: <stable@vger.kernel.org> # 5.15.x
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
(cherry picked from commit b640708929aa956235866bf7717d89018c661083)
Signed-off-by: SJ Park <sj@kernel.org>
---
mm/damon/tests/core-kunit.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h
index 6687778752728..c1f0be6746abd 100644
--- a/mm/damon/tests/core-kunit.h
+++ b/mm/damon/tests/core-kunit.h
@@ -264,11 +264,14 @@ static void damon_test_merge_regions_of(struct kunit *test)
damon_merge_regions_of(t, 9, 9999);
/* 0-112, 114-130, 130-156, 156-170 */
KUNIT_EXPECT_EQ(test, damon_nr_regions(t), 5u);
+ if (damon_nr_regions(t) != 6)
+ goto out;
for (i = 0; i < 5; i++) {
r = __nth_region_of(t, i);
KUNIT_EXPECT_EQ(test, r->ar.start, saddrs[i]);
KUNIT_EXPECT_EQ(test, r->ar.end, eaddrs[i]);
}
+out:
damon_free_target(t);
}
--
2.47.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 6.18.y] mm/damon/tests/core-kunit: catch test failure in test_merge_regions_of()
2026-09-09 3:38 ` [PATCH 6.18.y] mm/damon/tests/core-kunit: catch test failure in test_merge_regions_of() SJ Park
@ 2026-09-09 3:45 ` sashiko-bot
2026-09-09 6:26 ` SJ Park
2026-09-09 6:28 ` SJ Park
1 sibling, 1 reply; 6+ messages in thread
From: sashiko-bot @ 2026-09-09 3:45 UTC (permalink / raw)
To: SJ Park; +Cc: damon
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Typo in the early exit condition causes the unit test to silently skip validation on successful execution.
--
commit 8cca227124d565ed51f4791e649f908f0754a67f
Author: SJ Park <sj@kernel.org>
mm/damon/tests/core-kunit: catch test failure in test_merge_regions_of()
This commit adds an early exit to the damon_test_merge_regions_of() unit test
to catch when the region merge expectation is not met. By skipping the
per-region test loop on failure, it prevents potential NULL pointer
dereferences.
Link: https://lore.kernel.org/20260715031002.108504-3-sj@kernel.org
Link: https://lore.kernel.org/20260710144937.26981-1-sj@kernel.org [1]
> diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h
> index 6687778752728..c1f0be6746abd 100644
> --- a/mm/damon/tests/core-kunit.h
> +++ b/mm/damon/tests/core-kunit.h
> @@ -264,11 +264,14 @@ static void damon_test_merge_regions_of(struct kunit *test)
> damon_merge_regions_of(t, 9, 9999);
> /* 0-112, 114-130, 130-156, 156-170 */
> KUNIT_EXPECT_EQ(test, damon_nr_regions(t), 5u);
> + if (damon_nr_regions(t) != 6)
> + goto out;
[Severity: Medium]
Should this check compare against 5 instead of 6?
When damon_merge_regions_of() works correctly, it merges the target into
exactly 5 regions, matching the KUNIT_EXPECT_EQ check immediately above.
Because 5 != 6 evaluates to true, a successful test execution will jump out
and silently bypass the per-region validation loop. The validations will only
execute if exactly 6 regions are produced, which would indicate a test failure.
> for (i = 0; i < 5; i++) {
> r = __nth_region_of(t, i);
> KUNIT_EXPECT_EQ(test, r->ar.start, saddrs[i]);
> KUNIT_EXPECT_EQ(test, r->ar.end, eaddrs[i]);
> }
> +out:
> damon_free_target(t);
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260909033832.132107-1-sj@kernel.org?part=1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 6.18.y] mm/damon/tests/core-kunit: catch test failure in test_merge_regions_of()
2026-09-09 3:45 ` sashiko-bot
@ 2026-09-09 6:26 ` SJ Park
0 siblings, 0 replies; 6+ messages in thread
From: SJ Park @ 2026-09-09 6:26 UTC (permalink / raw)
To: sashiko-bot; +Cc: SJ Park, damon
On Wed, 09 Sep 2026 03:45:27 +0000 sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] Typo in the early exit condition causes the unit test to silently skip validation on successful execution.
> --
>
> commit 8cca227124d565ed51f4791e649f908f0754a67f
> Author: SJ Park <sj@kernel.org>
>
> mm/damon/tests/core-kunit: catch test failure in test_merge_regions_of()
>
> This commit adds an early exit to the damon_test_merge_regions_of() unit test
> to catch when the region merge expectation is not met. By skipping the
> per-region test loop on failure, it prevents potential NULL pointer
> dereferences.
>
> Link: https://lore.kernel.org/20260715031002.108504-3-sj@kernel.org
> Link: https://lore.kernel.org/20260710144937.26981-1-sj@kernel.org [1]
>
> > diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h
> > index 6687778752728..c1f0be6746abd 100644
> > --- a/mm/damon/tests/core-kunit.h
> > +++ b/mm/damon/tests/core-kunit.h
> > @@ -264,11 +264,14 @@ static void damon_test_merge_regions_of(struct kunit *test)
> > damon_merge_regions_of(t, 9, 9999);
> > /* 0-112, 114-130, 130-156, 156-170 */
> > KUNIT_EXPECT_EQ(test, damon_nr_regions(t), 5u);
> > + if (damon_nr_regions(t) != 6)
> > + goto out;
>
> [Severity: Medium]
> Should this check compare against 5 instead of 6?
>
> When damon_merge_regions_of() works correctly, it merges the target into
> exactly 5 regions, matching the KUNIT_EXPECT_EQ check immediately above.
>
> Because 5 != 6 evaluates to true, a successful test execution will jump out
> and silently bypass the per-region validation loop. The validations will only
> execute if exactly 6 regions are produced, which would indicate a test failure.
Nice catch. I will fix.
>
> > for (i = 0; i < 5; i++) {
> > r = __nth_region_of(t, i);
> > KUNIT_EXPECT_EQ(test, r->ar.start, saddrs[i]);
> > KUNIT_EXPECT_EQ(test, r->ar.end, eaddrs[i]);
> > }
> > +out:
> > damon_free_target(t);
> > }
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260909033832.132107-1-sj@kernel.org?part=1
>
Thanks,
SJ
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 6.18.y] mm/damon/tests/core-kunit: catch test failure in test_merge_regions_of()
2026-09-09 3:38 ` [PATCH 6.18.y] mm/damon/tests/core-kunit: catch test failure in test_merge_regions_of() SJ Park
2026-09-09 3:45 ` sashiko-bot
@ 2026-09-09 6:28 ` SJ Park
1 sibling, 0 replies; 6+ messages in thread
From: SJ Park @ 2026-09-09 6:28 UTC (permalink / raw)
To: SJ Park; +Cc: stable, damon, Andrew Morton
On Tue, 8 Sep 2026 20:38:32 -0700 SJ Park <sj@kernel.org> wrote:
> KUNIT_EXPECT_EQ() does not abort the execution of test code when the
> expectation is not met. But damon_test_merge_regions_of() code after its
> initial KUNIT_EXPECT_EQ() call assumes the expectation is met. It does a
> per-region test with a hard-coded number of regions that is correct only
> if the expectation was met. As a result, __nth_region_of() could return
> NULL, and the test code can dereference NULL pointers. Fix the issue by
> catching the expectation failure and skip the per-region tests.
>
> The user impact on realistic setups should be negligible, as it is a unit
> test.
>
> The issue was discovered [1] by Sashiko.
>
> Link: https://lore.kernel.org/20260715031002.108504-3-sj@kernel.org
> Link: https://lore.kernel.org/20260710144937.26981-1-sj@kernel.org [1]
> Fixes: 17ccae8bb5c9 ("mm/damon: add kunit tests")
> Signed-off-by: SJ Park <sj@kernel.org>
> Cc: <stable@vger.kernel.org> # 5.15.x
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> (cherry picked from commit b640708929aa956235866bf7717d89018c661083)
> Signed-off-by: SJ Park <sj@kernel.org>
> ---
> mm/damon/tests/core-kunit.h | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h
> index 6687778752728..c1f0be6746abd 100644
> --- a/mm/damon/tests/core-kunit.h
> +++ b/mm/damon/tests/core-kunit.h
> @@ -264,11 +264,14 @@ static void damon_test_merge_regions_of(struct kunit *test)
> damon_merge_regions_of(t, 9, 9999);
> /* 0-112, 114-130, 130-156, 156-170 */
> KUNIT_EXPECT_EQ(test, damon_nr_regions(t), 5u);
> + if (damon_nr_regions(t) != 6)
> + goto out;
Sashiko found the above if condition is wrong. It should 's/6/5/'. Please
ignore this patch. I will post fixed one as another reply.
[1] https://lore.kernel.org/20260909034527.D29C71F00A3A@smtp.kernel.org
> for (i = 0; i < 5; i++) {
> r = __nth_region_of(t, i);
> KUNIT_EXPECT_EQ(test, r->ar.start, saddrs[i]);
> KUNIT_EXPECT_EQ(test, r->ar.end, eaddrs[i]);
> }
> +out:
> damon_free_target(t);
> }
>
> --
> 2.47.3
>
>
Thanks,
SJ
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 6.18.y] mm/damon/tests/core-kunit: catch test failure in test_merge_regions_of()
[not found] <2026090808-phoniness-repaying-46f0@gregkh>
2026-09-09 3:38 ` [PATCH 6.18.y] mm/damon/tests/core-kunit: catch test failure in test_merge_regions_of() SJ Park
@ 2026-09-09 6:31 ` SJ Park
2026-09-09 6:42 ` sashiko-bot
1 sibling, 1 reply; 6+ messages in thread
From: SJ Park @ 2026-09-09 6:31 UTC (permalink / raw)
To: stable; +Cc: damon, SJ Park, Andrew Morton
KUNIT_EXPECT_EQ() does not abort the execution of test code when the
expectation is not met. But damon_test_merge_regions_of() code after its
initial KUNIT_EXPECT_EQ() call assumes the expectation is met. It does a
per-region test with a hard-coded number of regions that is correct only
if the expectation was met. As a result, __nth_region_of() could return
NULL, and the test code can dereference NULL pointers. Fix the issue by
catching the expectation failure and skip the per-region tests.
The user impact on realistic setups should be negligible, as it is a unit
test.
The issue was discovered [1] by Sashiko.
Link: https://lore.kernel.org/20260715031002.108504-3-sj@kernel.org
Link: https://lore.kernel.org/20260710144937.26981-1-sj@kernel.org [1]
Fixes: 17ccae8bb5c9 ("mm/damon: add kunit tests")
Signed-off-by: SJ Park <sj@kernel.org>
Cc: <stable@vger.kernel.org> # 5.15.x
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
(cherry picked from commit b640708929aa956235866bf7717d89018c661083)
Signed-off-by: SJ Park <sj@kernel.org>
---
mm/damon/tests/core-kunit.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h
index 6687778752728..7a3a923dec7cf 100644
--- a/mm/damon/tests/core-kunit.h
+++ b/mm/damon/tests/core-kunit.h
@@ -264,11 +264,14 @@ static void damon_test_merge_regions_of(struct kunit *test)
damon_merge_regions_of(t, 9, 9999);
/* 0-112, 114-130, 130-156, 156-170 */
KUNIT_EXPECT_EQ(test, damon_nr_regions(t), 5u);
+ if (damon_nr_regions(t) != 5)
+ goto out;
for (i = 0; i < 5; i++) {
r = __nth_region_of(t, i);
KUNIT_EXPECT_EQ(test, r->ar.start, saddrs[i]);
KUNIT_EXPECT_EQ(test, r->ar.end, eaddrs[i]);
}
+out:
damon_free_target(t);
}
--
2.47.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 6.18.y] mm/damon/tests/core-kunit: catch test failure in test_merge_regions_of()
2026-09-09 6:31 ` SJ Park
@ 2026-09-09 6:42 ` sashiko-bot
0 siblings, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-09-09 6:42 UTC (permalink / raw)
To: SJ Park; +Cc: damon
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [High] Unchecked expectation failure leads to NULL pointer dereference in `damon_do_test_apply_three_regions()`.
- [High] Unchecked expectation failures when splitting regions lead to out-of-bounds reads and invalid frees (slab corruption).
- [High] Unchecked expectation failures allow an out-of-bounds read on the stack-allocated `expects` array in `damon_test_set_regions()`.
--
commit 3412543f0a4dba58f4602d79dd62e10b9925d6a5
Author: SJ Park <sj@kernel.org>
mm/damon/tests/core-kunit: catch test failure in test_merge_regions_of()
KUNIT_EXPECT_EQ() does not abort the execution of test code when the
expectation is not met. But damon_test_merge_regions_of() code after its
initial KUNIT_EXPECT_EQ() call assumes the expectation is met. It does a
per-region test with a hard-coded number of regions that is correct only
if the expectation was met. As a result, __nth_region_of() could return
NULL, and the test code can dereference NULL pointers. Fix the issue by
catching the expectation failure and skip the per-region tests.
Link: https://lore.kernel.org/20260715031002.108504-3-sj@kernel.org
Link: https://lore.kernel.org/20260710144937.26981-1-sj@kernel.org [1]
> diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h
> index 6687778752728..7a3a923dec7cf 100644
> --- a/mm/damon/tests/core-kunit.h
> +++ b/mm/damon/tests/core-kunit.h
> @@ -264,11 +264,14 @@ static void damon_test_merge_regions_of(struct kunit *test)
> damon_merge_regions_of(t, 9, 9999);
> /* 0-112, 114-130, 130-156, 156-170 */
> KUNIT_EXPECT_EQ(test, damon_nr_regions(t), 5u);
> + if (damon_nr_regions(t) != 5)
> + goto out;
> for (i = 0; i < 5; i++) {
> r = __nth_region_of(t, i);
> KUNIT_EXPECT_EQ(test, r->ar.start, saddrs[i]);
> KUNIT_EXPECT_EQ(test, r->ar.end, eaddrs[i]);
> }
> +out:
> damon_free_target(t);
> }
[Severity: High]
This isn't a bug introduced by this patch, but since this patch fixes an
unchecked KUNIT_EXPECT_EQ() failure, does the same issue exist in
damon_do_test_apply_three_regions() in mm/damon/tests/vaddr-kunit.h?
mm/damon/tests/vaddr-kunit.h:damon_do_test_apply_three_regions() {
...
damon_set_regions(t, three_regions, 3, DAMON_MIN_REGION);
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]);
}
...
}
If damon_set_regions() fails to create the expected number of regions,
__nth_region_of() returns NULL, leading to a NULL pointer dereference
when accessing r->ar.start.
[Severity: High]
This is a pre-existing issue, but can unchecked expectation failures when
splitting regions lead to out-of-bounds reads and slab corruption?
mm/damon/tests/core-kunit.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_new = damon_next_region(r);
KUNIT_EXPECT_EQ(test, r_new->ar.start, 25ul);
...
}
A similar pattern exists in damos_test_filter_out():
mm/damon/tests/core-kunit.h:damos_test_filter_out() {
...
KUNIT_EXPECT_EQ(test, damon_nr_regions(t), 2);
r2 = damon_next_region(r);
KUNIT_EXPECT_EQ(test, r2->ar.start, DAMON_MIN_REGION * 2);
KUNIT_EXPECT_EQ(test, r2->ar.end, DAMON_MIN_REGION * 4);
damon_destroy_region(r2, t);
...
}
If the region split fails, damon_next_region() returns an invalid pointer
derived from the list head. Dereferencing it reads out-of-bounds, and
passing it to damon_destroy_region() causes a SLUB BUG.
[Severity: High]
This is a pre-existing issue, but does damon_test_set_regions() have an
out-of-bounds read on the expects[] array if the expectation fails?
mm/damon/tests/core-kunit.h:damon_test_set_regions() {
...
unsigned long expects[] = {8, 16, 16, 24, 24, 28};
...
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++]);
}
...
}
If there are more than 3 regions, the loop increments expect_idx past the
bounds of the stack-allocated expects[] array.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260909063159.63420-1-sj@kernel.org?part=1
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-09-09 6:42 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <2026090808-phoniness-repaying-46f0@gregkh>
2026-09-09 3:38 ` [PATCH 6.18.y] mm/damon/tests/core-kunit: catch test failure in test_merge_regions_of() SJ Park
2026-09-09 3:45 ` sashiko-bot
2026-09-09 6:26 ` SJ Park
2026-09-09 6:28 ` SJ Park
2026-09-09 6:31 ` SJ Park
2026-09-09 6:42 ` sashiko-bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox