[PATCH] mm/memory_hotplug: don't offline a hwpoisoned page still in the page cache

[PATCH] mm/memory_hotplug: don't offline a hwpoisoned page still in the page cache

[Mani Tofigh via B4 Relay]

From: Mani Tofigh <mt4022@columbia.edu>

Memory offline skips hwpoisoned pages, assuming nobody uses them
anymore.  memory_failure() however sets PG_hwpoison before it takes the
page, and taking it waits on pcp_batch_high_lock, which offline_pages()
holds for the whole offline.  A memory error during the offline thus
marks the page and then waits while the offline skips it:

  Task A (offline_pages)               Task B (memory_failure)
  zone_pcp_disable()
                                       TestSetPageHWPoison(P)
                                       get_hwpoison_page()
                                         /* waits for Task A */
  do_migrate_range()
    /* P is poisoned (not migrated) */
  test_pages_isolated()
    /* P is poisoned (accepted) */
  zone_pcp_enable()
                                       /* P is off the LRU (Ignored) */

The block goes offline while the file still points to P, and the next
walk of the file's pages (here drop_caches on ext4) crashes on a struct
page of offlined memory:

  [   11.710348] Memory failure: 0x24224: unhandlable page.
  [   11.710372] Memory failure: 0x24224: recovery action for get hwpoison page: Ignored
  [   11.765371] page:000000008c304eee is uninitialized and poisoned
  [   11.765389] page dumped because: VM_BUG_ON_PAGE(page_ref_count(page) == 0)
  [   11.765440] kernel BUG at include/linux/mm.h:1990!
  [   11.765458] Oops: invalid opcode: 0000 [#1] SMP KASAN NOPTI
  [   11.772284] CPU: 3 UID: 0 PID: 85 Comm: mfoffline Tainted: G        W            7.3.0-17087-gd865f2677c44 #1 PREEMPT
  [   11.777970] RIP: 0010:find_lock_entries+0x8ee/0xad0
  [   11.798761] Call Trace:
  [   11.799297]  <TASK>
  [   11.803284]  mapping_try_invalidate+0xfd/0x2c0
  [   11.810571]  drop_pagecache_sb+0x111/0x330
  [   11.811572]  __iterate_supers+0x2e2/0x370
  [   11.815863]  drop_caches_sysctl_handler+0x74/0xb0
  [   11.816994]  proc_sys_call_handler+0x2ec/0x440
  [   11.821360]  vfs_write+0x545/0x1250
  [   11.825220]  ksys_write+0xfa/0x200
  [   11.830202]  do_syscall_64+0xfb/0x590
  [   11.832084]  entry_SYSCALL_64_after_hwframe+0x77/0x7f
  [   11.848423]  </TASK>

Once the memory is also removed it's a use-after-free of the memory map.
tmpfs and the swap cache can end up the same way without the race.
Since commit a76054266661 ("mm: shmem: don't truncate page if memory
failure happens"), memory_failure() leaves a poisoned tmpfs folio in the
page cache on purpose, and it does the same for a dirty folio in the
swap cache.

Treat a hwpoisoned page whose folio is still in a page cache or swap
cache as busy in scan_movable_pages() and
__test_page_isolated_in_pageblock() so that the offline fails and a
waiting memory_failure() can go on.  A later offline succeeds once the
page is out of the cache.  With the fix, the offline fails with -EBUSY
and the file doesn't point to offlined memory anymore.

This bug was identified and confirmed with the help of a deterministic
hypervisor currently under development.  The C reproducer below however
hits it within about 10s.

Fixes: b023f46813cd ("memory-hotplug: skip HWPoisoned page when offlining pages")
Cc: stable@vger.kernel.org
Signed-off-by: Mani Tofigh <mt4022@columbia.edu>
---
Taking get_online_mems() in memory_failure(), as in Gregory's series
[1], would close only the race and not the tmpfs or swap cache case.

[1] https://lore.kernel.org/all/20261027042204.3741307-4-gourry@gourry.net/

Reproducer: https://github.com/manitofigh/files/blob/main/linux-repros/mfoffline.c
---
 mm/internal.h       | 9 +++++++++
 mm/memory_hotplug.c | 2 ++
 mm/page_isolation.c | 2 +-
 3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/mm/internal.h b/mm/internal.h
index aeebb3e49ed1..ba7b1650a5ca 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -1195,6 +1195,15 @@ static inline int unmap_poisoned_folio(struct folio *folio, unsigned long pfn, b
 }
 #endif
 
+/*
+ * A hwpoisoned page can be skipped by memory offline only once no page
+ * cache or swap cache can find it anymore.
+ */
+static inline bool hwpoison_page_offlineable(const struct page *page)
+{
+	return !folio_mapping(page_folio(page));
+}
+
 extern unsigned long  __must_check vm_mmap_pgoff(struct file *, unsigned long,
         unsigned long, unsigned long,
         unsigned long, unsigned long);
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index c9cc42e54f05..3d36f7f2e04b 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -1820,6 +1820,8 @@ static int scan_movable_pages(unsigned long start, unsigned long end,
 		struct folio *folio;
 
 		page = pfn_to_page(pfn);
+		if (PageHWPoison(page) && !hwpoison_page_offlineable(page))
+			return -EBUSY;
 		if (PageLRU(page) || page_has_movable_ops(page))
 			goto found;
 
diff --git a/mm/page_isolation.c b/mm/page_isolation.c
index 5aaad037384e..beb09ef345ee 100644
--- a/mm/page_isolation.c
+++ b/mm/page_isolation.c
@@ -593,7 +593,7 @@ __test_page_isolated_in_pageblock(unsigned long pfn, unsigned long end_pfn,
 			 */
 			pfn += 1 << buddy_order(page);
 		else if ((mode == PB_ISOLATE_MODE_MEM_OFFLINE) &&
-			 PageHWPoison(page))
+			 PageHWPoison(page) && hwpoison_page_offlineable(page))
 			/* A HWPoisoned page cannot be also PageBuddy */
 			pfn++;
 		else if ((mode == PB_ISOLATE_MODE_MEM_OFFLINE) &&

---
base-commit: 243e8cdab64129d6207c398f8cf5fbb1962ff7e1
change-id: 20261029-memhp-hwpoison-pagecache-a6e3a524a370

Best regards,
--  
Mani Tofigh <mt4022@columbia.edu>

Re: [PATCH] mm/memory_hotplug: don't offline a hwpoisoned page still in the page cache

[David Hildenbrand (Arm)]

On 11/1/26 20:40, Mani Tofigh via B4 Relay wrote:
> From: Mani Tofigh <mt4022@columbia.edu>
> 
> Memory offline skips hwpoisoned pages, assuming nobody uses them
> anymore.  memory_failure() however sets PG_hwpoison before it takes the
> page, and taking it waits on pcp_batch_high_lock, which offline_pages()
> holds for the whole offline.  A memory error during the offline thus
> marks the page and then waits while the offline skips it:
> 
>   Task A (offline_pages)               Task B (memory_failure)
>   zone_pcp_disable()
>                                        TestSetPageHWPoison(P)
>                                        get_hwpoison_page()
>                                          /* waits for Task A */
>   do_migrate_range()
>     /* P is poisoned (not migrated) */
>   test_pages_isolated()
>     /* P is poisoned (accepted) */
>   zone_pcp_enable()
>                                        /* P is off the LRU (Ignored) */
> 
> The block goes offline while the file still points to P, and the next
> walk of the file's pages (here drop_caches on ext4) crashes on a struct
> page of offlined memory:
> 
>   [   11.710348] Memory failure: 0x24224: unhandlable page.
>   [   11.710372] Memory failure: 0x24224: recovery action for get hwpoison page: Ignored
>   [   11.765371] page:000000008c304eee is uninitialized and poisoned
>   [   11.765389] page dumped because: VM_BUG_ON_PAGE(page_ref_count(page) == 0)
>   [   11.765440] kernel BUG at include/linux/mm.h:1990!
>   [   11.765458] Oops: invalid opcode: 0000 [#1] SMP KASAN NOPTI
>   [   11.772284] CPU: 3 UID: 0 PID: 85 Comm: mfoffline Tainted: G        W            7.3.0-17087-gd865f2677c44 #1 PREEMPT
>   [   11.777970] RIP: 0010:find_lock_entries+0x8ee/0xad0
>   [   11.798761] Call Trace:
>   [   11.799297]  <TASK>
>   [   11.803284]  mapping_try_invalidate+0xfd/0x2c0
>   [   11.810571]  drop_pagecache_sb+0x111/0x330
>   [   11.811572]  __iterate_supers+0x2e2/0x370
>   [   11.815863]  drop_caches_sysctl_handler+0x74/0xb0
>   [   11.816994]  proc_sys_call_handler+0x2ec/0x440
>   [   11.821360]  vfs_write+0x545/0x1250
>   [   11.825220]  ksys_write+0xfa/0x200
>   [   11.830202]  do_syscall_64+0xfb/0x590
>   [   11.832084]  entry_SYSCALL_64_after_hwframe+0x77/0x7f
>   [   11.848423]  </TASK>
> 
> Once the memory is also removed it's a use-after-free of the memory map.
> tmpfs and the swap cache can end up the same way without the race.
> Since commit a76054266661 ("mm: shmem: don't truncate page if memory
> failure happens"), memory_failure() leaves a poisoned tmpfs folio in the
> page cache on purpose, and it does the same for a dirty folio in the
> swap cache.
> 
> Treat a hwpoisoned page whose folio is still in a page cache or swap
> cache as busy in scan_movable_pages() and
> __test_page_isolated_in_pageblock() so that the offline fails and a
> waiting memory_failure() can go on.  A later offline succeeds once the
> page is out of the cache.  With the fix, the offline fails with -EBUSY
> and the file doesn't point to offlined memory anymore.
> 
> This bug was identified and confirmed with the help of a deterministic
> hypervisor currently under development.  The C reproducer below however
> hits it within about 10s.
> 
> Fixes: b023f46813cd ("memory-hotplug: skip HWPoisoned page when offlining pages")
> Cc: stable@vger.kernel.org
> Signed-off-by: Mani Tofigh <mt4022@columbia.edu>
> ---
> Taking get_online_mems() in memory_failure(), as in Gregory's series
> [1], would close only the race and not the tmpfs or swap cache case.
> 
> [1] https://lore.kernel.org/all/20261027042204.3741307-4-gourry@gourry.net/
> 
> Reproducer: https://github.com/manitofigh/files/blob/main/linux-repros/mfoffline.c
> ---
>  mm/internal.h       | 9 +++++++++
>  mm/memory_hotplug.c | 2 ++
>  mm/page_isolation.c | 2 +-
>  3 files changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/internal.h b/mm/internal.h
> index aeebb3e49ed1..ba7b1650a5ca 100644
> --- a/mm/internal.h
> +++ b/mm/internal.h
> @@ -1195,6 +1195,15 @@ static inline int unmap_poisoned_folio(struct folio *folio, unsigned long pfn, b
>  }
>  #endif
>  
> +/*
> + * A hwpoisoned page can be skipped by memory offline only once no page
> + * cache or swap cache can find it anymore.
> + */
> +static inline bool hwpoison_page_offlineable(const struct page *page)
> +{
> +	return !folio_mapping(page_folio(page));
> +}
> +
>  extern unsigned long  __must_check vm_mmap_pgoff(struct file *, unsigned long,
>          unsigned long, unsigned long,
>          unsigned long, unsigned long);
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index c9cc42e54f05..3d36f7f2e04b 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -1820,6 +1820,8 @@ static int scan_movable_pages(unsigned long start, unsigned long end,
>  		struct folio *folio;
>  
>  		page = pfn_to_page(pfn);
> +		if (PageHWPoison(page) && !hwpoison_page_offlineable(page))
> +			return -EBUSY;
>  		if (PageLRU(page) || page_has_movable_ops(page))
>  			goto found;
>  
> diff --git a/mm/page_isolation.c b/mm/page_isolation.c
> index 5aaad037384e..beb09ef345ee 100644
> --- a/mm/page_isolation.c
> +++ b/mm/page_isolation.c
> @@ -593,7 +593,7 @@ __test_page_isolated_in_pageblock(unsigned long pfn, unsigned long end_pfn,
>  			 */
>  			pfn += 1 << buddy_order(page);
>  		else if ((mode == PB_ISOLATE_MODE_MEM_OFFLINE) &&
> -			 PageHWPoison(page))
> +			 PageHWPoison(page) && hwpoison_page_offlineable(page))

I was recently wondering about that one as well.

What if we still have the page pinned somehow? (vmscplice etc).

Wouldn't it be better to make sure that poisoning code holds exactly one
reference (whoever set hwpoison) and check here that the refcount is 1 ?

Pagecache references would turn it > 1.

A note that I consider this here extremely low priority.

-- 
Cheers,

David

Re: [PATCH] mm/memory_hotplug: don't offline a hwpoisoned page still in the page cache

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] mm/memory_hotplug: do_migrate_range() isolates busy hwpoisoned folios from LRU
- [High] mm/memory_hotplug: scan_movable_pages() misses poisoned tail pages of folios
--

commit 90624953bdb2c5ae02a7473ca37edfda72128f6d
Author: Mani Tofigh <mt4022@columbia.edu>

mm/memory_hotplug: don't offline a hwpoisoned page still in the page cache

This commit prevents memory offlining from skipping hwpoisoned pages whose
folios still have a page-cache or swap-cache mapping in scan_movable_pages()
and __test_page_isolated_in_pageblock(). This avoids offlining memory while
files or swap caches still reference poisoned pages.

[ ... ]

> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index c9cc42e54f05d..3d36f7f2e04b6 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -1820,6 +1820,8 @@ static int scan_movable_pages(unsigned long start, unsigned long end,
>  		struct folio *folio;
>  
>  		page = pfn_to_page(pfn);
> +		if (PageHWPoison(page) && !hwpoison_page_offlineable(page))
> +			return -EBUSY;
>  		if (PageLRU(page) || page_has_movable_ops(page))
>  			goto found;

[Severity: High]
Does do_migrate_range() still isolate a non-offlineable hwpoisoned folio from
the LRU and leak its reference when another movable page precedes it in the
range?

In offline_pages(), scan_movable_pages() returns 0 as soon as it finds the
first movable page, and offline_pages() then passes the entire remaining
range [pfn, end_pfn) to do_migrate_range():

mm/memory_hotplug.c:offline_pages() {
    ...
			ret = scan_movable_pages(pfn, end_pfn, &pfn);
			if (!ret) {
				/*
				 * TODO: fatal migration failures should bail
				 * out
				 */
				do_migrate_range(pfn, end_pfn);
			}
    ...
}

In addition, because memory_failure() does not hold mem_hotplug_lock,
TestSetPageHWPoison() can run after scan_movable_pages() has already scanned
past that PFN.

When do_migrate_range() reaches the hwpoisoned page-cache or swap-cache folio
while memory_failure() is waiting in get_hwpoison_page() on
pcp_batch_high_lock, it isolates the folio from the LRU without checking
hwpoison_page_offlineable():

mm/memory_hotplug.c:do_migrate_range() {
    ...
		if (folio_contain_hwpoisoned_page(folio)) {
			if (folio_test_large(folio) && !folio_test_hugetlb(folio))
				goto put_folio;
			if (folio_test_lru(folio) && !folio_isolate_lru(folio))
				goto put_folio;
			if (folio_mapped(folio)) {
				folio_lock(folio);
				unmap_poisoned_folio(folio, pfn, false);
				folio_unlock(folio);
			}

			goto put_folio;
		}
    ...
}

On the next pass of the loop in offline_pages(), scan_movable_pages() reaches
the poisoned page and returns -EBUSY. However, folio_isolate_lru() has
already cleared PG_lru and taken an extra reference on the folio that
put_folio does not drop.

When the waiting memory_failure() resumes in get_hwpoison_page() ->
get_any_page() -> __get_hwpoison_page(), HWPoisonHandlable() sees
!PageLRU(page) and fails with -EIO instead of truncating the folio, and the
leaked reference from folio_isolate_lru() keeps the poisoned folio pinned in
the page cache so subsequent offline_pages() attempts also fail with -EBUSY.

Should do_migrate_range() also skip isolating the folio when
!hwpoison_page_offlineable() is true?

[Severity: High]
What happens in scan_movable_pages() when a tail page of a large page-cache
or swap-cache folio is hwpoisoned?

PG_hwpoison is set per subpage whereas PG_lru is on the head page. Because
scan_movable_pages() scans sequentially from start, it visits the head page
of a large folio before any tail page. At the head page, PageHWPoison(page)
is false and PageLRU(page) is true, so scan_movable_pages() takes the
goto found branch at the head PFN without scanning the poisoned tail PFN.

In do_migrate_range(), if PG_has_hwpoisoned is already set on the large
folio (for instance, when memory_failure() could not split the large folio):

mm/memory_hotplug.c:do_migrate_range() {
    ...
		if (folio_test_large(folio))
			pfn = folio_pfn(folio) + folio_nr_pages(folio) - 1;

		if (folio_contain_hwpoisoned_page(folio)) {
			/*
			 * unmap_poisoned_folio() cannot handle large folios
			 * in all cases yet.
			 */
			if (folio_test_large(folio) && !folio_test_hugetlb(folio))
				goto put_folio;
    ...
}

do_migrate_range() skips the large folio without clearing PG_lru, and the
do-while (!ret) loop in offline_pages() calls scan_movable_pages() again at
the same head PFN, looping indefinitely while holding mem_hotplug_lock and
pcp_batch_high_lock.

Conversely, if memory_failure() on a tail page is blocked in
get_hwpoison_page() before setting PG_has_hwpoisoned, do_migrate_range()
advances pfn to the end of the large folio, sees
folio_contain_hwpoisoned_page(folio) as false, and migrates the folio
despite the poisoned tail page.

Can scan_movable_pages() and do_migrate_range() check for poisoned subpages
across the folio so that large folios with poisoned tail pages are handled
consistently?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261101-memhp-hwpoison-pagecache-v1-1-ed8785ca44eb@columbia.edu?part=1