Re: [PATCH 07/19] mm/damon/core: use abs_diff() instead of abs()

[SJ Park <sj@kernel.org>]

On Fri, 10 Jul 2026 14:10:54 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [Critical] Unchecked memory allocation failure in `damon_apply_min_nr_regions()` leads to heap corruption and infinite loops
> --
> 
> --- Patch [7]: [PATCH 07/19] mm/damon/core: use abs_diff() instead of abs() ---
> commit 168e2738a1f4077551ffee47ed4ecb0a582831f3
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/core: use abs_diff() instead of abs()
> 
> Replaces abs() with abs_diff() in damon_merge_regions_of() to prevent
> silent integer overflow when casting unsigned int to signed int.
> 
> Link: https://lore.kernel.org/20260705213817.100841-1-sj@kernel.org/
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index fea97399b5754..4c7e8a12a9b67 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> [ ... ]
> > @@ -3269,7 +3269,7 @@ static void damon_merge_regions_of(struct damon_target *t, unsigned int thres,
> >  			r->age++;
> >  
> >  		if (prev && prev->ar.end == r->ar.start &&
> > -		    abs(prev->nr_accesses - r->nr_accesses) <= thres &&
> > +		    abs_diff(prev->nr_accesses, r->nr_accesses) <= thres &&
> >  		    damon_sz_region(prev) + damon_sz_region(r) <= sz_limit)
> >  			damon_merge_two_regions(t, prev, r);
> >  		else
> 
> [Severity: Critical]
> This is a pre-existing issue and was not introduced by the patch under review,
> but I noticed a potential memory corruption issue in
> damon_apply_min_nr_regions() when a memory allocation fails.
> 
> If damon_split_region_at() fails to allocate memory, it aborts silently and
> leaves r->ar.end unchanged.
> 
> The loop then unconditionally advances r:
> 
> mm/damon/core.c:damon_apply_min_nr_regions() {
> 	damon_for_each_target(t, ctx) {
> 		damon_for_each_region_safe(r, next, t) {
> 			while (damon_sz_region(r) > max_region_sz) {
> 				damon_split_region_at(t, r, max_region_sz);
> 				r = damon_next_region(r);
> 			}
> 		}
> 	}
> }
> 
> Can this corrupt memory or cause infinite loops if the allocation fails on
> the last region in the list?
> 
> If this occurs on the last region in the target's list,
> r = damon_next_region(r) resolves to the list head (&t->regions_list).
> 
> Because the list head is embedded inside struct damon_target, container_of()
> returns a bogus struct damon_region pointer pointing before the damon_target
> object.
> 
> The next iteration of the while loop would read r->ar.start and r->ar.end
> from this invalid memory. If the computed difference exceeds max_region_sz,
> it enters the loop again. A successful damon_split_region_at() could then
> write the new region's start address to r->ar.end, corrupting memory before
> the damon_target structure.

Good finding.  Not a blocker of this patch, though.  I will separately work on
this.

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


Thanks,
SJ