Re: [PATCH v2 1/6] arm64: make huge_ptep_get handled unaligned addresses

Re: [PATCH v2 1/6] arm64: make huge_ptep_get handled unaligned addresses

[David Hildenbrand (Arm)]

On 7/2/26 07:13, Dev Jain wrote:
> huge_ptep_get() can be handed a virtual address pointing to the middle of
> a contpmd/contpte mapped hugetlb folio (examples of callers are
> pagemap_hugetlb_range, page_mapped_in_vma).
> 
> The arm64 helper rewalks the pgtables in find_num_contig to answer whether
> the huge pte we have maps a contpmd or a contpte hugetlb folio, and
> returns CONT_PMDS or CONT_PTES, so that it can collect a/d bits over the
> contiguous ptes. We can falsely return CONT_PTES instead of CONT_PMDS
> if the addr is not aligned.
> 
> Fix this by aligning the pmdp pointer down to a contpmd base before
> checking equality with the passed huge pte pointer, to correctly answer
> whether the huge pte is the base of a contpmd block.
> 
> Fixes: 29cb80519689 ("arm64: hugetlb: Cleanup huge_pte size discovery mechanisms")
> Cc: stable@vger.kernel.org
> Signed-off-by: Dev Jain <dev.jain@arm.com>
> ---
>  arch/arm64/mm/hugetlbpage.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c
> index 30772a909aea3..8e799c1fe0aa6 100644
> --- a/arch/arm64/mm/hugetlbpage.c
> +++ b/arch/arm64/mm/hugetlbpage.c
> @@ -87,7 +87,7 @@ static int find_num_contig(struct mm_struct *mm, unsigned long addr,
>  	p4dp = p4d_offset(pgdp, addr);
>  	pudp = pud_offset(p4dp, addr);
>  	pmdp = pmd_offset(pudp, addr);
> -	if ((pte_t *)pmdp == ptep) {
> +	if ((pte_t *)PTR_ALIGN_DOWN(pmdp, sizeof(*pmdp) * CONT_PMDS) == ptep) {
>  		*pgsize = PMD_SIZE;
>  		return CONT_PMDS;
>  	}


Acked-by: David Hildenbrand (Arm) <david@kernel.org>

-- 
Cheers,

David

Re: [PATCH v2 2/6] mm/rmap: use huge_ptep_get() in try_to_unmap_one()

[David Hildenbrand (Arm)]

On 7/2/26 11:35, Muchun Song wrote:
> 
> 
>> On Jul 2, 2026, at 17:08, Dev Jain <dev.jain@arm.com> wrote:
>>
>>
>>
>> On 02/07/26 2:17 pm, Muchun Song wrote:
>>>
>>>
>>>
>>> Maybe I didn't express my thoughts clearly in the first version, let me
>>> explain in more detail.
>>>
>>> We should define this stub as a no-op for !CONFIG_HUGETLB_PAGE (like
>>> set_huge_pte_at, that is why I mentioned 5d4af6195c87c6 for your reference
>>> in your previous version). Currently, you've added a declaration, but the
>>> function itself doesn't actually exist, which seems quite strange to me.
>>
>> https://lore.kernel.org/all/a4fe8ba6-2ecd-4bb9-95a9-27f9f1e87d2e@kernel.org/
>>
>> David suggested this. Honestly I quite like David's suggestion, what do you
>> think?
> 
> Thanks for pointing that out, I missed it earlier. That said, looking at
> hugetlb.h, it already contains quite a few no-op stubs. To keep things
> consistent, I'd personally prefer a stub here. Since David suggested this,
> I’d love to hear his thoughts on this as well.

It's the shortest possible way to enforce (through the linker!) that you
function is not called from wrong context.

The only alternative is using a stub with a BUILD_BUG() in it. Which will 4 5 LOC :)

-- 
Cheers,

David

Re: [PATCH v2 2/6] mm/rmap: use huge_ptep_get() in try_to_unmap_one()

[David Hildenbrand (Arm)]

On 7/2/26 07:13, Dev Jain wrote:
> try_to_unmap_one() handles hugetlb folios when memory failure needs
> to replace a poisoned hugetlb mapping with a hwpoison entry. In that
> case page_vma_mapped_walk() returns the pte pointer to the hugetlb folio
> in pvmw.pte, but the code reads it with ptep_get().
> 
> On arches which provide their own huge_ptep_get() to dereference a huge
> pte pointer, accessing via ptep_get() would cause pte_pfn(), pte_present()
> etc to misbehave.
> 
> It is not clear whether this has a trivially visible effect to userspace.
> 
> Just use huge_ptep_get() for dereferencing a huge pte pointer.
> 
> Fixes: c7ab0d2fdc84 ("mm: convert try_to_unmap_one() to use page_vma_mapped_walk()")
> Cc: stable@vger.kernel.org
> Reported-by: David Hildenbrand <david@kernel.org>
> Signed-off-by: Dev Jain <dev.jain@arm.com>
> ---
>  include/linux/hugetlb.h |  3 +++
>  mm/rmap.c               | 16 ++++++++++------
>  2 files changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
> index 2abaf99321e90..fdb7bdf7645c5 100644
> --- a/include/linux/hugetlb.h
> +++ b/include/linux/hugetlb.h
> @@ -1261,6 +1261,9 @@ static inline void hugetlb_count_sub(long l, struct mm_struct *mm)
>  {
>  }
>  
> +pte_t huge_ptep_get(struct mm_struct *mm, unsigned long addr,
> +		    pte_t *ptep);

Two tabs, or just in a single line.

If others prefer a stub, I don't care. This here is shortest to let the linker
bail out.

> +
>  static inline pte_t huge_ptep_clear_flush(struct vm_area_struct *vma,
>  					  unsigned long addr, pte_t *ptep)
>  {
> diff --git a/mm/rmap.c b/mm/rmap.c
> index 1c77d5dc06e9f..aa8a254efaecc 100644
> --- a/mm/rmap.c
> +++ b/mm/rmap.c
> @@ -2095,11 +2095,16 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
>  		/* Unexpected PMD-mapped THP? */
>  		VM_BUG_ON_FOLIO(!pvmw.pte, folio);
>  
> -		/*
> -		 * Handle PFN swap PTEs, such as device-exclusive ones, that
> -		 * actually map pages.
> -		 */

That comment now actually belongs above the pte_present() check below.


-- 
Cheers,

David

Re: [PATCH v2 2/6] mm/rmap: use huge_ptep_get() in try_to_unmap_one()

[Dev Jain]

On 02/07/26 9:02 pm, David Hildenbrand (Arm) wrote:
> On 7/2/26 07:13, Dev Jain wrote:
>> try_to_unmap_one() handles hugetlb folios when memory failure needs
>> to replace a poisoned hugetlb mapping with a hwpoison entry. In that
>> case page_vma_mapped_walk() returns the pte pointer to the hugetlb folio
>> in pvmw.pte, but the code reads it with ptep_get().
>>
>> On arches which provide their own huge_ptep_get() to dereference a huge
>> pte pointer, accessing via ptep_get() would cause pte_pfn(), pte_present()
>> etc to misbehave.
>>
>> It is not clear whether this has a trivially visible effect to userspace.
>>
>> Just use huge_ptep_get() for dereferencing a huge pte pointer.
>>
>> Fixes: c7ab0d2fdc84 ("mm: convert try_to_unmap_one() to use page_vma_mapped_walk()")
>> Cc: stable@vger.kernel.org
>> Reported-by: David Hildenbrand <david@kernel.org>
>> Signed-off-by: Dev Jain <dev.jain@arm.com>
>> ---
>>  include/linux/hugetlb.h |  3 +++
>>  mm/rmap.c               | 16 ++++++++++------
>>  2 files changed, 13 insertions(+), 6 deletions(-)
>>
>> diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
>> index 2abaf99321e90..fdb7bdf7645c5 100644
>> --- a/include/linux/hugetlb.h
>> +++ b/include/linux/hugetlb.h
>> @@ -1261,6 +1261,9 @@ static inline void hugetlb_count_sub(long l, struct mm_struct *mm)
>>  {
>>  }
>>  
>> +pte_t huge_ptep_get(struct mm_struct *mm, unsigned long addr,
>> +		    pte_t *ptep);
> 
> Two tabs, or just in a single line.
> 
> If others prefer a stub, I don't care. This here is shortest to let the linker
> bail out.
> 
>> +
>>  static inline pte_t huge_ptep_clear_flush(struct vm_area_struct *vma,
>>  					  unsigned long addr, pte_t *ptep)
>>  {
>> diff --git a/mm/rmap.c b/mm/rmap.c
>> index 1c77d5dc06e9f..aa8a254efaecc 100644
>> --- a/mm/rmap.c
>> +++ b/mm/rmap.c
>> @@ -2095,11 +2095,16 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
>>  		/* Unexpected PMD-mapped THP? */
>>  		VM_BUG_ON_FOLIO(!pvmw.pte, folio);
>>  
>> -		/*
>> -		 * Handle PFN swap PTEs, such as device-exclusive ones, that
>> -		 * actually map pages.
>> -		 */
> 
> That comment now actually belongs above the pte_present() check below.

Oops I mindlessly just copied the comment.

I will then rather put it in the else block which gets the pfn from the softleaf,
that is the most appropriate.

> 
> 

Re: [PATCH v2 2/6] mm/rmap: use huge_ptep_get() in try_to_unmap_one()

[David Hildenbrand (Arm)]

>>> diff --git a/mm/rmap.c b/mm/rmap.c
>>> index 1c77d5dc06e9f..aa8a254efaecc 100644
>>> --- a/mm/rmap.c
>>> +++ b/mm/rmap.c
>>> @@ -2095,11 +2095,16 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
>>>  		/* Unexpected PMD-mapped THP? */
>>>  		VM_BUG_ON_FOLIO(!pvmw.pte, folio);
>>>  
>>> -		/*
>>> -		 * Handle PFN swap PTEs, such as device-exclusive ones, that
>>> -		 * actually map pages.
>>> -		 */
>>
>> That comment now actually belongs above the pte_present() check below.
> 
> Oops I mindlessly just copied the comment.
> 
> I will then rather put it in the else block which gets the pfn from the softleaf,
> that is the most appropriate.

Works for me, thanks

-- 
Cheers,

David

Re: [PATCH v2 3/6] mm/rmap: use huge_ptep_get() in try_to_migrate_one()

[David Hildenbrand (Arm)]

On 7/2/26 07:13, Dev Jain wrote:
> try_to_migrate_one() is used by folio migration to replace a present
> mapping with a migration entry. For hugetlb folios, page_vma_mapped_walk()
> returns the pte pointer to the hugetlb folio in pvmw.pte, but the code
> reads the huge pte entry with ptep_get().
> 
> On arches which provide their own huge_ptep_get() to dereference a huge
> pte pointer, accessing via ptep_get() would cause pte_pfn(), pte_present()
> etc to misbehave.
> 
> It is not clear whether this has a trivially visible effect to userspace.
> 
> Use huge_ptep_get() to dereference a huge pte pointer.
> 
> Commit a98a2f0c8ce1 copied the bug from try_to_unmap_one into
> try_to_migrate_one.
> 
> Fixes: a98a2f0c8ce1 ("mm/rmap: split migration into its own function")
> Cc: stable@vger.kernel.org
> Acked-by: Muchun Song <muchun.song@linux.dev>
> Signed-off-by: Dev Jain <dev.jain@arm.com>
> ---
>  mm/rmap.c | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/mm/rmap.c b/mm/rmap.c
> index aa8a254efaecc..abc3a44baaa3d 100644
> --- a/mm/rmap.c
> +++ b/mm/rmap.c
> @@ -2505,11 +2505,16 @@ static bool try_to_migrate_one(struct folio *folio, struct vm_area_struct *vma,
>  		/* Unexpected PMD-mapped THP? */
>  		VM_BUG_ON_FOLIO(!pvmw.pte, folio);
>  
> -		/*
> -		 * Handle PFN swap PTEs, such as device-exclusive ones, that
> -		 * actually map pages.
> -		 */
> -		pteval = ptep_get(pvmw.pte);
> +		address = pvmw.address;
> +		if (folio_test_hugetlb(folio)) {
> +			pteval = huge_ptep_get(mm, address, pvmw.pte);
> +		} else {
> +			/*
> +			 * Handle PFN swap PTEs, such as device-exclusive ones,
> +			 * that actually map pages.
> +			 */

Again, the comment is related to the pte_present() handling, so best to move it
down there.

Apart from that LGTM.

-- 
Cheers,

David

Re: [PATCH v2 4/6] mm/migrate: use huge_ptep_get() in remove_migration_pte()

[David Hildenbrand (Arm)]

On 7/2/26 07:13, Dev Jain wrote:
> remove_migration_pte() converts migration entries back to present PTEs
> after folio migration completes. For hugetlb folios,
> page_vma_mapped_walk() returns the pte pointer to the hugetlb folio in
> pvmw.pte, but the code reads it with ptep_get().
> 
> On arches which provide their own huge_ptep_get() to dereference a huge
> pte pointer, accessing via ptep_get() would cause pte_pfn(),
> pte_present() etc to misbehave.
> 
> It is not clear whether this has a trivially visible effect to userspace.

Right, for non-present entries it's even weirder. Fortunately your patch #1 also
handles that.

Acked-by: David Hildenbrand (Arm) <david@kernel.org>

-- 
Cheers,

David

Re: [PATCH v2 5/6] mm/page_vma_mapped: use huge_ptep_get() for hugetlb

[David Hildenbrand (Arm)]

On 7/2/26 07:13, Dev Jain wrote:
> check_pte() is the final validation step in page_vma_mapped_walk().
> It reads pvmw->pte with ptep_get() to decide whether the entry maps
> the PFN range being walked. For hugetlb VMAs, that pointer refers
> to a hugetlb entry.
> 
> On arches which provide their own huge_ptep_get() to dereference a huge
> pte pointer, accessing via ptep_get() would cause pte_pfn(),
> pte_present() etc to misbehave.
> 
> It is not clear whether this has a trivially visible effect to userspace.
> 
> Use huge_ptep_get() to dereference a huge pte pointer.
> 
> Fixes: ace71a19cec5 ("mm: introduce page_vma_mapped_walk()")
> Cc: stable@vger.kernel.org
> Signed-off-by: Dev Jain <dev.jain@arm.com>
> ---

Acked-by: David Hildenbrand (Arm) <david@kernel.org>

-- 
Cheers,

David

Re: [PATCH v2 6/6] mm/mprotect: use huge_ptep_get() for hugetlb

[David Hildenbrand (Arm)]

On 7/2/26 07:13, Dev Jain wrote:
> prot_none_hugetlb_entry() is the hugetlb callback for the early
> mprotect(PROT_NONE) PFN permission walk on x86.
> 
> The callback passes the decoded PFN to pfn_modify_allowed(). For a
> hugetlb callback, the pte pointer refers to a hugetlb entry. On
> architectures where hugetlb entries need huge_ptep_get(), reading that
> entry with ptep_get() can make the permission check use the wrong PFN.
> 
> Use huge_ptep_get() before decoding the hugetlb PFN.
> 
> Currently there is no path which can trigger a bug: huge_ptep_get() is a
> simple ptep_get() for x86, and the prot_none walk occurs only for x86.
> 
> So no need to backport - use the correct helper anyways.
> 
> Fixes: 42e4089c7890 ("x86/speculation/l1tf: Disallow non privileged high MMIO PROT_NONE mappings")
> Signed-off-by: Dev Jain <dev.jain@arm.com>
> ---
>  mm/mprotect.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/mprotect.c b/mm/mprotect.c
> index 9cbf932b028cf..23779632d18bf 100644
> --- a/mm/mprotect.c
> +++ b/mm/mprotect.c
> @@ -699,14 +699,20 @@ static int prot_none_pte_entry(pte_t *pte, unsigned long addr,
>  		0 : -EACCES;
>  }
>  
> +#ifdef CONFIG_HUGETLB_PAGE
>  static int prot_none_hugetlb_entry(pte_t *pte, unsigned long hmask,
>  				   unsigned long addr, unsigned long next,
>  				   struct mm_walk *walk)
>  {
> -	return pfn_modify_allowed(pte_pfn(ptep_get(pte)),
> +	pte_t entry = huge_ptep_get(walk->mm, addr, pte);

Nit: can be const :)

> +
> +	return pfn_modify_allowed(pte_pfn(entry),
>  				  *(pgprot_t *)(walk->private)) ?
>  		0 : -EACCES;

Indentation is odd.

Can we just make this readable?

if (pfn_modify_allowed ...)
	return 0
return -EACCESS;


-- 
Cheers,

David

Re: [PATCH v2 6/6] mm/mprotect: use huge_ptep_get() for hugetlb

[Dev Jain]

On 02/07/26 9:17 pm, David Hildenbrand (Arm) wrote:
> On 7/2/26 07:13, Dev Jain wrote:
>> prot_none_hugetlb_entry() is the hugetlb callback for the early
>> mprotect(PROT_NONE) PFN permission walk on x86.
>>
>> The callback passes the decoded PFN to pfn_modify_allowed(). For a
>> hugetlb callback, the pte pointer refers to a hugetlb entry. On
>> architectures where hugetlb entries need huge_ptep_get(), reading that
>> entry with ptep_get() can make the permission check use the wrong PFN.
>>
>> Use huge_ptep_get() before decoding the hugetlb PFN.
>>
>> Currently there is no path which can trigger a bug: huge_ptep_get() is a
>> simple ptep_get() for x86, and the prot_none walk occurs only for x86.
>>
>> So no need to backport - use the correct helper anyways.
>>
>> Fixes: 42e4089c7890 ("x86/speculation/l1tf: Disallow non privileged high MMIO PROT_NONE mappings")
>> Signed-off-by: Dev Jain <dev.jain@arm.com>
>> ---
>>  mm/mprotect.c | 8 +++++++-
>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/mm/mprotect.c b/mm/mprotect.c
>> index 9cbf932b028cf..23779632d18bf 100644
>> --- a/mm/mprotect.c
>> +++ b/mm/mprotect.c
>> @@ -699,14 +699,20 @@ static int prot_none_pte_entry(pte_t *pte, unsigned long addr,
>>  		0 : -EACCES;
>>  }
>>  
>> +#ifdef CONFIG_HUGETLB_PAGE
>>  static int prot_none_hugetlb_entry(pte_t *pte, unsigned long hmask,
>>  				   unsigned long addr, unsigned long next,
>>  				   struct mm_walk *walk)
>>  {
>> -	return pfn_modify_allowed(pte_pfn(ptep_get(pte)),
>> +	pte_t entry = huge_ptep_get(walk->mm, addr, pte);
> 
> Nit: can be const :)
> 
>> +
>> +	return pfn_modify_allowed(pte_pfn(entry),
>>  				  *(pgprot_t *)(walk->private)) ?
>>  		0 : -EACCES;
> 
> Indentation is odd.
> 
> Can we just make this readable?
> 
> if (pfn_modify_allowed ...)
> 	return 0
> return -EACCESS;
> 

Sure I'll do this.


>