[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?