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

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?

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