[PATCH v5 01/12] memory-failure: Fix hardware poison check in unpoison_memory() again

[PATCH v5 01/12] memory-failure: Fix hardware poison check in unpoison_memory() again

[Matthew Wilcox (Oracle)]

The earlier patch (6c54312f9689) that attempted to fix unpoison_memory()
was incorrect for hugetlb folios.  Before a6fddef49eef, we checked
the head page for poison (which was correct for hugetlb and incorrect
for THP).  We are currently incapable of unpoisoning pages (other than
the first page) in a hugetlb folio.

Use is_page_hwpoison() which handles hugetlb pages sufficiently well for
this purpose.  This is racy as we don't have a reference on the folio at
this point, but fixing that properly requires deeper surgery and this
is a CAP_SYS_ADMIN path only so this will do for the moment and can be
easily backported.  It's no worse than the situation before a6fddef49eef.

The other bug in a6fddef49eef is that we currently clear the HWPoison
flag on the precise page, which is wrong for hugetlb.  Fix that too.

Fixes: a6fddef49eef ("mm/memory-failure: convert unpoison_memory() to folios")
Cc: stable@vger.kernel.org
Cc: Naoya Horiguchi <naoya.horiguchi@nec.com>
Cc: Miaohe Lin <linmiaohe@huawei.com>
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 mm/memory-failure.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index 51508a55c405..944e6e1d4971 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -2695,8 +2695,8 @@ int unpoison_memory(unsigned long pfn)
 		goto unlock_mutex;
 	}
 
-	if (!PageHWPoison(p)) {
-		unpoison_pr_info("%#lx: page was already unpoisoned\n",
+	if (!is_page_hwpoison(p)) {
+		unpoison_pr_info("%#lx: page is not poisoned\n",
 				 pfn, &unpoison_rs);
 		goto unlock_mutex;
 	}
@@ -2748,6 +2748,7 @@ int unpoison_memory(unsigned long pfn)
 				folio_put(folio);
 				goto unlock_mutex;
 			}
+			p = &folio->page;
 		}
 
 		folio_put(folio);
-- 
2.47.3

Re: [PATCH v5 01/12] memory-failure: Fix hardware poison check in unpoison_memory() again

[Matthew Wilcox]

Sashiko:
> --- Patch [1]: [PATCH v5 01/12] memory-failure: Fix hardware poison check in unpoison_memory() again ---
> commit 4beed8e27f2afa68261389ac2133cfe324d37d0e
> Author: Matthew Wilcox (Oracle) <willy@infradead.org>
> 
> memory-failure: Fix hardware poison check in unpoison_memory() again
> 
> This commit modifies unpoison_memory() to use is_page_hwpoison() to better
> support hugetlb folios and ensures the HWPoison flag is cleared on the correct
> head page.
> 
> > diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> > index 51508a55c4055..944e6e1d49719 100644
> > --- a/mm/memory-failure.c
> > +++ b/mm/memory-failure.c
> > @@ -2695,8 +2695,8 @@ int unpoison_memory(unsigned long pfn)
> >  		goto unlock_mutex;
> >  	}
> >  
> > -	if (!PageHWPoison(p)) {
> > -		unpoison_pr_info("%#lx: page was already unpoisoned\n",
> > +	if (!is_page_hwpoison(p)) {
> > +		unpoison_pr_info("%#lx: page is not poisoned\n",
> 
> Can this speculative evaluation of is_page_hwpoison() trigger a panic?
> 
> Since this is evaluated without a reference on the page, the underlying
> page could be concurrently freed and reallocated as a tail page.
> Looking at is_page_hwpoison() in include/linux/page-flags.h, it calls
> folio_test_hugetlb(folio), which uses const_folio_flags(). That function

No it doesn't.  folio_test_hugetlb() is one of th FOLIO_TYPE_OPS which is
defined as return data_race(folio->page.page_type >> 24) == PGTY_##lname;
and no asserts.  I mean, there probably should be!  But this is irrelevant;
this whole function contains many such folio tests and the same problem
applies to all of them.  This patch doesn't make it worse.

> enforces a strict assertion:
> 
> VM_BUG_ON_PGFLAGS(PageTail(&folio->page))
> 
> If the page is concurrently reallocated as a tail page while
> is_page_hwpoison() executes, won't this crash the kernel on debug builds?
> 
> >  				 pfn, &unpoison_rs);
> >  		goto unlock_mutex;
> >  	}
> 
> [ ... ]
> 
> > @@ -2748,6 +2748,7 @@ int unpoison_memory(unsigned long pfn)
> >  				folio_put(folio);
> >  				goto unlock_mutex;
> >  			}
> > +			p = &folio->page;
> >  		}
> >  
> >  		folio_put(folio);
> 
> This isn't a bug introduced by this patch, but does this lead to a
> double folio_put() and use-after-free for hugetlb folios?
> 
> If memory_failure() handles an in-use hugetlb folio, me_huge_page()
> might fail to dissolve it, meaning no extra poison reference is taken
> but the folio remains marked as hwpoisoned. If the folio is later
> unmapped and holds a single transient reference, unpoison_memory()
> will acquire a reference (ghp = 1) and perform the folio_put(folio)
> seen just below this reassignment.

I'm not an expert in the memory-poison field, but this also seems wrong.
Don't all hugetlb folios have a folio_mapping()?  And thus we'll never
take the dissolve path in me_huge_page().

At any rate, it says "not introduced by this patch", so I'm not going
to take any action.

> By reassigning p = &folio->page here, the subsequent call to
> TestClearPageHWPoison(p) will be tested on the head page and return
> true, triggering a second unconditional folio_put(folio). Does this
> underflow the transient pin and cause a use-after-free?
> 
> 
> This is a pre-existing issue, but does this same logic underflow the
> refcount for THP folios that previously failed to split?
> 
> If memory_failure() handles a THP and try_to_split_thp_page() fails, it
> explicitly drops the reference acquired via get_hwpoison_page(p), leaving
> the THP with no extra poison pin. However, TestSetPageHWPoison(p) was
> already called.
> 
> If the THP later reaches a state with a single transient reference
> and no mappings, unpoison_memory() acquires a reference (ghp = 1),
> calls folio_put(folio) once, and then TestClearPageHWPoison(p) clears
> the flag and returns true. This triggers the second folio_put(folio),
> dropping a non-existent poison pin.

I don't know, and I'm losing patience with the Sashiko rabbit holes.
This is all getting very far afield from the problem I actually want
to fix.

Re: [PATCH v5 01/12] memory-failure: Fix hardware poison check in unpoison_memory() again

[Andrew Morton]

On Mon, 27 Jul 2026 15:43:53 +0100 Matthew Wilcox <willy@infradead.org> wrote:

> > If the THP later reaches a state with a single transient reference
> > and no mappings, unpoison_memory() acquires a reference (ghp = 1),
> > calls folio_put(folio) once, and then TestClearPageHWPoison(p) clears
> > the flag and returns true. This triggers the second folio_put(folio),
> > dropping a non-existent poison pin.
> 
> I don't know, and I'm losing patience with the Sashiko rabbit holes.
> This is all getting very far afield from the problem I actually want
> to fix.

That's fair enough - you aren't a memory-failure maintainer.  If you
want to take a shot at the pre-existing things then great, but
otherwise then let's ensure that the relevant maintainers were cc'ed
and move on.

One relatively minor issue here is that if Sashiko's pre-exiting thing
is serious, we may decide to fix it with a cc:stable and such a fix
should be staged ahead of your series for backportability reasons.  If
we end up upstreaming these things in the "wrong" order then the
backportable-fix developers will be asked to prepare a backportable
version of their patch, which is a bit cumbersome, but not the end of the
world.