Re: [PATCH v8 10/15] mm: Check individual hugetlb pages for poison

On Tue, Aug 04, 2026 at 03:15:37PM -0400, Gregory Price wrote:
> On Fri, Jul 31, 2026 at 09:07:55PM +0100, Matthew Wilcox (Oracle) wrote:
> > +/*
> > + * We have no reference on the folio containing this page.
> > + * The hugetlb_lock keeps hugetlb folios from being freed.
> > + */
> > +bool hugetlb_unref_page_hwpoison(const struct page *page)
> > +{
> > +	const struct folio *folio;
> > +	unsigned long flags;
> > +	bool ret;
> > +
> > +	spin_lock_irqsave(&hugetlb_lock, flags);
> > +	folio = page_folio(page);
> > +	if (!folio_test_huge_poison(folio)) {
> > +		ret = PageHWPoison(page);
> > +		goto unlock;
> > +	}
> > +
> > +	ret = precise_page_poisoned(folio, page);
> > +unlock:
> > +	spin_unlock_irqrestore(&hugetlb_lock, flags);
> > +	return ret;
> > +}
> > +
> 
> I ended up with the same question as sashiko - i think this behavior
> implies we must hold a reference on the page, otherwise the folio can
> be invalid and all these accesses are unsafe.
> 
> But that must be the existing behavior for poison checks like this, so
> this is at least no worse. Not sure it's worth addressing.

Let me just paste Sashiko's comment in here so we preserve it in the
lore archives and so we're definitely talking about the same thing:

> This is a pre-existing issue, but does calling page_folio()
> here without holding a reference to the page risk a panic during
> lockless PFN scanning?  If a monitoring tool reads /proc/kpageflags or
> /proc/kcore, pfn_to_online_page() retrieves the page without acquiring
> a reference. If the page is concurrently freed and reallocated for
> driver use, compound_info (which aliases lru.next) could be modified.
> Could this cause page_folio() to misidentify the page as a tail page
> and return an invalid folio pointer, leading to an out-of-bounds memory
> read when folio_test_huge_poison() accesses folio->page.page_type?
> While the hugetlb_lock is held here, can it stabilize unreferenced
> pages that might have already been repurposed into non-hugetlb pages?

Sashiko is mistaken (... and fair enough, I suspect you aren't the only
human who's confused by this either)

It's quite right that we can observe a page in literally any state since
we do not hold a refcount.  But it is forbidden to use bit 0 of lru.next
for any purpose other than indicating "this is a tail page":

       /*
         * Five words (20/40 bytes) are available in this union.
         * WARNING: bit 0 of the first word is used for PageTail(). That
         * means the other users of this union MUST NOT use the bit to
         * avoid collision and false-positive PageTail().
         */

That's been the rule since 2015 with commit 1d798ca3f164

So the pointer we get back from page_folio() must have been a folio _at
some point_.  It may not be a folio now.  It may be a folio, but not one
that contains this page.  But it's not a wild pointer, and treating it
as if it's a folio won't cause any harm (as long as we're really careful).

Specifically, we call:

> +	if (!folio_test_huge_poison(folio)) {

and all that does is access folio->page.page_type aka mapcount.  So if
the pointer we have is not a current folio, it'll just return false and
we'll check PageHWPoison.

So the only case this can return 'true' is if the folio was hugetlb
at that exact point.  And we've got the hugetlb lock, so it can't stop
being a hugetlb folio.  At this point it's safe to walk the list.

That's my reasoning, and I think Sashiko has explained enough of its
reasoning to be fairly sure Sashiko is wrong about this.  But hey,
you're not Sashiko.  Maybe you've found a gap in my logic.