mm/damon: adjust isolated pages stat for DAMOS_MIGRATE_{HOT,COLD}
Subsystems (3)
SJ Park
maintainerRFC 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 -34400This issue was discovered [2] by Sashiko.
[1] https://github.com/damonitor/damo
[2] https://lore.kernel.org/[email protected]
--- 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
@@ -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
@@ -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
@@ -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
[email protected]
AI reviewThank 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
[ ... ]
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@@ -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
-0put_folio: addr +=folio_size (folio);
[ ... ]
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
SJ Park
maintainerQuoted 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 indamon_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[ ... ]
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@@ -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 ifs->target_nidis invalid (e.g., if it defaults toNUMA_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_listis a stack variable here indamon_pa_migrate()and is passed todamon_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_ISOLATEDstats because the cleanup loop indamon_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
-0put_folio: addr +=folio_size (folio);[ ... ]
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
Thanks,
SJ