Re: [RFC PATCH v1.1 3/7] mm/damon/vaddr-kunit: check region count in three_regions test

On Fri, 17 Jul 2026 14:50:49 +0000 [email protected] wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] Out-of-bounds array read in KUnit test helpers due to missing array bounds check and ignored return values
> --
> 
> --- Patch [3]: [RFC PATCH v1.1 3/7] mm/damon/vaddr-kunit: check region count in three_regions test ---
> commit b2d38d564a3004fa6df02a9f1d54af3423302437
> Author: SJ Park <[email protected]>
> 
> mm/damon/vaddr-kunit: check region count in three_regions test
> 
> This commit adds a check in damon_do_test_apply_three_regions() to ensure
> damon_set_regions() succeeded in setting the expected number of regions.
> This prevents a potential NULL pointer dereference during test iteration.
> 
> Link: https://lore.kernel.org/[email protected]
> 
> > diff --git a/mm/damon/tests/vaddr-kunit.h b/mm/damon/tests/vaddr-kunit.h
> > index 61f844336ffb5..6a95441d193ae 100644
> > --- a/mm/damon/tests/vaddr-kunit.h
> > +++ b/mm/damon/tests/vaddr-kunit.h
> 
> [ ... ]
> 
> [Severity: High]
> This is a pre-existing issue, but while reviewing this test fix, I noticed
> similar missing bounds checks and unhandled return values in
> mm/damon/tests/core-kunit.h that can cause out-of-bounds array reads.
> 
> In damon_test_commit_target_regions_for():
> 
> 	damon_commit_target_regions(dst_target, src_target, 1);
> 	i = 0;
> 	damon_for_each_region(r, dst_target) {
> 		KUNIT_EXPECT_EQ(test, r->ar.start, expect_start_end[i][0]);
> 		KUNIT_EXPECT_EQ(test, r->ar.end, expect_start_end[i][1]);
> 		i++;
> 	}
> 
> If memory allocation fails inside damon_commit_target_regions(), it returns
> an error without modifying dst_target. Because the return value is ignored
> here, could this iterate over the original regions of dst_target and read
> past the end of the expect_start_end stack array?
> 
> A similar pattern appears in damos_test_commit_quota_goals_for():
> 
> 	damos_commit_quota_goals(&dst, &src);
> 
> 	i = 0;
> 	damos_for_each_quota_goal(goal, (&dst)) {
> 		KUNIT_EXPECT_EQ(test, goal->metric, src_goals[i].metric);
> 		KUNIT_EXPECT_EQ(test, goal->target_value,
> 				src_goals[i++].target_value);
> 	}
> 
> Does ignoring the return value of damos_commit_quota_goals() risk an
> out-of-bounds read on src_goals[] if the destination goals are not updated
> and retain more items than expected?
> 
> Finally, 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]);
> 		KUNIT_EXPECT_EQ(test, dst.weight_arr[i], src_weight_arr[i]);
> 	}
> 
> Since KUNIT_EXPECT_EQ does not abort test execution on failure, could an
> out-of-bounds read occur on src_node_id_arr[] and src_weight_arr[] if
> dst.nr_dests ends up larger than src_nr_dests?

These issues are fixed by other patches in this series.

> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3


Thanks,
SJ