Re: [PATCH v2 1/3] mm/huge_memory: Fix use of NULL folio in move_pages_huge_pmd()

[Lorenzo Stoakes]

+cc THP.

You didn't cc the right people at all, which meant I just spent a few hours
tracking down and fixing the same bug [0]... PLEASE PLEASE run
get_maintainers.pl. For the love of all that's holy.

MEMORY MANAGEMENT - THP (TRANSPARENT HUGE PAGE)
M:	Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
M:	David Hildenbrand <david@xxxxxxxxxx>
M:	Lorenzo Stoakes <lorenzo.stoakes@xxxxxxxxxx>
R:	Zi Yan <ziy@xxxxxxxxxx>
R:	Baolin Wang <baolin.wang@xxxxxxxxxxxxxxxxx>
R:	Liam R. Howlett <Liam.Howlett@xxxxxxxxxx>
R:	Nico Pache <npache@xxxxxxxxxx>
R:	Ryan Roberts <ryan.roberts@xxxxxxx>
R:	Dev Jain <dev.jain@xxxxxxx>
R:	Barry Song <baohua@xxxxxxxxxx>
R:	Lance Yang <lance.yang@xxxxxxxxx>


I'm giving review feedback below, so you should respin, but in the next series
can you cc everyone above and _please_ make sure the threading works correctly?
As I can't even find all the patches in this series properly, it all seems to be
broken.

On Thu, Feb 26, 2026 at 10:15:31PM +0800, Chris Down wrote:
> move_pages_huge_pmd() handles UFFDIO_MOVE for both normal THPs and huge
> zero pages. For the huge zero page path, src_folio is explicitly set to
> NULL, and is used as a sentinel to skip folio operations like lock and
> rmap.
>
> In the huge zero page branch, src_folio is NULL, so folio_mk_pmd(NULL,
> pgprot) passes NULL through folio_pfn() and page_to_pfn(). With
> SPARSEMEM_VMEMMAP this silently produces a bogus PFN, installing a PMD
> pointing to non-existent physical memory. On other memory models it is a
> NULL dereference.
>
> Use page_folio(src_page) to obtain the valid huge zero folio from the
> page, which was obtained from pmd_page() and remains valid throughout.
>
> Fixes: e3981db444a0 ("mm: add folio_mk_pmd()")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Chris Down <chris@xxxxxxxxxxxxxx>
> ---
>  mm/huge_memory.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 44ff8a648afd..fed57951a7cd 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -2794,7 +2794,7 @@ int move_pages_huge_pmd(struct mm_struct *mm, pmd_t *dst_pmd, pmd_t *src_pmd, pm
>  		_dst_pmd = pmd_mkwrite(pmd_mkdirty(_dst_pmd), dst_vma);
>  	} else {
>  		src_pmdval = pmdp_huge_clear_flush(src_vma, src_addr, src_pmd);
> -		_dst_pmd = folio_mk_pmd(src_folio, dst_vma->vm_page_prot);
> +		_dst_pmd = folio_mk_pmd(page_folio(src_page), dst_vma->vm_page_prot);

I prefer my version at [0].

Cleaner to actually pull out the zero_folio into a local variable, and also we
should mark it special to be consistent with other codepaths.

[0]:https://lore.kernel.org/all/20260302170619.867056-1-lorenzo.stoakes@xxxxxxxxxx/


>  	}
>  	set_pmd_at(mm, dst_addr, dst_pmd, _dst_pmd);
>
> --
> 2.51.2
>
>
>

Thanks, Lorenzo