Re: [PATCH 4/7] mm/damon/core-kunit: handle region split failure in filter_out()

On Fri, 17 Jul 2026 00:46:05 +0000 [email protected] wrote:

> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] The patch adds an early exit check `if (damon_nr_regions(t) != 2) goto out;` to prevent an invalid pointer dereference when the first region split test fails, but misses the exact same vulnerability in the second region split test directly below it.
> - [High] In `damon_apply_min_nr_regions`, if `damon_split_region_at` fails to allocate memory, the inner loop advances to the next list entry without inserting a new region, leading to an out-of-bounds read and write when it reaches the list head.
> --
> 
> --- Patch [4]: [PATCH 4/7] mm/damon/core-kunit: handle region split failure in filter_out() ---
> commit 705cf12a4e8501ec816031495df35460614c42c2
> Author: SJ Park <[email protected]>
> 
> mm/damon/core-kunit: handle region split failure in filter_out()
>     
> damos_test_filter_out() checks if damos_filter_match() of an address
> filter splits the region as expected. But the test continued regardless of
> whether the split succeeded, which could lead to dereferencing invalid
> pointers or corrupting memory. This adds an early exit check on the first
> test block to prevent continuing on failure.
> 
> Link: https://lore.kernel.org/[email protected]
> 
> > diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h
> > index 322a295ac2cec..bd3bbd421392f 100644
> > --- a/mm/damon/tests/core-kunit.h
> > +++ b/mm/damon/tests/core-kunit.h
> > @@ -1365,6 +1365,8 @@ static void damos_test_filter_out(struct kunit *test)
> >  	KUNIT_EXPECT_EQ(test, r->ar.start, 1);
> >  	KUNIT_EXPECT_EQ(test, r->ar.end, 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, 2);
> >  	KUNIT_EXPECT_EQ(test, r2->ar.end, 4);
> > @@ -1384,6 +1386,7 @@ static void damos_test_filter_out(struct kunit *test)
> >  	KUNIT_EXPECT_EQ(test, r2->ar.end, 8);
> >  	damon_destroy_region(r2, t);
> >  
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but does this second region split
> test have the same vulnerability as the one fixed above?
> 
> If damos_filter_match() fails to split the region in this second test block
> (e.g. due to memory allocation failure), damon_nr_regions(t) would remain 1.
> r2 would be assigned damon_next_region(r), which could incorrectly resolve
> to the damon_target list head.
> 
> Calling damon_destroy_region(r2, t) would then call kmem_cache_free() on
> this out-of-bounds pointer, potentially corrupting memory. Should another
> early exit check be added here as well?

Good finding.  I will fix this in the next revision of this series.

> 
> > +out:
> >  	damon_free_target(t);
> >  	damos_free_filter(f);
> >  }
> 
> [Severity: High]
> This is a pre-existing issue, but while looking at region splits, could a
> similar unchecked failure cause memory corruption in mm/damon/core.c?
> 
> In damon_apply_min_nr_regions():
> 
> 			while (damon_sz_region(r) > max_region_sz) {
> 				damon_split_region_at(t, r, max_region_sz);
> 				r = damon_next_region(r);
> 			}
> 
> If damon_split_region_at() fails due to memory pressure, it does not split
> the region. The call to damon_next_region(r) then erroneously advances r to
> the next element.
> 
> If r was the last region, it becomes the list head (&t->regions_list). The
> loop condition then evaluates damon_sz_region(r) on the list head, which
> reads memory 32 bytes before the damon_target struct allocation. If this
> difference exceeds max_region_sz, damon_split_region_at() could be called
> on the list head, which might subsequently write to r->ar.end, corrupting
> SLUB metadata or preceding objects.
> 
> Should damon_split_region_at() return an error code so callers can verify
> if the split actually succeeded?

Sashiko found this in a previous reply.  I'm gonna separately work on this.

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


Thanks,
SJ