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

[sashiko-bot@kernel.org]

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?

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