Re: [PATCH 6/7] mm/damon/core-kunit: skip wrong quota goal walk in commit_quota_goals()

On Fri, 17 Jul 2026 00:44:33 +0000 [email protected] wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [Medium] Missing error check in `damon_test_commit_target_regions_for()` leads to an out-of-bounds read if `damon_commit_target_regions()` fails.
> --
> 
> --- Patch [6]: [PATCH 6/7] mm/damon/core-kunit: skip wrong quota goal walk in commit_quota_goals() ---
> commit 9bf8891739c4d265498b06b272b3083da8b65ed2
> Author: SJ Park <[email protected]>
> 
> mm/damon/core-kunit: skip wrong quota goal walk in commit_quota_goals()
> 
> This commit fixes an out-of-bounds access in
> damos_test_commit_quota_goals_for() by verifying the number of quota goals
> matches the expected amount before traversing them. This prevents test setups
> from crashing if damos_commit_quota_goals() creates an unexpected number of
> goals.
> 
> Link: https://lore.kernel.org/[email protected]
> 
> > --- a/mm/damon/tests/core-kunit.h
> > +++ b/mm/damon/tests/core-kunit.h
> [ ... ]
> > @@ -861,6 +862,14 @@ static void damos_test_commit_quota_goals_for(struct kunit *test,
> >  
> >  	damos_commit_quota_goals(&dst, &src);
> >  
> > +	damos_for_each_quota_goal(goal, &dst)
> > +		nr_dst++;
> > +	damos_for_each_quota_goal(goal, &src)
> > +		nr_src++;
> > +	KUNIT_EXPECT_EQ(test, nr_dst, nr_src);
> > +	if (nr_dst != nr_src)
> > +		goto out;
> > +
> >  	i = 0;
> >  	damos_for_each_quota_goal(goal, (&dst)) {
> >  		KUNIT_EXPECT_EQ(test, goal->metric, src_goals[i].metric);
> 
> [Severity: Medium]
> This is a pre-existing issue, but since this patch fixes this exact pattern 
> for quota goals, does damon_test_commit_target_regions_for() need a similar 
> fix for target regions?
> 
> If damon_commit_target_regions() fails (for example, due to a memory 
> allocation failure), it returns an error code and leaves dst_target's 
> regions unmodified. The test code currently ignores the return value and 
> blindly iterates over dst_target:
> 
> mm/damon/tests/core-kunit.h: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++;
> 	}
> ...
> }
> 
> In a case where the original number of destination regions is larger than 
> the number of expected regions, could this loop execute more times than the 
> size of expect_start_end[] and cause an out-of-bounds read on the stack array?

Yes, and that is fixed by another patch in this series.

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


Thanks,
SJ