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

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

On 2/18/26 08:52, David Hildenbrand (Arm) wrote:
> On 2/18/26 04:45, 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 (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@vger.kernel.org
>> Signed-off-by: Chris Down <chris@chrisdown.name>
>> ---
>>   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);
>>       }
>>       set_pmd_at(mm, dst_addr, dst_pmd, _dst_pmd);
>>   
> 
> God this code is horrible.
> 
> Why can't we simply move the PMD like move_huge_pmd() would do and make
> this code less error prone?
> 
>     _dst_pmd = src_pmdval;
> 
> According to validate_move_areas(), the permissions of both VMAs are equal.
> 
> 
> We might have to handle it just like move_huge_pmd():
> 
>     _dst_pmd = move_soft_dirty_pmd(src_pmdval);
> 
> To also set the softdirty bit.
> 
> Then, we would want to just clear uffd.
> 
>     _dst_pmd = move_soft_dirty_pmd(src_pmdval);
>     _dst_pmd = clear_uffd_wp_pmd(_dst_pmd);
> 

Chris, do you have time to follow up, or should I look into it? The
issue looks quite bad, so we should tackle it ASAP.

-- 
Cheers,

David