All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH] mm/damon: adjust isolated pages stat for DAMOS_MIGRATE_{HOT,COLD}
@ 2026-07-26 20:55 SJ Park
  2026-07-26 21:10 ` sashiko-bot
  0 siblings, 1 reply; 3+ messages in thread
From: SJ Park @ 2026-07-26 20:55 UTC (permalink / raw)
  Cc: SJ Park, stable, Andrew Morton, Honggyu Kim, Hyeongtak Ji, damon,
	linux-kernel, linux-mm

Callers of migrate_pages() should adjust NR_MIGRATED_{ANON,FILE} for
isolations and putback of the folios.  That for migration succeeded
folios is done by migrate_pages(), in migrate_folio_done().  That for
MR_DEMOTION reason is an exception though.

DAMOS_MIGRATE_{HOT,COLD} call migrate_pages() but mistakenly not doing
the stat adjustment.  As a result, use of DAMOS_MIGRATE_{HOT,COLD} could
corrupt the stat.  It could confuse too_many_isolated(), make compaction
and reclaim to behave in unexpected ways.  The stat corruption can be
reproduced and confirmed using DAMON user-space tool [1] on NUMA
systems, like below.

    $ numactl --hardware
    available: 2 nodes (0-1)
    [...]
    $ sudo ./damo start --damos_action migrate_hot 1
    $ sudo cat /proc/sys/vm/stat_refresh
    $ sudo dmesg
    [...]
    [   80.215554] vmstat_refresh: nr_isolated_anon -5578
    [   80.216842] vmstat_refresh: nr_isolated_file -34400

This issue was discovered [2] by Sashiko.

[1] https://github.com/damonitor/damo
[2] https://lore.kernel.org/20260726164356.87940-1-sj@kernel.org

Fixes: b51820ebea65 ("mm/damon/paddr: introduce DAMOS_MIGRATE_COLD action for demotion")
Cc: <stable@vger.kernel.org> # 6.11.x
Signed-off-by: SJ Park <sj@kernel.org>
---
 mm/damon/ops-common.c | 2 ++
 mm/damon/paddr.c      | 2 ++
 mm/damon/vaddr.c      | 3 ++-
 3 files changed, 6 insertions(+), 1 deletion(-)

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
@@ -374,6 +374,8 @@ static unsigned int damon_migrate_folio_list(struct list_head *folio_list,
 	while (!list_empty(folio_list)) {
 		folio = lru_to_folio(folio_list);
 		list_del(&folio->lru);
+		node_stat_sub_folio(folio, NR_ISOLATED_ANON +
+				folio_is_file_lru(folio));
 		folio_putback_lru(folio);
 	}
 
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);
 put_folio:
 		addr += folio_size(folio);
diff --git a/mm/damon/vaddr.c b/mm/damon/vaddr.c
index 4976775fe2762..0648400b2d65b 100644
--- a/mm/damon/vaddr.c
+++ b/mm/damon/vaddr.c
@@ -627,7 +627,8 @@ static void damos_va_migrate_dests_add(struct folio *folio,
 isolate:
 	if (!folio_isolate_lru(folio))
 		return;
-
+	node_stat_add_folio(folio, NR_ISOLATED_ANON +
+			folio_is_file_lru(folio));
 	list_add(&folio->lru, &migration_lists[i]);
 }
 

base-commit: 1e15fbef3a3c465a6a2db0f38e3980d75420bd6d
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [RFC PATCH] mm/damon: adjust isolated pages stat for DAMOS_MIGRATE_{HOT,COLD}
  2026-07-26 20:55 [RFC PATCH] mm/damon: adjust isolated pages stat for DAMOS_MIGRATE_{HOT,COLD} SJ Park
@ 2026-07-26 21:10 ` sashiko-bot
  2026-07-26 21:27   ` SJ Park
  0 siblings, 1 reply; 3+ messages in thread
From: sashiko-bot @ 2026-07-26 21:10 UTC (permalink / raw)
  To: SJ Park; +Cc: damon

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 <sj@kernel.org>

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/20260726164356.87940-1-sj@kernel.org

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

>  put_folio:
>  		addr += folio_size(folio);

[ ... ]

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [RFC PATCH] mm/damon: adjust isolated pages stat for DAMOS_MIGRATE_{HOT,COLD}
  2026-07-26 21:10 ` sashiko-bot
@ 2026-07-26 21:27   ` SJ Park
  0 siblings, 0 replies; 3+ messages in thread
From: SJ Park @ 2026-07-26 21:27 UTC (permalink / raw)
  To: sashiko-bot; +Cc: SJ Park, damon

On Sun, 26 Jul 2026 21:10:48 +0000 sashiko-bot@kernel.org 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 <sj@kernel.org>
> 
> 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/20260726164356.87940-1-sj@kernel.org
> 
> > 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/20260726014815.1280757-1-dayou5941@163.com

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


Thanks,
SJ

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-07-26 21:27 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-26 20:55 [RFC PATCH] mm/damon: adjust isolated pages stat for DAMOS_MIGRATE_{HOT,COLD} SJ Park
2026-07-26 21:10 ` sashiko-bot
2026-07-26 21:27   ` SJ Park

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.