Re: [PATCH] mm/huge_memory: fix folio isn't locked in softleaf_to_folio()

["David Hildenbrand (Arm)" <david@kernel.org>]

On 2/25/26 09:12, Jinjiang Tu wrote:
> On arm64 server, we found folio that get from migration entry isn't locked
> in softleaf_to_folio(). This issue triggers when mTHP splitting and
> zap_nonpresent_ptes() races, and the root cause is lack of memory barrier
> in softleaf_to_folio(). The race is as follows:
> 
> 	CPU0                                             CPU1
> 
> deferred_split_scan()                              zap_nonpresent_ptes()
>   lock folio
>   split_folio()
>     unmap_folio()
>       change ptes to migration entries
>     __split_folio_to_order()                         softleaf_to_folio()
>       set flags(including PG_locked) for tail pages    folio = pfn_folio(softleaf_to_pfn(entry))
>       smp_wmb()                                        VM_WARN_ON_ONCE(!folio_test_locked(folio))
>       prep_compound_page() for tail pages
> 

In general, relying on a "struct page" for a migration entry is shaky,
because it can change any time from being a large folio to being a small
folio.

So we generally only check properties that would be true for either the
old (large) or the new (smaller) folio, like folio_test_ksm() or
folio_test_anon().

It's important that these properties were written for the new folio
before a migration entry user might look at the page indeed.

So it's not just about the locked state.

> In __split_folio_to_order(), smp_wmb() guarantees page flags of tail pages
> are visible before the tail page becomes non-compound. smp_wmb() should
> be paired with smp_rmb() in softleaf_to_folio(), which is missed. As a
> result, if zap_nonpresent_ptes() accesses migration entry that stores
> tail pfn, softleaf_to_folio() may see the updated compound_head of tail
> page before page->flags.
> 
> Although the code exists for long time, this issue should only exist after
> mTHP splitting is supported. For THP splitting, there is only a pmd
> migration entry 

When splitting, we first install a PTE table, no?

unmap_folio() passes TTU_SPLIT_HUGE_PMD.

and it's impossible to access migration entry that stores
> tail page pfn.
> 
> To fix it, add missing smp_rmb() if the softleaf entry is migration entry
> in softleaf_to_folio() and softleaf_to_page().
> 
> Fixes: 7dc7c5ef6463 ("mm: allow deferred splitting of arbitrary anon large folios")
> Signed-off-by: Jinjiang Tu <tujinjiang@huawei.com>
> ---
>  include/linux/leafops.h | 39 ++++++++++++++++++++++++++-------------
>  1 file changed, 26 insertions(+), 13 deletions(-)
> 
> diff --git a/include/linux/leafops.h b/include/linux/leafops.h
> index a9ff94b744f2..f823f390ba6b 100644
> --- a/include/linux/leafops.h
> +++ b/include/linux/leafops.h
> @@ -371,14 +371,21 @@ static inline unsigned long softleaf_to_pfn(softleaf_t entry)
>   */
>  static inline struct page *softleaf_to_page(softleaf_t entry)
>  {
> -	struct page *page = pfn_to_page(softleaf_to_pfn(entry));
> +	struct page *page;
>  
>  	VM_WARN_ON_ONCE(!softleaf_has_pfn(entry));
> -	/*
> -	 * Any use of migration entries may only occur while the
> -	 * corresponding page is locked
> -	 */
> -	VM_WARN_ON_ONCE(softleaf_is_migration(entry) && !PageLocked(page));
> +
> +	page = pfn_to_page(softleaf_to_pfn(entry));
> +	if (softleaf_is_migration(entry)) {
> +		/* See __split_folio_to_order() comment */
> +		smp_rmb();
> +
> +		/*
> +		 * Any use of migration entries may only occur while the
> +		 * corresponding page is locked
> +		 */
> +		VM_WARN_ON_ONCE(!PageLocked(page));
> +	}

Conceptually, wouldn't the smb_rmb() have to happen *after* the
page_folio(), like we have in softleaf_to_folio()?

-- 
Cheers,

David