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

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

[SJ Park]

damon_test_split_at() test next region that is assumed to be created by
damon_split_region_at() invocation.  But the split might fail.  In this
case, the succeeding test may dereference invalid pointers returned by
damon_next_region().

The invalid pointer may not cause a really bad user impact, because of the
implementation detail.  It would only read wrong contents in the belonging
damon_target struct.  Depending on the future change of the offset from
the link header to the accessing field, this could also be really
dangerous, though.  Still, the realistic user impact would be limited.  It
would affect only test run setups.

Fix it by testing if the number of regions was also changed as expected
and exit early for the failure.

The issue was discovered [1] by Sashiko.

Link: https://lore.kernel.org/20260718001442.87129-3-sj@kernel.org
Link: https://lore.kernel.org/20260714142352.100478-1-sj@kernel.org [1]
Fixes: 17ccae8bb5c9 ("mm/damon: add kunit tests")
Signed-off-by: SJ Park <sj@kernel.org>
Cc: Brendan Higgins <brendan.higgins@linux.dev>
Cc: <stable@vger.kernel.org> # 5.15.x
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
(cherry picked from commit 9b18ef3c3aa1ce24176e340061caf15fa2742564)
Signed-off-by: SJ Park <sj@kernel.org>
---
 mm/damon/core-test.h | 5 +++++
 1 file changed, 5 insertions(+)

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;
+
 	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);
 }
-- 
2.47.3

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

[sashiko-bot]

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

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

[Sasha Levin]

> damon_test_split_at() test next region that is assumed to be created by
> damon_split_region_at() invocation.  But the split might fail.  In this
> case, the succeeding test may dereference invalid pointers returned by
> damon_next_region().

Queued for 6.6, thanks.

-- 
Thanks,
Sasha