* [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