Re: [PATCH mm-hotfixes v2 0/2] mm/huge_memory: fix huge_zero_pfn race

[Andrew Morton]

On Thu, 30 Jul 2026 11:55:46 +0100 "Lorenzo Stoakes (ARM)" <ljs@xxxxxxxxxx> wrote:

> There is a subtle race in the reference-counted huge_zero_folio
> implementation.
> 
> The fast path atomic logic fails to account for the fact that the
> shrinker (which drops the final huge_zero_refcount pin) can overwrite
> huge_zero_pfn with the ~0UL sentinel value in shrink_huge_zero_folio_scan()
> after a racing get_huge_zero_folio() installed a valid value there.
> 
> This results in huge_zero_folio being correctly set but huge_zero_pfn being
> set incorrectly and thus is_huge_zero_pfn() and consequently
> is_huge_zero_pmd() will misidentify the huge zero folio as being an
> ordinary THP folio.
> 
> This can result in the huge zero folio being split and otherwise treated
> incorrectly.
> 
> The solution to this is very subtle as there is an atomic fast path, and
> thus ordering in weakly ordered architectures has to be treated very
> carefully.
> 
> The first commit fixes the issue by introducing a spinlock around
> huge_zero_[pfn, folio, refcount] write, with careful consideration paid to
> load/store ordering in the fast path. It is placed first and kept as small
> as possible so that it can be backported on its own.
> 
> The second commit is a pure cleanup which reworks the
> CONFIG_PERSISTENT_HUGE_ZERO_FOLIO logic to better separate the persistent
> logic from the dynamically allocated one.

Thanks, updated.

And moved to mm-hotfixes.  But is it appropriate to fast-track a
tricky-looking fix to a four year old bug?

> Andrew - I know you don't like a mix of fix/cleanup, but thought it'd be
> easier to keep the 2 together as there's a dependency.

Sure.

If we were to upstream just the fix then really we should defer merging
the cleanup into any tree until a later time.  Because the cleanup
might accidentally fix a flaw in the fix.  Sending just half the series
upstream results in an untested code combination.  Unlikely, but it's
happened a small number of times and it's a risk.  Keeping both patches
together eliminates that risk.

> ---
> v2:
> - Reordered the series so the fix now comes first with the
>   CONFIG_PERSISTENT_HUGE_ZERO_FOLIO separation following it as a pure
>   cleanup as per David.
> - Fixed up wording as per David.
> - Cleaned up Cc's since dependency of fix on cleanup no longer exists.

Here's how v2 altered mm.git.  One changed line, unchangelogged!

--- a/mm/huge_memory.c~b
+++ a/mm/huge_memory.c
@@ -328,7 +328,7 @@ static unsigned long shrink_huge_zero_fo
 			return 0;
 
 		zero_folio = huge_zero_folio;
-		VM_WARN_ON_ONCE(!huge_zero_folio);
+		VM_WARN_ON_ONCE(!zero_folio);
 		WRITE_ONCE(huge_zero_folio, NULL);
 		WRITE_ONCE(huge_zero_pfn, HUGE_ZERO_UNSET_PFN);
 	}
_


Sashiko said some waffle because it still doesn't understand that
__init-time allocations are considered immortal.

A cleanup opportunity is to get into this code and formalize that
convention.  And the convention that debugfs return errors are to be
ignored.