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

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

[Chris Down]

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@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);
 
-- 
2.51.2

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@linux-foundation.org>
M:	David Hildenbrand <david@kernel.org>
M:	Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
R:	Zi Yan <ziy@nvidia.com>
R:	Baolin Wang <baolin.wang@linux.alibaba.com>
R:	Liam R. Howlett <Liam.Howlett@oracle.com>
R:	Nico Pache <npache@redhat.com>
R:	Ryan Roberts <ryan.roberts@arm.com>
R:	Dev Jain <dev.jain@arm.com>
R:	Barry Song <baohua@kernel.org>
R:	Lance Yang <lance.yang@linux.dev>


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@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);

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@oracle.com/


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

Thanks, Lorenzo

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

[David Hildenbrand (Arm)]

>>  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.

I argued in v1 that we should handle it similar to an ordinary move
during mremap()->move_huge_pmd() and not split it over two patches.

It's still split over two patches, which doesn't make sense.

https://lore.kernel.org/linux-mm/0b653dcd-842b-4360-bc1c-8fe779efbc23@kernel.org/

I don't think there is no need to get the folio involved at all if we
know that we have a well-prepared PMD (zero folio, makred as special).

The less code we have that has to deal with setting PMDs special (and
possible messing it up), the better.


@Chris, please make sure to CC all relevant maintainers (I didn't check)
and send the patches as a proper thread (e.g., through git send-mail").

-- 
Cheers,

David

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

[Lorenzo Stoakes]

On Mon, Mar 02, 2026 at 05:23:30PM +0000, Lorenzo Stoakes wrote:
> > 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@oracle.com/

OK ignore me, I saw David's comment and agree with him, as I said to you in the
2/3, let's just take that as the patch and mark that fixes e3981db444a0.

Assuming you agree David?

Thanks, Lorenzo

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

[David Hildenbrand (Arm)]

On 3/2/26 18:36, Lorenzo Stoakes wrote:
> On Mon, Mar 02, 2026 at 05:23:30PM +0000, Lorenzo Stoakes wrote:
>>> 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@oracle.com/
> 
> OK ignore me, I saw David's comment and agree with him, as I said to you in the
> 2/3, let's just take that as the patch and mark that fixes e3981db444a0.
> 
> Assuming you agree David?

Yes, that's what I suggested I think.

But if Chris is not able to get it done in a timely fashion, maybe you
can take care of it.

I asked something similar in reply to v1:

"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."

https://lore.kernel.org/linux-mm/7da49940-e1a4-4018-9db1-208411598e77@kernel.org/

-- 
Cheers,

David

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

[Lorenzo Stoakes]

On Mon, Mar 02, 2026 at 06:35:46PM +0100, David Hildenbrand (Arm) wrote:
>
> >>  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.
>
> I argued in v1 that we should handle it similar to an ordinary move
> during mremap()->move_huge_pmd() and not split it over two patches.
>
> It's still split over two patches, which doesn't make sense.

Yes, let's not do that, I made the same comment.

>
> https://lore.kernel.org/linux-mm/0b653dcd-842b-4360-bc1c-8fe779efbc23@kernel.org/
>
> I don't think there is no need to get the folio involved at all if we
> know that we have a well-prepared PMD (zero folio, makred as special).
>
> The less code we have that has to deal with setting PMDs special (and
> possible messing it up), the better.

Yup I agree, I replied accordingly. That's a more elegant thing than duplicating
huge zero installation code.

I had just assumed that there was _some reason_ why we wouldn't want to do that
given the original patch from Suren didn't just do that, and for the sakes of a
backport no need to think too deep on it.

But you're right I don't think there's any reason we need to diverge from what
mremap() would do.

That does have:

		if (vma_has_uffd_without_event_remap(vma))
			pmd = clear_uffd_wp_pmd(pmd);

Though rather than unconditonally invoking clear_uffd_wp_pmd().

Is that correct?

(I hate the uffd wp stuff)

>
>
> @Chris, please make sure to CC all relevant maintainers (I didn't check)
> and send the patches as a proper thread (e.g., through git send-mail").
>
> --
> Cheers,
>
> David

Thanks, Lorenzo

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

[David Hildenbrand (Arm)]

On 3/2/26 18:43, Lorenzo Stoakes wrote:
> On Mon, Mar 02, 2026 at 06:35:46PM +0100, David Hildenbrand (Arm) wrote:
>>
>>>
>>> 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.
>>
>> I argued in v1 that we should handle it similar to an ordinary move
>> during mremap()->move_huge_pmd() and not split it over two patches.
>>
>> It's still split over two patches, which doesn't make sense.
> 
> Yes, let's not do that, I made the same comment.
> 
>>
>> https://lore.kernel.org/linux-mm/0b653dcd-842b-4360-bc1c-8fe779efbc23@kernel.org/
>>
>> I don't think there is no need to get the folio involved at all if we
>> know that we have a well-prepared PMD (zero folio, makred as special).
>>
>> The less code we have that has to deal with setting PMDs special (and
>> possible messing it up), the better.
> 
> Yup I agree, I replied accordingly. That's a more elegant thing than duplicating
> huge zero installation code.
> 
> I had just assumed that there was _some reason_ why we wouldn't want to do that
> given the original patch from Suren didn't just do that, and for the sakes of a
> backport no need to think too deep on it.
> 
> But you're right I don't think there's any reason we need to diverge from what
> mremap() would do.
> 
> That does have:
> 
> 		if (vma_has_uffd_without_event_remap(vma))
> 			pmd = clear_uffd_wp_pmd(pmd);
> 
> Though rather than unconditonally invoking clear_uffd_wp_pmd().
> 
> Is that correct?

My conclusion was that UFFDIO_MOVE will never move uffd-wp information
(just like we currently don't do for any moved PTEs).

mremap() might sometimes. But it also effectively moves all (most) uffd
VMA properties, so it has slightly different semantics.

-- 
Cheers,

David