[PATCH 6.12.y] mm/damon/core-kunit: handle region split failure in filter_out()

[PATCH 6.12.y] mm/damon/core-kunit: handle region split failure in filter_out()

[SJ Park]

damos_test_filter_out() test checks if damos_filter_match() of an address
filter splits the region as expected under a given condition.  But, the
test continued regardless of the split successes.  As a result, the later
part of the test could dereference invalid pointers that returned from
damon_next_region().  Further, it could corrupt memory from
damon_destroy_region().

The consequent user impact (memory corruption) is quite bad.  The
realistic user impact would be limited, though.  It would affect only test
run setups.

Fix it by exiting early for the number of regions test failure.

The issue was discovered [1] by Sashiko.

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

diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h
index d7737f5df6688..a9d852e666657 100644
--- a/mm/damon/tests/core-kunit.h
+++ b/mm/damon/tests/core-kunit.h
@@ -552,6 +552,8 @@ static void damos_test_filter_out(struct kunit *test)
 	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);
+	if (damon_nr_regions(t) != 2)
+		goto out;
 	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);
@@ -565,11 +567,14 @@ static void damos_test_filter_out(struct kunit *test)
 	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);
+	if (damon_nr_regions(t) != 2)
+		goto out;
 	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);
 
+out:
 	damon_free_target(t);
 	damos_free_filter(f);
 }
-- 
2.47.3

Re: [PATCH 6.12.y] mm/damon/core-kunit: handle region split failure in filter_out()

[sashiko-bot]

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

Pre-existing issues:
- [High] Missing NULL pointer checks for memory allocations in `damos_test_filter_out()`.
- [High] Invalid pointer dereference in `damon_test_split_at()` upon allocation failure.
- [High] NULL pointer dereference in `damon_test_merge_regions_of()` on test failure.
--

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

mm/damon/core-kunit: handle region split failure in filter_out()
    
damos_test_filter_out() test checks if damos_filter_match() of an address
filter splits the region as expected under a given condition.  But, the
test continued regardless of the split successes.  As a result, the later
part of the test could dereference invalid pointers that returned from
damon_next_region().  Further, it could corrupt memory from
damon_destroy_region().
    
The consequent user impact (memory corruption) is quite bad.  The
realistic user impact would be limited, though.  It would affect only test
run setups.
    
Fix it by exiting early for the number of regions test failure.
    
The issue was discovered [1] by Sashiko.
    
Link: https://lore.kernel.org/20260718001442.87129-5-sj@kernel.org
Link: https://lore.kernel.org/20260714142352.100478-1-sj@kernel.org [1]

> diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h
> index d7737f5df6688..a9d852e666657 100644
> --- a/mm/damon/tests/core-kunit.h
> +++ b/mm/damon/tests/core-kunit.h
> @@ -552,6 +552,8 @@ static void damos_test_filter_out(struct kunit *test)
>  	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);
> +	if (damon_nr_regions(t) != 2)
> +		goto out;
>  	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);
> @@ -565,11 +567,14 @@ static void damos_test_filter_out(struct kunit *test)
>  	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);
> +	if (damon_nr_regions(t) != 2)
> +		goto out;
>  	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);
>  
> +out:
>  	damon_free_target(t);
>  	damos_free_filter(f);
>  }

[Severity: High]
This isn't a bug introduced by this patch, but does this test miss NULL
pointer checks for memory allocations earlier in the same function?

If memory allocation fails (e.g., during KUnit OOM simulation),
damos_new_filter(), damon_new_target(), or damon_new_region() will
return NULL. The returned pointers f, t, and r are immediately
dereferenced without any verification:

mm/damon/tests/core-kunit.h:damos_test_filter_out() {
	f = damos_new_filter(DAMOS_FILTER_TYPE_ADDR, true);
	f->addr_range = (struct damon_addr_range){
		.start = DAMON_MIN_REGION * 2, .end = DAMON_MIN_REGION * 6};

	t = damon_new_target();
	r = damon_new_region(DAMON_MIN_REGION * 3, DAMON_MIN_REGION * 5);
	damon_add_region(r, t);
    ...
}

While this only impacts test environments, KUnit tests should not trigger
kernel panics on allocation failures. This is a standard already followed
by other tests in the same file which safely use
kunit_skip(test, "alloc fail");


[Severity: High]
This is a pre-existing issue, but could this code dereference an invalid
pointer upon allocation failure?

damon_split_region_at() silently returns without splitting if
damon_new_region() fails to allocate memory. In damon_test_split_at(), if
the split fails, r remains the last region. damon_next_region(r) will then
use container_of() on the list head, returning an invalid pointer outside
the bounds of struct damon_target. Dereferencing this pointer reads
memory out of bounds:

mm/damon/tests/core-kunit.h:damon_test_split_at() {
    ...
	damon_split_region_at(t, r, 25);
	KUNIT_EXPECT_EQ(test, r->ar.start, 0ul);
	KUNIT_EXPECT_EQ(test, r->ar.end, 25ul);

	r_new = damon_next_region(r);
	KUNIT_EXPECT_EQ(test, r_new->ar.start, 25ul);
    ...
}

This requires an allocation failure during test execution, which is the exact
same class of vulnerability that this patch addresses for
damos_test_filter_out().


[Severity: High]
This is a pre-existing issue, but could this test trigger a NULL pointer
dereference on failure?

The test expects 5 regions to remain after a merge operation. It verifies
this with KUNIT_EXPECT_EQ(test, damon_nr_regions(t), 5u);. However,
KUNIT_EXPECT_EQ does not abort test execution if the expectation fails. If
the merge operation results in fewer than 5 regions, the subsequent loop
will call __nth_region_of(t, i) with an out-of-bounds index, returning a
NULL pointer. The NULL pointer is then unconditionally dereferenced by
r->ar.start:

mm/damon/tests/core-kunit.h:damon_test_merge_regions_of() {
    ...
	damon_merge_regions_of(t, 9, 9999);
	/* 0-112, 114-130, 130-156, 156-170 */
	KUNIT_EXPECT_EQ(test, damon_nr_regions(t), 5u);
	for (i = 0; i < 5; i++) {
		r = __nth_region_of(t, i);
		KUNIT_EXPECT_EQ(test, r->ar.start, saddrs[i]);
    ...
}

Should KUNIT_ASSERT_EQ be used instead of KUNIT_EXPECT_EQ to safely abort
the test and prevent the subsequent NULL pointer dereference?

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

Re: [PATCH 6.12.y] mm/damon/core-kunit: handle region split failure in filter_out()

[Sasha Levin]

> damos_test_filter_out() test checks if damos_filter_match() of an address
> filter splits the region as expected under a given condition.  But, the
> test continued regardless of the split successes.

Queued for 6.12, thanks.

-- 
Thanks,
Sasha