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

[SJ Park <sj@kernel.org>]

On Thu, 02 Jul 2026 21:15:39 +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 negative-sized regions to bypass subsequent validation, resulting in overlapping regions and eventually negative-sized regions in the DAMON core, leading to unbounded memory allocation (OOM).
> --
> 
> commit d23d0a5af36c320aac75064c21548cc9de76c4f1
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/sysfs: remove region size validation
> 
> DAMON_SYSFS validates user inputs for monitoring target regions to
> disallow negative size regions.  DAMON core assumes only positive size
> regions, though.  The validation is incomplete.  Fortunately
> damon_set_regions(), which is eventually used by DAMON_SYSFS, does the
> complete validation.  Remove the incomplete and unnecessary 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]
> Does removing this validation allow negative-sized regions to bypass the
> subsequent overlap checks in damon_sysfs_set_regions()?
> 
> If we have an inverted region (e.g., start=2000, end=1000) followed by another
> region (e.g., start=1500, end=3000), the overlap check in
> damon_sysfs_set_regions() evaluates:
> 
> 	if (ranges[i - 1].end > ranges[i].start)
> 
> Which results in ranges[0].end (1000) > ranges[1].start (1500).
> 
> This is false, meaning the inverted region bypasses the overlap check.
> 
> Will this inverted region also bypass the alignment-based bounds check in
> mm/damon/core.c:damon_set_regions()?
> 
> mm/damon/core.c:damon_set_regions() {
> 	...
> 	for (i = 0; i < nr_ranges; i++) {
> 		if (ALIGN_DOWN(ranges[i].start, min_region_sz) >=
> 				ALIGN(ranges[i].end, min_region_sz))
> 			return -EINVAL;
> 	}
> 
> If the regions fall within the same alignment block (e.g., start=2000 and
> end=1000 with min_region_sz=4096 align to 0 and 4096), the check evaluates
> 0 >= 4096, which is false, passing the invalid input to the core.
> 
> When the negative-sized region reaches mm/damon/core.c:damon_apply_min_nr_regions(),
> can this cause an out-of-memory loop?
> 
> 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);
> 			}
> 
> The inverted region size would underflow to a very large number, which appears
> to cause this loop to continuously split the region, allocating region structs
> until the system runs out of memory.

We are working on a fix [1] of this in a separate thread.

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


Thanks,
SJ

[...]