[PATCH v3] mm/memory-failure: set PG_hwpoison of a non-hugetlb page under hugetlb_lock

[PATCH v3] mm/memory-failure: set PG_hwpoison of a non-hugetlb page under hugetlb_lock

[Mani Tofigh via B4 Relay]

From: Mani Tofigh <mt4022@columbia.edu>

memory_failure() checks under hugetlb_lock that the page isn't part of
a hugetlb folio and sets PG_hwpoison only after dropping the lock.  The
page can become a tail page of a new hugetlb folio in the meantime.
With hugetlb_free_vmemmap=on its struct page then lives in the tail
page that all such folios in the zone share, so the flag lands on tail
pages of every one of them:

  Task A (memory_failure)              Task B (nr_hugepages write)
  get_huge_page_for_hwpoison()
    /* P not hugetlb */
                                       alloc_fresh_hugetlb_folio()
                                         /* P becomes a tail page */
                                         hugetlb_vmemmap_optimize_folio()
                                           /* P's struct page now in
                                              the shared tail page */
  TestSetPageHWPoison(P)

The error on P itself is ignored, while healthy pages of other folios
stay marked and are never reused once their folio is freed.  After
freeing the pool, /proc/kpageflags shows them as hwpoisoned.

This is the allocation side of the race that commit 32c877191e02
("hugetlb: do not clear hugetlb dtor until allocating vmemmap") fixed
for freeing.

Set the hugetlb flag of a new folio under hugetlb_lock and set
PG_hwpoison of a non-hugetlb page inside the locked check, so that the
vmemmap can't be optimized in between.  The retry path clears it under
the same lock, unless the page has become part of a hugetlb folio in
the meantime and its struct page isn't in the first vmemmap page of the
folio, which HVO doesn't share.  The retry then knows the flag is its
own.  With the fix, no healthy page gets marked 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 35s.

Fixes: f41f2ed43ca5 ("mm: hugetlb: free the vmemmap pages associated with each HugeTLB page")
Cc: stable@vger.kernel.org
Signed-off-by: Mani Tofigh <mt4022@columbia.edu>
---
Changes in v3:
- Keep the flag only for a page beyond the first vmemmap page of the
  folio, and fix the comment that said a head page is never remapped
  (Sashiko)
- Don't take a flag kept by the retry path for an earlier error (Sashiko)
- Rebase onto current mm-new
- Link to v2: https://patch.msgid.link/20261103-hwpoison-hugetlb-ro-v2-1-43876fc9eef1@columbia.edu

Reproducer: https://github.com/manitofigh/files/blob/main/linux-repros/hugetlbro.c
---
 mm/hugetlb.c        |  3 +++
 mm/memory-failure.c | 47 ++++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 43 insertions(+), 7 deletions(-)

diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 4eb7a8c1f7f6..da4ae43de9f7 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -1792,7 +1792,10 @@ static void account_new_hugetlb_folio(struct hstate *h, struct folio *folio)
 
 void init_new_hugetlb_folio(struct folio *folio)
 {
+	/* Pairs with get_huge_page_for_hwpoison(), see there. */
+	spin_lock_irq(&hugetlb_lock);
 	__folio_set_hugetlb(folio);
+	spin_unlock_irq(&hugetlb_lock);
 	INIT_LIST_HEAD(&folio->lru);
 	hugetlb_set_folio_subpool(folio, NULL);
 	set_hugetlb_cgroup(folio, NULL);
diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index 78f0a13349cb..9795f07b26f5 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -61,6 +61,7 @@
 #include <linux/pagewalk.h>
 #include <linux/shmem_fs.h>
 #include <linux/sysctl.h>
+#include <linux/vmemmap-optimization.h>
 
 #define CREATE_TRACE_POINTS
 #include <trace/events/memory-failure.h>
@@ -1957,6 +1958,7 @@ static unsigned long __folio_free_raw_hwp(struct folio *folio, bool move_flag)
 #define	MF_HUGETLB_FOLIO_PRE_POISONED	3	/* folio already poisoned */
 #define	MF_HUGETLB_PAGE_PRE_POISONED	4	/* exact page already poisoned */
 #define	MF_HUGETLB_RETRY		5	/* hugepage is busy, retry */
+#define	MF_HUGETLB_NON_HUGEPAGE_POISONED	6	/* not a hugepage, already poisoned */
 /*
  * Set hugetlb folio as hwpoisoned, update folio private raw hwpoison list
  * to keep track of the poisoned pages.
@@ -2041,7 +2043,11 @@ static int get_huge_page_for_hwpoison(unsigned long pfn, int flags,
 	spin_lock_irq(&hugetlb_lock);
 	folio = page_folio(page);
 	if (!folio_test_hugetlb(folio)) {
-		ret = MF_HUGETLB_NON_HUGEPAGE;
+		/* Mark it before it can become part of a new hugetlb folio. */
+		if (TestSetPageHWPoison(page))
+			ret = MF_HUGETLB_NON_HUGEPAGE_POISONED;
+		else
+			ret = MF_HUGETLB_NON_HUGEPAGE;
 		goto out_unlock;
 	} else if (flags & MF_COUNT_INCREASED) {
 		ret = MF_HUGETLB_IN_USED;
@@ -2092,7 +2098,8 @@ static int get_huge_page_for_hwpoison(unsigned long pfn, int flags,
  * PageHWPoison) should be done in single hugetlb_lock range.
  * Returns:
  *	0		- recovered
- *	-ENOENT		- no hugetlb page
+ *	-ENOENT		- no hugetlb page, now marked hwpoison
+ *	-EEXIST		- no hugetlb page, already marked hwpoison
  *	-EBUSY		- not recovered
  *	-EOPNOTSUPP	- hwpoison_filter'ed
  *	-EHWPOISON	- folio or exact page already poisoned
@@ -2111,6 +2118,8 @@ static int try_memory_failure_hugetlb(unsigned long pfn, int flags)
 	switch (res) {
 	case MF_HUGETLB_NON_HUGEPAGE:	/* fallback to normal page handling */
 		return -ENOENT;
+	case MF_HUGETLB_NON_HUGEPAGE_POISONED:
+		return -EEXIST;
 	case MF_HUGETLB_RETRY:
 		if (!(flags & MF_NO_RETRY)) {
 			flags |= MF_NO_RETRY;
@@ -2170,10 +2179,32 @@ static int try_memory_failure_hugetlb(unsigned long pfn, int flags)
 	return identify_page_state(pfn, p, page_flags);
 }
 
+/*
+ * If the page has become part of a hugetlb folio in the meantime, HVO may
+ * remap its struct page unless it is in the first vmemmap page of the
+ * folio.  Keep the flag then, since the retry takes it as its own.
+ */
+static void clear_non_hugetlb_hwpoison(struct page *page)
+{
+	struct folio *folio;
+
+	spin_lock_irq(&hugetlb_lock);
+	folio = page_folio(page);
+	if (!folio_test_hugetlb(folio) ||
+	    folio_page_idx(folio, page) < VMEMMAP_OPTIMIZATION_NR_STRUCT_PAGES)
+		ClearPageHWPoison(page);
+	spin_unlock_irq(&hugetlb_lock);
+}
+
 #else
 static inline int try_memory_failure_hugetlb(unsigned long pfn, int flags)
 {
-	return -ENOENT;
+	return TestSetPageHWPoison(pfn_to_page(pfn)) ? -EEXIST : -ENOENT;
+}
+
+static inline void clear_non_hugetlb_hwpoison(struct page *page)
+{
+	ClearPageHWPoison(page);
 }
 
 static inline unsigned long folio_free_raw_hwp(struct folio *folio, bool flag)
@@ -2451,12 +2482,14 @@ int memory_failure(unsigned long pfn, int flags)
 try_again:
 	res = try_memory_failure_hugetlb(pfn, flags);
 	/*
-	 * -ENOENT means the page we found is not hugetlb, so proceed with normal page handling
+	 * -ENOENT and -EEXIST mean the page is not hugetlb and is now marked
+	 * hwpoison, so proceed with normal page handling.
 	 */
-	if (res != -ENOENT)
+	if (res != -ENOENT && res != -EEXIST)
 		goto unlock_mutex;
 
-	if (TestSetPageHWPoison(p)) {
+	/* On the retry, the flag may still be set from the first try. */
+	if (res == -EEXIST && retry) {
 		res = -EHWPOISON;
 		if (flags & MF_ACTION_REQUIRED)
 			res = kill_accessing_process(current, pfn, flags);
@@ -2487,7 +2520,7 @@ int memory_failure(unsigned long pfn, int flags)
 			} else {
 				/* We lost the race, try again */
 				if (retry) {
-					ClearPageHWPoison(p);
+					clear_non_hugetlb_hwpoison(p);
 					retry = false;
 					goto try_again;
 				}

---
base-commit: dba8aba0a9a00ff781c03d82ae534fc6f8c2e263
change-id: 20261030-hwpoison-hugetlb-ro-5f8881a60af9

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

Re: [PATCH v3] mm/memory-failure: set PG_hwpoison of a non-hugetlb page under hugetlb_lock

[Andrew Morton]

On Fri, 06 Nov 2026 11:09:50 -0500 Mani Tofigh via B4 Relay <devnull+mt4022.columbia.edu@kernel.org> wrote:

> From: Mani Tofigh <mt4022@columbia.edu>
> 
> memory_failure() checks under hugetlb_lock that the page isn't part of
> a hugetlb folio and sets PG_hwpoison only after dropping the lock.  The
> page can become a tail page of a new hugetlb folio in the meantime.
> With hugetlb_free_vmemmap=on its struct page then lives in the tail
> page that all such folios in the zone share, so the flag lands on tail
> pages of every one of them:
> 
>   Task A (memory_failure)              Task B (nr_hugepages write)
>   get_huge_page_for_hwpoison()
>     /* P not hugetlb */
>                                        alloc_fresh_hugetlb_folio()
>                                          /* P becomes a tail page */
>                                          hugetlb_vmemmap_optimize_folio()
>                                            /* P's struct page now in
>                                               the shared tail page */
>   TestSetPageHWPoison(P)
> 
> The error on P itself is ignored, while healthy pages of other folios
> stay marked and are never reused once their folio is freed.  After
> freeing the pool, /proc/kpageflags shows them as hwpoisoned.
> 
> This is the allocation side of the race that commit 32c877191e02
> ("hugetlb: do not clear hugetlb dtor until allocating vmemmap") fixed
> for freeing.
> 
> Set the hugetlb flag of a new folio under hugetlb_lock and set
> PG_hwpoison of a non-hugetlb page inside the locked check, so that the
> vmemmap can't be optimized in between.  The retry path clears it under
> the same lock, unless the page has become part of a hugetlb folio in
> the meantime and its struct page isn't in the first vmemmap page of the
> folio, which HVO doesn't share.  The retry then knows the flag is its
> own.  With the fix, no healthy page gets marked 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 35s.

Thanks.

So as I understand it, if a single 4k page turns bad at just the right
time, we can mark a lot of healthy 4k pages as bad (hence lost until
reboot) while the bad 4k page remains in use?  But that race is very
unlikely?

> Fixes: f41f2ed43ca5 ("mm: hugetlb: free the vmemmap pages associated with each HugeTLB page")
> Cc: stable@vger.kernel.org

I'm hoping we don't need to backport, because the race is so unlikely,
the consequences aren't world-ending and the backporting story is
messy.

But I don't think I'll get my hope, because the consequences of this
race are worse in older kernels:

In 6.6.162 HVO maps shared tail metadata read-only, so the misplaced
PG_hwpoison write can cause an oops.

Also in 6.6.162 memory_failure() still has a ClearPageHWPoison(p) in
its PageCompound retry block so with HVO enabled and
CONFIG_TRANSPARENT_HUGEPAGE=n, its compound retry can still reach
read-only tail metadata.

Miaohe, wdyt?

Re: [PATCH v3] mm/memory-failure: set PG_hwpoison of a non-hugetlb page under hugetlb_lock

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential
issues to consider:
- [High] mm/memory-failure: missed retry in memory_failure() on hugetlb
  allocation
- [High] mm/memory-failure: first vmemmap page HVO race in
  clear_non_hugetlb_hwpoison()
- [Medium] mm/memory-failure: stale tail PG_hwpoison in
  clear_non_hugetlb_hwpoison()
--

commit 473f986d60ad157a360960de8cae3064b0d63305
Author: Mani Tofigh <mt4022@columbia.edu>

mm/memory-failure: set PG_hwpoison of a non-hugetlb page under hugetlb_lock

This commit sets PG_hugetlb on new hugetlb folios and sets PG_hwpoison on
non-hugetlb pages under hugetlb_lock to prevent racing with HugeTLB vmemmap
optimization (HVO), and adds a helper to conditionally clear PG_hwpoison on
the memory_failure() retry path.

> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> index 78f0a13349cb3..9795f07b26f59 100644
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c
[ ... ]
> @@ -2170,10 +2179,32 @@ static int try_memory_failure_hugetlb(unsigned long pfn, int flags)
>  	return identify_page_state(pfn, p, page_flags);
>  }
>  
> +/*
> + * If the page has become part of a hugetlb folio in the meantime, HVO may
> + * remap its struct page unless it is in the first vmemmap page of the
> + * folio.  Keep the flag then, since the retry takes it as its own.
> + */
> +static void clear_non_hugetlb_hwpoison(struct page *page)
> +{
> +	struct folio *folio;
> +
> +	spin_lock_irq(&hugetlb_lock);
> +	folio = page_folio(page);
> +	if (!folio_test_hugetlb(folio) ||
> +	    folio_page_idx(folio, page) < VMEMMAP_OPTIMIZATION_NR_STRUCT_PAGES)
> +		ClearPageHWPoison(page);

[Severity: High]
Does HVO also remap the first vmemmap page of a hugetlb folio without holding
hugetlb_lock?

In vmemmap_optimize_pte(), when vmemmap_optimizable_pfn() is false (which is
the first vmemmap page of the folio), HVO duplicates src into a newly
allocated page dst, remaps the PTE to dst, and frees src:

mm/hugetlb_vmemmap.c:vmemmap_optimize_pte() {
	if (likely(vmemmap_optimizable_pfn(page_to_pfn(page), folio_order(folio)))) {
		dst = folio_vmemmap_shared_tail_page(folio);
		if (!dst)
			return -ENOMEM;
	} else {
		dst = vmemmap_dup_page_from_list(src, walk->vmemmap_pages);
	}

	list_add(&src->lru, walk->vmemmap_pages);
	...
	set_pte_at(&init_mm, addr, pte, mk_pte(dst, PAGE_KERNEL));
}

Because init_new_hugetlb_folio() sets PG_hugetlb before HVO runs, this creates
a race window on the first vmemmap page:

CPU0 (memory_failure)             CPU1 (hugetlb allocation)
get_huge_page_for_hwpoison()
  TestSetPageHWPoison(page)
                                  init_new_hugetlb_folio()
                                    __folio_set_hugetlb(folio)
                                  vmemmap_optimize_pte()
                                    dst = vmemmap_dup_page_from_list(src, ...)
                                      copy_page(dst, src)
clear_non_hugetlb_hwpoison()
  ClearPageHWPoison(page)  <-- clears old src page
                                    set_pte_at(..., dst, ...)
                                    flush_tlb_all()

After the remap, dst still has PG_hwpoison set.  If page is the head page
(index 0), the retry in get_huge_page_for_hwpoison() ->
hugetlb_update_hwpoison() sees folio_test_set_hwpoison(folio) is already true
on dst and aborts recovery with -EHWPOISON (MF_FAILED), leaving the poisoned
hugetlb folio undissolved in the pool or mapped in user processes with an
empty raw_hwp_page list.  Conversely, if hugetlb_update_hwpoison() runs
between copy_page() and set_pte_at()/flush_tlb_all(), its writes to
folio->flags and folio->_hugetlb_hwpoison (at struct page index 3, also in
the first vmemmap page) land in the displaced src page and are lost when src
is freed.

> +	spin_unlock_irq(&hugetlb_lock);
> +}

[Severity: Medium]
When folio_test_hugetlb(folio) is true and folio_page_idx(folio, page) is at
least VMEMMAP_OPTIMIZATION_NR_STRUCT_PAGES, can skipping ClearPageHWPoison()
leave a stale PG_hwpoison bit on a tail struct page?

When HVO is disabled, unsupported for the hstate, fails, or has not run yet,
the tail struct page is not remapped.  On the retry path,
try_memory_failure_hugetlb() -> get_huge_page_for_hwpoison() ->
hugetlb_update_hwpoison() sets PG_hwpoison on the head page and adds page to
raw_hwp_list_head(folio), but never clears PG_hwpoison on the tail struct
page.

If unpoison_memory() is later called on that tail PFN while the hugetlb folio
is in use (ghp > 0), it frees raw_hwp_list_head(folio), drops the ghp
reference, and then TestClearPageHWPoison(p) succeeds on the tail page and
calls folio_put(folio) a second time, dropping the active reference on the
in-use hugetlb folio while leaving PG_hwpoison set on the head page:

mm/memory-failure.c:unpoison_memory() {
	} else {
		if (folio_test_hugetlb(folio)) {
			huge = true;
			count = folio_free_raw_hwp(folio, false);
			if (count == 0) {
				folio_put(folio);
				goto unlock_mutex;
			}
		}

		folio_put(folio);
		if (TestClearPageHWPoison(p)) {
			folio_put(folio);
			ret = 0;
		}
	}
}

[ ... ]
> @@ -2487,7 +2520,7 @@ int memory_failure(unsigned long pfn, int flags)
>  			} else {
>  				/* We lost the race, try again */
>  				if (retry) {
> -					ClearPageHWPoison(p);
> +					clear_non_hugetlb_hwpoison(p);
>  					retry = false;
>  					goto try_again;
>  				}

[Severity: High]
Can memory_failure() miss this retry altogether when a page poisoned in
get_huge_page_for_hwpoison() becomes part of a new hugetlb folio?

In only_alloc_fresh_hugetlb_folio(), alloc_buddy_frozen_folio() and
alloc_gigantic_frozen_folio() remove the pages from the buddy allocator and
clear PageBuddy before init_new_hugetlb_folio() acquires hugetlb_lock and sets
PG_hugetlb.  When check_pages_enabled is false (the default when
CONFIG_DEBUG_VM is disabled and init_on_alloc/page_poison are off) or in
alloc_contig_frozen_range_noprof() (which ignores check_new_pages()'s return
value), the allocator does not reject a page with PG_hwpoison set:

CPU0 (memory_failure)             CPU1 (hugetlb allocation)
                                  only_alloc_fresh_hugetlb_folio()
                                    alloc_buddy_frozen_folio()
                                      /* clears PageBuddy(p) */
get_huge_page_for_hwpoison()
  /* !folio_test_hugetlb(folio) */
  TestSetPageHWPoison(p)
  return MF_HUGETLB_NON_HUGEPAGE
                                    init_new_hugetlb_folio()
                                      __folio_set_hugetlb(folio)
                                  hugetlb_vmemmap_optimize_folio()
                                    /* remaps p to shared tail page,
                                       frees old vmemmap page */
get_hwpoison_page(p, flags)
  get_hwpoison_hugetlb_folio()
    /* returns -EBUSY or 0 */
/* skips is_free_buddy_page(p) retry,
   returns MF_IGNORED */          <-- PG_hwpoison lost

Because the retry block in memory_failure() is only reached when
get_hwpoison_page() returns 0 and is_free_buddy_page(p) is true, a concurrent
hugetlb allocation that has already cleared PageBuddy bypasses the retry in
every hugetlb state:

1. While the new folio is still being initialized or HVO-optimized (neither
   HPG_freed nor HPG_migratable is set), get_hwpoison_hugetlb_folio() returns
   -EBUSY, get_any_page() returns -EIO after exhausting its retries, and
   memory_failure() exits via the default case (MF_MSG_GET_HWPOISON,
   MF_IGNORED).
2. Once the folio is placed on the hugetlb freelist (HPG_freed set),
   get_hwpoison_page() returns 0, is_free_buddy_page(p) is false, and
   memory_failure() exits via the MF_MSG_KERNEL_HIGH_ORDER (MF_IGNORED)
   branch.
3. If the folio is allocated for use and marked HPG_migratable,
   get_hwpoison_page() returns 1 and memory_failure() falls through into the
   non-hugetlb in-use page recovery path (hwpoison_filter(),
   split_huge_page_to_order(), etc.) on a hugetlb folio.

In (1) and (2), try_memory_failure_hugetlb() is never re-run: if HVO remaps a
tail page, the PG_hwpoison bit set in the old vmemmap page is discarded when
that vmemmap page is freed, leaving the corrupted physical page in the
hugetlb pool as a healthy page.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106-hwpoison-hugetlb-ro-v3-1-c11186963647@columbia.edu?part=1