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

[Dev Jain <dev.jain@arm.com>]

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.


>