Re: [RFC PATCH] mm/damon: adjust isolated pages stat for DAMOS_MIGRATE_{HOT,COLD}

On Sun, 26 Jul 2026 21:10:48 +0000 [email protected] wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] Early return in damon_migrate_pages() leaves isolated folios on the caller's list, causing memory leaks, stat inflation, and stack UAF.
> --
> 
> --- Patch [1]: [RFC PATCH] mm/damon: adjust isolated pages stat for DAMOS_MIGRATE_{HOT,COLD} ---
> commit 1efec5dea2b48b50043eba6bdae1821a99318049
> Author: SJ Park <[email protected]>
> 
> mm/damon: adjust isolated pages stat for DAMOS_MIGRATE_{HOT,COLD}
> 
> Callers of migrate_pages() should adjust NR_MIGRATED_{ANON,FILE} for
> isolations and putback of the folios. This patch fixes DAMOS_MIGRATE_{HOT,COLD}
> which mistakenly missed the stat adjustment, potentially confusing compaction
> and reclaim.
> 
> Link: https://github.com/damonitor/damo
> Link: https://lore.kernel.org/[email protected]
> 
> > diff --git a/mm/damon/ops-common.c b/mm/damon/ops-common.c
> > index e59f77eca83b2..9809fa12b09a8 100644
> > --- a/mm/damon/ops-common.c
> > +++ b/mm/damon/ops-common.c
> 
> [ ... ]
> 
> > diff --git a/mm/damon/paddr.c b/mm/damon/paddr.c
> > index 5a6a78054784a..5c6c3a597fd0b 100644
> > --- a/mm/damon/paddr.c
> > +++ b/mm/damon/paddr.c
> > @@ -346,6 +346,8 @@ static unsigned long damon_pa_migrate(struct damon_region *r,
> >  
> >  		if (!folio_isolate_lru(folio))
> >  			goto put_folio;
> > +		node_stat_add_folio(folio, NR_ISOLATED_ANON +
> > +				folio_is_file_lru(folio));
> >  		list_add(&folio->lru, &folio_list);
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but if s->target_nid is invalid
> (e.g., if it defaults to NUMA_NO_NODE), damon_migrate_pages() returns early
> without draining the folio list:
> 
> mm/damon/ops-common.c:damon_migrate_pages() {
> 	if (target_nid < 0 || target_nid >= MAX_NUMNODES ||
> 			!node_state(target_nid, N_MEMORY))
> 		return nr_migrated;
>     ...
> }
> 
> Since folio_list is a stack variable here in damon_pa_migrate() and is passed
> to damon_migrate_pages() later in this function, wouldn't this early return
> permanently leak the isolated folios and cause a use-after-free when the stack
> variable goes out of scope? 
> 
> Furthermore, with this patch applied, doesn't this error path also permanently
> inflate the newly added NR_ISOLATED stats because the cleanup loop in
> damon_migrate_folio_list() is never reached?
> 
> A similar issue appears to exist in mm/damon/vaddr.c:damos_va_migrate(),
> where priv.migration_lists is freed without checking if any folios were left
> behind by an early return.

Good finding, but this is a known issue.  liyouhong is working [1] on this
issue.  This patch will also further updated to decrement the stat value on the
new putback logic.

[1] https://lore.kernel.org/[email protected]

> 
> >  put_folio:
> >  		addr += folio_size(folio);
> 
> [ ... ]
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1


Thanks,
SJ