DAMON development mailing list
 help / color / mirror / Atom feed
* [PATCH 6.6.y] mm/damon/vaddr-kunit: check region count in three_regions test
       [not found] <2026090820-shadiness-speculate-ce42@gregkh>
@ 2026-09-09  3:49 ` SJ Park
  2026-09-09  4:00   ` sashiko-bot
  2026-09-09 20:26   ` Sasha Levin
  0 siblings, 2 replies; 3+ messages in thread
From: SJ Park @ 2026-09-09  3:49 UTC (permalink / raw)
  To: stable; +Cc: damon, SJ Park, Brendan Higgins, Andrew Morton

damon_do_test_apply_three_regions() iterates regions after
damon_set_regions() call assuming the function would succeed at setting
the number of regions the same to the expected one.  It might have failed.
In this case, __nth_region_of() in the iteration could return NULL and
NULL dereference can happen in the test.

The consequent user impact (NULL dereference) is quite bad.  The realistic
user impact would be limited, though.  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-4-sj@kernel.org
Link: https://lore.kernel.org/20260713144757.39740-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 5fea07e460874c8c7cf00f728efbe22abc62c8d8)
Signed-off-by: SJ Park <sj@kernel.org>
---
 mm/damon/vaddr-test.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/mm/damon/vaddr-test.h b/mm/damon/vaddr-test.h
index 292736d3f3954..c0546447b21e6 100644
--- a/mm/damon/vaddr-test.h
+++ b/mm/damon/vaddr-test.h
@@ -149,12 +149,17 @@ static void damon_do_test_apply_three_regions(struct kunit *test,
 
 	damon_set_regions(t, three_regions, 3);
 
+	KUNIT_EXPECT_EQ(test, damon_nr_regions(t), nr_expected / 2);
+	if (damon_nr_regions(t) != nr_expected / 2)
+		goto out;
+
 	for (i = 0; i < nr_expected / 2; i++) {
 		r = __nth_region_of(t, i);
 		KUNIT_EXPECT_EQ(test, r->ar.start, expected[i * 2]);
 		KUNIT_EXPECT_EQ(test, r->ar.end, expected[i * 2 + 1]);
 	}
 
+out:
 	damon_destroy_target(t);
 }
 
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH 6.6.y] mm/damon/vaddr-kunit: check region count in three_regions test
  2026-09-09  3:49 ` [PATCH 6.6.y] mm/damon/vaddr-kunit: check region count in three_regions test SJ Park
@ 2026-09-09  4:00   ` sashiko-bot
  2026-09-09 20:26   ` Sasha Levin
  1 sibling, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-09-09  4:00 UTC (permalink / raw)
  To: SJ Park; +Cc: damon

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

Pre-existing issues:
- [High] Missing early exit on region count mismatch can lead to a NULL pointer dereference in damon_test_merge_regions_of().
- [High] Missing early exit on region count mismatch can lead to out-of-bounds array read in damon_test_set_regions().
--

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

mm/damon/vaddr-kunit: check region count in three_regions test

damon_do_test_apply_three_regions() iterates regions after a 
damon_set_regions() call assuming the region count matches the expected 
number. If it fails, __nth_region_of() can return NULL, leading to a NULL 
dereference during the test. This commit fixes the issue by explicitly 
testing if the number of regions changed as expected and exiting early on 
failure.

Link: https://lore.kernel.org/20260718001442.87129-4-sj@kernel.org
Link: https://lore.kernel.org/20260713144757.39740-1-sj@kernel.org [1]

> diff --git a/mm/damon/vaddr-test.h b/mm/damon/vaddr-test.h
> index 292736d3f3954..c0546447b21e6 100644
> --- a/mm/damon/vaddr-test.h
> +++ b/mm/damon/vaddr-test.h
> @@ -149,12 +149,17 @@ static void damon_do_test_apply_three_regions(struct kunit *test,
>  
>  	damon_set_regions(t, three_regions, 3);
>  
> +	KUNIT_EXPECT_EQ(test, damon_nr_regions(t), nr_expected / 2);
> +	if (damon_nr_regions(t) != nr_expected / 2)
> +		goto out;
> +

[Severity: High]
This isn't a bug introduced by this patch, but since we're fixing missing 
early exits on region count mismatches here, there are similar vulnerabilities 
in mm/damon/core-test.h that might need the same treatment.

In mm/damon/core-test.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]);

Can this lead to a NULL pointer dereference? 

If damon_merge_regions_of() produces fewer than 5 regions due to a test 
failure, KUNIT_EXPECT_EQ logs the failure but continues execution. The loop 
attempts to fetch the 5th region using __nth_region_of(), which returns NULL. 
Dereferencing r->ar.start then causes a panic.

[Severity: High]
This is a pre-existing issue, but there is also a potential out-of-bounds 
read in mm/damon/core-test.h:damon_test_set_regions():

	unsigned long expects[] = {8, 16, 16, 24, 24, 28};
	int expect_idx = 0;
...
	damon_set_regions(t, &range, 1);

	KUNIT_EXPECT_EQ(test, damon_nr_regions(t), 3);
	damon_for_each_region(r, t) {
		KUNIT_EXPECT_EQ(test, r->ar.start, expects[expect_idx++]);
		KUNIT_EXPECT_EQ(test, r->ar.end, expects[expect_idx++]);
	}

Can this lead to an out-of-bounds array read? 

If damon_set_regions() produces more than 3 regions due to a bug in the 
tested logic, KUNIT_EXPECT_EQ logs a failure but continues. The loop 
damon_for_each_region(r, t) then iterates over all regions, causing expect_idx 
to exceed the bounds of the 6-element expects[] array when accessing 
expects[expect_idx++].

>  	for (i = 0; i < nr_expected / 2; i++) {
>  		r = __nth_region_of(t, i);
>  		KUNIT_EXPECT_EQ(test, r->ar.start, expected[i * 2]);
>  		KUNIT_EXPECT_EQ(test, r->ar.end, expected[i * 2 + 1]);
>  	}
>  
> +out:
>  	damon_destroy_target(t);
>  }

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH 6.6.y] mm/damon/vaddr-kunit: check region count in three_regions test
  2026-09-09  3:49 ` [PATCH 6.6.y] mm/damon/vaddr-kunit: check region count in three_regions test SJ Park
  2026-09-09  4:00   ` sashiko-bot
@ 2026-09-09 20:26   ` Sasha Levin
  1 sibling, 0 replies; 3+ messages in thread
From: Sasha Levin @ 2026-09-09 20:26 UTC (permalink / raw)
  To: stable; +Cc: Sasha Levin, damon, SJ Park, Brendan Higgins, Andrew Morton

> damon_do_test_apply_three_regions() iterates regions after
> damon_set_regions() call assuming the function would succeed at setting
> the number of regions the same to the expected one.  It might have failed.

Queued for 6.6, thanks.

-- 
Thanks,
Sasha

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-09-09 20:26 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <2026090820-shadiness-speculate-ce42@gregkh>
2026-09-09  3:49 ` [PATCH 6.6.y] mm/damon/vaddr-kunit: check region count in three_regions test SJ Park
2026-09-09  4:00   ` sashiko-bot
2026-09-09 20:26   ` Sasha Levin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox