Re: [RFC PATCH 8/8] mm/damon/sysfs: remove region size validation

[SJ Park <sj@kernel.org>]

On Wed, 01 Jul 2026 16:34:54 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Removing the `start > end` validation allows users to configure monitoring regions with negative sizes, causing integer underflow in region size calculations. This leads to infinite memory allocation loops (OOM), potential list corruption, and CPU soft lockups during DAMON region splitting and scheme operations.
> --
> 
> commit 013d66b2108372ca2ada7fc6fcfd5e07f93fd78c
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/sysfs: remove region size validation
> 
> This commit removes the region size validation in DAMON sysfs that checks
> for negative size regions. The commit message states that the validation is
> incomplete and that damon_set_regions() already performs complete
> validation.
> 
> > diff --git a/mm/damon/sysfs.c b/mm/damon/sysfs.c
> > index e666dddf1feba..b65651498e0d1 100644
> > --- a/mm/damon/sysfs.c
> > +++ b/mm/damon/sysfs.c
> > @@ -1987,9 +1987,6 @@ static int damon_sysfs_set_regions(struct damon_target *t,
> >  		struct damon_sysfs_region *sys_region >  			sysfs_regions->regions_arr[i];
> >  
> > -		if (sys_region->ar.start > sys_region->ar.end)
> > -			goto out;
> > -
> 
> [Severity: High]
> The commit message mentions that damon_set_regions() does the complete
> validation, but looking at damon_set_regions() in mm/damon/core.c, does it
> actually validate that start <= end?
> 
> mm/damon/core.c:damon_set_regions() {
> 	...
> 	r = damon_new_region(
> 		ALIGN_DOWN(ranges[i].start, min_region_sz),
> 		ALIGN(ranges[i].end, min_region_sz));
> 	...
> }
> 
> If a user configures a region where start > end, wouldn't this cause an
> integer underflow when calculating the region size later? 
> 
> For example, could this underflow lead to an infinite memory allocation loop
> in mm/damon/core.c: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);
> 	}
> 
> Could this loop evaluate the underflowed size and attempt to allocate new
> regions repeatedly until an OOM occurs?
> 
> Additionally, for newly split regions where start < end locally, could this
> spin over a massive address range without yielding and cause a CPU soft
> lockup in mm/damon/paddr.c:damon_pa_pageout():
> 
> 	while (addr < damon_pa_phys_addr(r->ar.end, addr_unit)) {
> 		...
> 	}

My previous reply [1] should again be applied here.

[1] https://lore.kernel.org/20260702001650.86841-1-sj@kernel.org


Thanks,
SJ

[...]