Re: [PATCH 6.6.y v2] 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: list head type confusion and memory corruption on allocation failure
--

commit 47762d8f5b1f74489d2b1ca149de65b77ac867a2
Author: Liew Rui Yan <aethernet65535@gmail.com>

mm/damon/core: fix unconditionally skip last region

Once quota set, the charge_{target,addr}_from unconditionally skips and
resets at the last region of the tracked target, so the last region can be
skipped even when it has not been processed. Fix this by only resetting
the charge_{target,addr}_from when last region is reached, only skipping
when it is applied or cannot split.

Link: https://lore.kernel.org/20260908134739.96919-1-sj@kernel.org

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 0713fda22682c..c31683c12dd48 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -883,37 +883,41 @@ static bool damos_skip_charged_region(struct damon_target *t,
>  	struct damon_region *r = *rp;
>  	struct damos_quota *quota = &s->quota;
>  	unsigned long sz_to_skip;
> +	bool skip = false;
>  
>  	/* Skip previously charged regions */
>  	if (quota->charge_target_from) {
>  		if (t != quota->charge_target_from)
>  			return true;

[ ... ]

>  		if (quota->charge_addr_from && r->ar.start <
>  				quota->charge_addr_from) {
>  			sz_to_skip = ALIGN_DOWN(quota->charge_addr_from -
>  					r->ar.start, DAMON_MIN_REGION);
>  			if (!sz_to_skip) {
> -				if (damon_sz_region(r) <= DAMON_MIN_REGION)
> -					return true;
> +				if (damon_sz_region(r) <= DAMON_MIN_REGION) {
> +					skip = true;
> +					goto out;
> +				}
>  				sz_to_skip = DAMON_MIN_REGION;
>  			}
>  			damon_split_region_at(t, r, sz_to_skip);
>  			r = damon_next_region(r);
>  			*rp = r;
> +			skip = false;

[Severity: High]
If damon_split_region_at() fails to allocate memory for the new region, it
silently returns without splitting. When r is the last region,
damon_next_region() wraps around to the list head, resulting in an invalid
struct damon_region pointer. Can this cause an invalid pointer to be
assigned to *rp?

>  		}
> +	}
> +out:
> +	if (r == damon_last_region(t)) {
>  		quota->charge_target_from = NULL;
>  		quota->charge_addr_from = 0;
>  	}
> -	return false;
> +	return skip;
>  }

[Severity: High]
If the allocation failed and r evaluates to a bogus list head pointer, this
check for damon_last_region() will fail. The function will then return
skip = false, causing the caller in damon_do_apply_schemes() to proceed
with the invalid pointer.

Could this lead to out-of-bounds reads in damos_valid_target() and
out-of-bounds writes in damos_apply_scheme() when the pointer is
dereferenced?

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