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

[Lance Yang]

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 :)

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

Cheers, Lance