Re: [PATCH] arm64/mm: restrict transparent contpte mappings to valid PTEs

[Baolin Wang <baolin.wang@linux.alibaba.com>]

On 10/14/26 5:24 PM, Lance Yang wrote:
> 
> 
> On 2026/10/14 16:38, Baolin Wang wrote:
>>
>>
>> On 10/14/26 2:46 PM, Lance Yang wrote:
>>>
>>>
>>> On 2026/10/14 14:31, Zhao Gongyi wrote:
>>>> On 10/13/26 11:39 PM, Lance Yang wrote:
>>>>>>
>>>>>> +    /*
>>>>>> +     * Keep the producer-side invariant documented in pgtable.h 
>>>>>> true even
>>>>>> +     * if a direct caller passes an invalid descriptor with 
>>>>>> PTE_CONT set.
>>>>>> +     */
>>>>>> +    if (!pte_valid(pte))
>>>>>> +        return __set_ptes(mm, addr, ptep, pte_mknoncont(pte), nr);
>>>>>
>>>>> set_ptes() is the only in-tree caller, and it already clears PTE_CONT
>>>>> before we get here ... no? Why not fold !pte_valid(pte) into the
>>>>> !mm_is_user(mm) check below and return __set_ptes() directly?
>>>>>
>>>>>     if (!mm_is_user(mm) || !pte_valid(pte))
>>>>>         return __set_ptes(mm, addr, ptep, pte, nr);
>>>>>
>>>>> If a direct caller does pass a !pte_valid PTE with PTE_CONT set, I'd
>>>>> rather add a VM_WARN_ON() to catch that. Silently clearing the bit 
>>>>> would
>>>>> hide a caller bug :)
>>>>>
>>>>
>>>> Hi Lance,
>>>>
>>>> Thanks for taking a look.
>>>
>>> Cheers!
>>>
>>>> Yes, you are right. The public set_ptes() wrapper already strips 
>>>> PTE_CONT
>>>> before calling contpte_set_ptes(), so the pte_mknoncont() in the 
>>>> invalid
>>>> path is unnecessary for the current in-tree path.
>>>>
>>>> The issue I was trying to avoid is not that the incoming pte already 
>>>> has
>>>> PTE_CONT set, but that contpte_set_ptes() rebuilds the pte from pfn/ 
>>>> prot
>>>> and then adds PTE_CONT again based on alignment. For an invalid 
>>>> descriptor,
>>>> that transparent contiguous encoding should be skipped entirely.
>>>>
>>>> So your suggested shape makes sense to me. I will change this to return
>>>> through __set_ptes() directly for invalid descriptors, and add a 
>>>> warning
>>>> for any unexpected direct caller that passes an invalid PTE with 
>>>> PTE_CONT
>>>> set, e.g.:
>>>>
>>>>     if (!mm_is_user(mm) || !pte_valid(pte)) {
>>>>         VM_WARN_ON_ONCE(!pte_valid(pte) && pte_cont(pte));
>>
>> mprotect() can trigger the warning here when setting PAGE_NONE for a 
>> batch of PTEs?
> 
> Nope. set_ptes() clears PTE_CONT before calling contpte_set_ptes().
> PAGE_NONE makes !pte_valid(pte) true, but pte_cont(pte) is false,
> so the warning won't fire :)

Ah, right.

>>>>         return __set_ptes(mm, addr, ptep, pte, nr);
>>
>> I am trying to convince myself that the 'nr' passed in is always 
>> CONT_PTE_SIZE aligned, but it seems OK.
> 
> Still nope :)
> 
> IIRC, any partially covered contpte block should already be unfolded
> by the time we get here ...
> 
> We're just installing PTEs without PTE_CONT into cleared entries,
> so nr doesn't need to be aligned to CONT_PTES.

OK. I see. Thanks for explanation. So looks good to me.

Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>