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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] mm: invalid list head dereference if damon_split_region_at() fails
- [Medium] mm: scheme state leak due to early return for small last region
--

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

mm/damon/core: fix unconditionally skip last region

This commit modifies damos_skip_charged_region() to only reset the charge
state when the last region is reached. It ensures the last region is skipped
only when it is applied or cannot be 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..5ede1733369f5 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c

[ ... ]

>  		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;

[Severity: Medium]
Does this early return for a small last region bypass the state clearing at
the relocated out label?

If damos_skip_charged_region() processes the last region in a target and it
is smaller than DAMON_MIN_REGION, the code executes return true early. This
bypasses the out block that clears quota->charge_target_from.

In the subsequent window, the scheme could again skip all targets until the
saved target, evaluate its last region, and hit the same early return,
potentially skipping targets endlessly across windows.

>  				sz_to_skip = DAMON_MIN_REGION;
>  			}
>  			damon_split_region_at(t, r, sz_to_skip);
>  			r = damon_next_region(r);
>  			*rp = r;

[Severity: High]
Can unconditionally advancing the region pointer after a potentially failed
split on the last element cause list head type confusion?

When damos_skip_charged_region() processes the last region of a target and
attempts to split it, it calls damon_split_region_at(). If the split fails
due to a memory allocation failure, the region is not split and remains the
last element in the list.

The subsequent call:
r = damon_next_region(r);

will dereference r->list.next, which points to the list head
(&t->regions_list), returning a type-confused pointer.

Passing this bogus pointer back via *rp = r means that subsequent calls to
damos_valid_target() and damos_apply_scheme() will dereference it, which
could lead to a heap out-of-bounds access.

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

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