Skip to contentSkip to conversation

mm/damon: adjust isolated pages stat for DAMOS_MIGRATE_{HOT,COLD}

RFC PATCH In review SJ Park 2 participants 3 messages Jul 26

Subsystems (3)
lore
Share & tools
lore

SJ Park

author maintainer
#0 lore raw

RFC PATCH 1/1 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. 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/[email protected]

Fixes b51820ebea65 ("mm/damon/paddr: introduce DAMOS_MIGRATE_COLD action for demotion")
Cc <[email protected]> # 6.11.x
---
 mm/damon/ops-common.c | 2 ++
 mm/damon/paddr.c      | 2 ++
 mm/damon/vaddr.c      | 3 ++-
 3 files changed, 6 insertions(+), 1 deletion(-)
mm/damon/ops-common.c +2 -0
index e59f77eca83b2..9809fa12b09a8 100644
@@ -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);
 	}
 
mm/damon/paddr.c +2 -0
index 5a6a78054784a..5c6c3a597fd0b 100644
@@ -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);
mm/damon/vaddr.c +2 -1
index 4976775fe2762..0648400b2d65b 100644
@@ -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

Signature · 1 line
2.47.3
#1 lore raw
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

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.

Quoted 4 lines
mm/damon/ops-common.c +0 -0
index e59f77eca83b2..9809fa12b09a8 100644

[ ... ]

mm/damon/paddr.c static unsigned long damon_pa_migrate(struct damon_region *r, · line 351

Quoted 11 lines
mm/damon/paddr.c +2 -0
index 5a6a78054784a..5c6c3a597fd0b 100644
@@ -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.

Quoted 2 lines
mm/damon/paddr.c +0 -0
 put_folio:
 		addr += folio_size(folio);

[ ... ]

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
author maintainer
#2 lore raw
Quoted 56 lines · 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.

Quoted 4 lines
mm/damon/ops-common.c +0 -0
index e59f77eca83b2..9809fa12b09a8 100644

[ ... ]

mm/damon/paddr.c static unsigned long damon_pa_migrate(struct damon_region *r, · line 351

Quoted 11 lines
mm/damon/paddr.c +2 -0
index 5a6a78054784a..5c6c3a597fd0b 100644
@@ -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?

Quoted 7 lines

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]

Quoted 8 lines
Quoted 3 lines
mm/damon/paddr.c +0 -0
 put_folio:
 		addr += folio_size(folio);

[ ... ]

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1

Thanks,
SJ