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

[sashiko-bot@kernel.org]

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.

>  		ranges[i].start = sys_region->ar.start;
>  		ranges[i].end = sys_region->ar.end;
>  		if (i == 0)

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