Re: [PATCH 6.6.y] mm/damon/core-kunit: check region count before testing in split_at()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Medium] The patch addresses an invalid pointer dereference in `damon_test_split_at()` by checking `damon_nr_regions(t) != 2` after a split failure, but misses the exact same vulnerability in `damos_test_filter_out()`, which will cause a kernel panic if a split fails.
--

commit eea94ba78cece48ae60d9049e4528675f25050ea
Author: SJ Park <sj@kernel.org>

mm/damon/core-kunit: check region count before testing in split_at()

The commit addresses an issue in damon_test_split_at() where a failure
during region splitting would cause the subsequent code to dereference an
invalid pointer. By checking if the region count is 2 after the split,
the test now correctly exits early on failure. This fixes a potential
memory corruption issue during test execution.

Link: https://lore.kernel.org/20260718001442.87129-3-sj@kernel.org
Link: https://lore.kernel.org/20260714142352.100478-1-sj@kernel.org [1]

> diff --git a/mm/damon/core-test.h b/mm/damon/core-test.h
> index a2aa410d6d0d0..f2f0978495086 100644
> --- a/mm/damon/core-test.h
> +++ b/mm/damon/core-test.h
> @@ -162,6 +162,10 @@ static void damon_test_split_at(struct kunit *test)
>  	}
>  	damon_add_region(r, t);
>  	damon_split_region_at(t, r, 25);
> +	KUNIT_EXPECT_EQ(test, damon_nr_regions(t), 2);
> +	if (damon_nr_regions(t) != 2)
> +		goto out;
> +

[Severity: Medium]
This is a pre-existing issue, but does damos_test_filter_out() in the same
file suffer from the exact same missing failure check?

If memory allocation fails during region splitting in __damos_filter_out(),
damon_nr_regions(t) remains 1. The test code expects 2, but KUNIT_EXPECT_EQ()
does not abort execution:

	/* filter should have split the region */
	KUNIT_EXPECT_EQ(test, r->ar.start, DAMON_MIN_REGION * 1);
	KUNIT_EXPECT_EQ(test, r->ar.end, DAMON_MIN_REGION * 2);
	KUNIT_EXPECT_EQ(test, damon_nr_regions(t), 2);
	r2 = damon_next_region(r);
	KUNIT_EXPECT_EQ(test, r2->ar.start, DAMON_MIN_REGION * 2);
	KUNIT_EXPECT_EQ(test, r2->ar.end, DAMON_MIN_REGION * 4);
	damon_destroy_region(r2, t);

Could passing the invalid pointer returned by damon_next_region() to
damon_destroy_region() lead to a crash when it executes list_del() and
kmem_cache_free() on the corrupted pointer?

A similar pattern also occurs slightly later in the same function:

	/* filter should have split the region */
	KUNIT_EXPECT_EQ(test, r->ar.start, DAMON_MIN_REGION * 2);
	KUNIT_EXPECT_EQ(test, r->ar.end, DAMON_MIN_REGION * 6);
	KUNIT_EXPECT_EQ(test, damon_nr_regions(t), 2);
	r2 = damon_next_region(r);
	KUNIT_EXPECT_EQ(test, r2->ar.start, DAMON_MIN_REGION * 6);
	KUNIT_EXPECT_EQ(test, r2->ar.end, DAMON_MIN_REGION * 8);
	damon_destroy_region(r2, t);

Should similar early exits be added in damos_test_filter_out() to prevent
this?

>  	KUNIT_EXPECT_EQ(test, r->ar.start, 0ul);
>  	KUNIT_EXPECT_EQ(test, r->ar.end, 25ul);
>  
> @@ -169,6 +173,7 @@ static void damon_test_split_at(struct kunit *test)
>  	KUNIT_EXPECT_EQ(test, r->ar.start, 25ul);
>  	KUNIT_EXPECT_EQ(test, r->ar.end, 100ul);
>  
> +out:
>  	damon_free_target(t);
>  	damon_destroy_ctx(c);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260909040016.165245-1-sj@kernel.org?part=1