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] Out-of-bounds array read in damon_test_commit_target_regions_for() due to missing region count validation before traversing dst_target.
--

--- Patch [6]: [PATCH 6/7] mm/damon/core-kunit: skip wrong quota goal walk in commit_quota_goals() ---
commit 4bcb5123bbb5f6b4908f1d776a8608ef05a89501
Author: SJ Park <[email protected]>
Subject: mm/damon/core-kunit: skip wrong quota goal walk in commit_quota_goals()

This commit addresses an out-of-bounds array access in the KUnit test
damos_test_commit_quota_goals_for(). If the number of created quota goals
does not match the expected count, blindly traversing them can cause the test
to read past the bounds of the expected values array. The patch adds a count
validation and an early exit to avoid this invalid traversal.

Link: https://lore.kernel.org/[email protected]

> diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h
> index 9b8c20a50ad53..fbf986aee2dea 100644
> --- 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 isn't a bug introduced by this patch, but should a similar early exit
check be added to damon_test_commit_target_regions_for() to prevent an
out-of-bounds read?

Just like the issue fixed here, damon_test_commit_target_regions_for()
iterates over dst_target regions without validating the region count against
nr_expect_regions first:

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++;
	}
	KUNIT_EXPECT_EQ(test, damon_nr_regions(dst_target), nr_expect_regions);
    ...
}

If damon_commit_target_regions() incorrectly leaves more regions in dst_target
than nr_expect_regions, the loop will read past the end of the
expect_start_end[] array before reaching the KUNIT_EXPECT_EQ count validation
at the end of the function.

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