If !CONFIG_PERSISTENT_HUGE_ZERO_FOLIO, the huge_zero_folio is refcounted by huge_zero_refcount and returned by mm_get_huge_zero_folio(). When the caller is done with the huge zero page, its reference count is decremented. Only a shrinker can set the reference count to zero. A race can unfortunately occur between a shrinker decrementing the reference count to zero and a concurrent page fault. This is because shrink_huge_zero_folio_scan() might, if very unlucky, be preempted between setting huge_zero_refcount to zero and writing an invalid value. During this time get_huge_zero_folio() could write to huge_zero_pfn before shrink_huge_zero_folio_scan() resumes. In this event the huge zero folio will be persistently misidentified causing the THP code path to be entered inappropriately for the huge zero folio: CPU 0 CPU 1 =======================================|================================= shrink_huge_zero_folio_scan() | atomic_cmpxchg() sets refcount to 0 | xchg() sets huge_zero_folio to NULL | get_huge_zero_folio() | | atomic_inc_not_zero() -> zero preempted for a long time | Allocate new huge zero folio | | Write valid huge_zero_folio v | Write valid huge_zero_pfn Overwrite huge_zero_pfn with ~0UL <--- Invalid overwrite! This results in is_huge_zero_pfn() and is_huge_zero_pmd() incorrectly returning false for a huge zero page which could result in issues like the huge zero folio being incorrectly split. Note that the issue is with huge_zero_pfn not huge_zero_folio, as get_huge_zero_folio() uses cmpxchg() gated on huge_zero_folio being NULL with a retry loop and shrink_huge_zero_folio_scan() uses xchg() to set huge_zero_folio. Fix the issue by introducing a spinlock, huge_zero_lock, to prevent concurrent write of huge_zero_folio, huge_zero_pfn and huge_zero_refcount. There needs to be significant care taken here to ensure correctness: The fast path in get_huge_zero_folio() uses atomic_inc_not_zero(), which is outside of the critical section when !CONFIG_PERSISTENT_HUGE_ZERO_FOLIO, and means huge zero allocation is gated on zero huge_zero_refcount. The fast path doesn't use huge_zero_lock, so the critical section is irrelevant to it. So invariants are required - huge_zero_refcount MUST: * Only be set in the huge_zero_lock critical section to ensure serialisation of huge_zero_pfn, huge_zero_folio and huge_zero_refcount writes. * Be set non-zero only AFTER huge_zero_[pfn, folio] are set to valid values so installation of the huge zero folio on read page fault ensures concurrent is_huge_zero_*() calls correctly identify the huge zero folio. * Be set zero only BEFORE huge_zero_[pfn, folio] are set to NULL and ~0UL respectively, and atomically. Establish these by: * Only updating huge_zero_refcount in the huge_zero_lock critical section in get_huge_zero_folio() and shrink_huge_zero_folio_scan(). * Using atomic_set_release(&huge_zero_refcount) in get_huge_zero_folio() after huge_zero_[pfn, folio] are set. This is paired with atomic_inc_not_zero() to ensure atomic_inc_not_zero() only observes a non-zero value if huge_zero_[pfn, folio] are set. * Using atomic_cmpxchg() in shrink_huge_zero_folio_scan() to ensure that it is set zero only when equal to 1 and set atomically. * atomic_cmpxchg() being fully ordered ensures this is done prior to huge_zero_[folio, pfn] being set to NULL and ~0UL respectively. Note that only the huge zero shrinker (via shrink_huge_zero_folio_scan()) can actually set huge_zero_refcount to zero, which is the count of mm's which have at least one huge zero folio installed plus one shrinker pin. Additionally convert a BUG_ON() to a VM_WARN_ON_ONCE(). Suggested-by: David Hildenbrand (Arm) Reported-by: Hengbin Zhang Closes: https://lore.kernel.org/linux-mm/20260727154001.4102341-1-uqbarz@gmail.com/ Fixes: 3b77e8c8cde5 ("mm/thp: make is_huge_zero_pmd() safe and quicker") Cc: stable@vger.kernel.org # 6.18.x: dependent on prior commit Signed-off-by: Lorenzo Stoakes (ARM) --- mm/huge_memory.c | 42 ++++++++++++++++++++++++++++-------------- 1 file changed, 28 insertions(+), 14 deletions(-) diff --git a/mm/huge_memory.c b/mm/huge_memory.c index 0f60bc82e87a..acee2d28b9bb 100644 --- a/mm/huge_memory.c +++ b/mm/huge_memory.c @@ -41,6 +41,7 @@ #include #include #include +#include #include #include "internal.h" @@ -82,6 +83,7 @@ struct folio *huge_zero_folio __read_mostly; unsigned long huge_zero_pfn __read_mostly = HUGE_ZERO_UNSET_PFN; #ifndef CONFIG_PERSISTENT_HUGE_ZERO_FOLIO static atomic_t huge_zero_refcount; +static DEFINE_SPINLOCK(huge_zero_lock); static struct shrinker *huge_zero_folio_shrinker; #endif @@ -270,7 +272,8 @@ void mm_put_huge_zero_folio(struct mm_struct *mm) static bool get_huge_zero_folio(void) { struct folio *zero_folio; -retry: + + /* Paired with atomic_set_release(). */ if (likely(atomic_inc_not_zero(&huge_zero_refcount))) return true; @@ -278,17 +281,21 @@ static bool get_huge_zero_folio(void) if (unlikely(!zero_folio)) return false; - preempt_disable(); - if (cmpxchg(&huge_zero_folio, NULL, zero_folio)) { - preempt_enable(); + /* Paired with critical section in shrink_huge_zero_folio_scan(). */ + spin_lock(&huge_zero_lock); + if (huge_zero_folio) { + /* Somebody else already installed it. */ + atomic_inc(&huge_zero_refcount); + spin_unlock(&huge_zero_lock); folio_put(zero_folio); - goto retry; + return true; } + WRITE_ONCE(huge_zero_folio, zero_folio); WRITE_ONCE(huge_zero_pfn, folio_pfn(zero_folio)); + /* Paired with atomic_inc_not_zero(). +1 for shrinker pin. */ + atomic_set_release(&huge_zero_refcount, 2); + spin_unlock(&huge_zero_lock); - /* We take additional reference here. It will be put back by shrinker */ - atomic_set(&huge_zero_refcount, 2); - preempt_enable(); count_vm_event(THP_ZERO_PAGE_ALLOC); return true; } @@ -312,15 +319,22 @@ static unsigned long shrink_huge_zero_folio_count(struct shrinker *shrink, static unsigned long shrink_huge_zero_folio_scan(struct shrinker *shrink, struct shrink_control *sc) { - if (atomic_cmpxchg(&huge_zero_refcount, 1, 0) == 1) { - struct folio *zero_folio = xchg(&huge_zero_folio, NULL); - BUG_ON(zero_folio == NULL); + struct folio *zero_folio; + + /* Paired with critical section in get_huge_zero_folio(). */ + scoped_guard(spinlock, &huge_zero_lock) { + /* Paired with atomic_inc_not_zero() in get_huge_zero_folio(). */ + if (atomic_cmpxchg(&huge_zero_refcount, 1, 0) != 1) + return 0; + + zero_folio = huge_zero_folio; + VM_WARN_ON_ONCE(!huge_zero_folio); + WRITE_ONCE(huge_zero_folio, NULL); WRITE_ONCE(huge_zero_pfn, HUGE_ZERO_UNSET_PFN); - folio_put(zero_folio); - return HPAGE_PMD_NR; } - return 0; + folio_put(zero_folio); + return HPAGE_PMD_NR; } static int __init huge_zero_init(void) -- 2.55.0