Re: [PATCH 2/5] mm/damon/tests/core-kunit: catch test failure in test_merge_regions_of()

On Tue, 14 Jul 2026 14:01:45 +0000 [email protected] wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] This problem wasn't introduced by this patch, but the patch attempts to fix a crash where unit tests continue execution despite a failed KUNIT_EXPECT_EQ() condition and misses several identical vulnerable patterns in the same file. In multiple other test functions, if a tested function fails and the KUNIT expectation is not met, the tests will continue and perform out-of-bounds array accesses or invalid list traversals leading to slab corruption.
> --
> 
> --- Patch [2]: [PATCH 2/5] mm/damon/tests/core-kunit: catch test failure in test_merge_regions_of() ---
> commit 951f8b5681b80f05d0ef2d36767a3d72e546ac33
> Author: SJ Park <[email protected]>
> 
> mm/damon/tests/core-kunit: catch test failure in test_merge_regions_of()
> 
> This commit addresses a panic in damon_test_merge_regions_of(). If the initial
> KUNIT_EXPECT_EQ() expectation is not met, the code previously continued executing
> a per-region test assuming the incorrect hard-coded number of regions, leading
> to a NULL pointer dereference. The fix catches the expectation failure and
> skips the per-region tests.
> 
> Link: https://lore.kernel.org/[email protected]
> 
> > diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h
> > index 485472ddebd19..eba643762132f 100644
> > --- a/mm/damon/tests/core-kunit.h
> > +++ b/mm/damon/tests/core-kunit.h
> > @@ -260,11 +260,14 @@ static void damon_test_merge_regions_of(struct kunit *test)
> >  	damon_merge_regions_of(t, 9, 9999, ctx, true);
> >  	/* 0-112, 114-130, 130-156, 156-170, 170-230, 230-10170 */
> >  	KUNIT_EXPECT_EQ(test, damon_nr_regions(t), 6u);
> > +	if (damon_nr_regions(t) != 6)
> > +		goto out;
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but while addressing the crash
> here, could we also fix identical pre-existing regressions in the rest of this
> file?
> 
> In several other test functions, if a tested function fails and the
> KUNIT_EXPECT_EQ() condition is not met, execution continues and causes
> out-of-bounds array accesses or invalid pointer dereferences.
> 
> For example, in damos_test_commit_dests_for():
> 
> 	KUNIT_EXPECT_EQ(test, dst.nr_dests, src_nr_dests);
> 	for (i = 0; i < dst.nr_dests; i++) {
> 		KUNIT_EXPECT_EQ(test, dst.node_id_arr[i], src_node_id_arr[i]);
> 
> If dst.nr_dests is larger than src_nr_dests, doesn't this loop read out
> of bounds on src_node_id_arr[]?
> 
> Similarly, in damon_test_split_at() and 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, 2);
> 	KUNIT_EXPECT_EQ(test, r2->ar.end, 4);
> 	damon_destroy_region(r2, t);
> 
> If the split fails and there is only 1 region, does damon_next_region() return
> an invalid pointer for the list head? If so, passing it to
> damon_destroy_region() could lead to slab corruption.
> 
> A similar pattern also exists in damon_test_commit_target_regions_for() and
> damos_test_commit_quota_goals_for(). Should similar expectation checks and
> early exits be applied to these functions to prevent kernel panics during test
> failures?

Same class of issues were reported before, and I'm separately working on this.

> 
> >  	for (i = 0; i < 6; 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);
> >  	damon_destroy_ctx(ctx);
> >  }
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2


Thanks,
SJ