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

[SJ Park <sj@kernel.org>]

On Sat, 10 Oct 2026 06:46:00 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] mm: damon: use-after-free and permanent scheme disablement on target destroy
> --
> 
> commit c97d3ff78284ee81310d99deee85b79f32b561d5
> 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.
> 
> Link: https://lore.kernel.org/20260908134739.96919-1-sj@kernel.org
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index b5c85e6488596..1b9fb00405cfb 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;
> 
> [Severity: High]
> Does this code have a use-after-free and permanent scheme disablement issue
> when a target is destroyed?
> 
> If the user dynamically removes a target via sysfs, damon_sysfs_set_targets()
> calls damon_destroy_target() which frees the target memory:
> 
> mm/damon/sysfs.c:damon_sysfs_set_targets() {
>         ...
> 		} else {
> 			if (damon_target_has_pid(ctx))
> 				put_pid(t->pid);
> 			damon_destroy_target(t);
> 		}
>         ...
> }
> 
> mm/damon/core.c:damon_destroy_target() {
> 	damon_del_target(t);
> 	damon_free_target(t);
> }
> 
> Existing schemes are updated in-place via damon_sysfs_update_scheme(), which
> does not clear the charge_target_from pointer, leaving a dangling pointer in
> the scheme.
> 
> Immediately after, when kdamond_apply_schemes() runs in the same thread,
> damon_do_apply_schemes() compares the valid remaining targets against this
> dangling pointer (if (t != quota->charge_target_from)). Since it does not
> match the remaining targets, it unconditionally executes continue, skipping
> the scheme for all regions.
> 
> Because a matching target is never reached, the pointer is never reset (the
> code cannot reach the clean_charge_from_skip label). This results in
> permanent disablement of the scheme and persistent use-after-free comparisons
> against the dangling pointer.

This is a pre-existing issue that also exists in the mainline.  I will
separately work on this.

> 
> > -			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;
> > +			}
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20261010063042.8611-1-sj@kernel.org?part=2


Thanks,
SJ