Re: [PATCH 6.1.y v2 2/2] mm/damon/core: fix unconditionally skip last region

[SJ Park <sj@kernel.org>]

On Fri, 09 Oct 2026 15:04:56 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] mm/damon: Out-of-bounds access due to unchecked split failure on last region
> --
> 
> commit aacf3c387b1624c2944e2e5044dad18623db3d5f
> Author: Liew Rui Yan <aethernet65535@gmail.com>
> 
> mm/damon/core: fix unconditionally skip last region
> 
> This commit modifies the quota mechanism in DAMON to only reset the charge
> target and address from when the last region is reached. It ensures the last
> region is not skipped if it has not been processed.
> 
> Link: https://lore.kernel.org/20260908134739.96919-1-sj@kernel.org
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index c65d2cbeea6b5..f2f6f125c09b1 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> [ ... ]
> > @@ -768,14 +769,11 @@ static void damon_do_apply_schemes(struct damon_ctx *c,
> >  		if (quota->charge_target_from) {
> >  			if (t != quota->charge_target_from)
> >  				continue;
> > -			if (r == damon_last_region(t)) {
> > -				quota->charge_target_from = NULL;
> > -				quota->charge_addr_from = 0;
> > -				continue;
> > -			}
> >  			if (quota->charge_addr_from &&
> > -					r->ar.end <= quota->charge_addr_from)
> > -				continue;
> > +					r->ar.end <= quota->charge_addr_from) {
> > +				skip = true;
> > +				goto clean_charge_from_skip;
> > +			}
> >  
> >  			if (quota->charge_addr_from && r->ar.start <
> >  					quota->charge_addr_from) {
> > @@ -783,17 +781,25 @@ static void damon_do_apply_schemes(struct damon_ctx *c,
> >  						r->ar.start, DAMON_MIN_REGION);
> >  				if (!sz) {
> >  					if (damon_sz_region(r) <=
> > -					    DAMON_MIN_REGION)
> > -						continue;
> > +					    DAMON_MIN_REGION) {
> > +						skip = true;
> > +						goto clean_charge_from_skip;
> > +					}
> >  					sz = DAMON_MIN_REGION;
> >  				}
> >  				damon_split_region_at(t, r, sz);
> >  				r = damon_next_region(r);
> >  				sz = damon_sz_region(r);
> 
> [Severity: High]
> Since this commit removes the check that bypassed damon_split_region_at() for
> the target's last region, does this introduce an out-of-bounds access if
> memory allocation fails?
> 
> If the last region overlaps the charge boundary, it is now passed to
> damon_split_region_at(). This function performs a non-blocking memory
> allocation that can fail under memory pressure and return silently without
> splitting the region:
> 
> mm/damon/core.c:damon_split_region_at() {
>     new = damon_new_region(r->ar.start + sz_r, r->ar.end);
>     if (!new)
>         return;
>     ...
> }
> 
> If the allocation fails when processing the last region, the region is not
> split. The code then unconditionally executes:
> 
>     r = damon_next_region(r);
>     sz = damon_sz_region(r);
> 
> Because r was the last region and wasn't split, damon_next_region() reads
> the list.next pointer which points back to the list head:
> 
> include/linux/damon.h:damon_next_region() {
>     return container_of(r->list.next, struct damon_region, list);
> }
> 
> This produces an out-of-bounds pointer derived from the list head. When this
> pointer is immediately dereferenced by damon_sz_region(r), could this
> regression result in out-of-bounds memory reads?

Good finding.  Patch 1 should avoided this issue by removing the safe iterator
workaround, but I mistakenly didn't remove the workaround.  I will fix it in
the next spin.

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


Thanks,
SJ