[PATCH] arm64/mm: Sanity check PTE address before runtime P4D/PUD folding

[Ard Biesheuvel]

On Mon, 4 Nov 2024 at 14:50, Catalin Marinas <catalin.marinas at arm.com> wrote:
>
> On Fri, Nov 01, 2024 at 04:58:01PM +0100, Ard Biesheuvel wrote:
> > diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
> > index dd5dcf7ae056..0d729adf894c 100644
> > --- a/arch/arm64/include/asm/pgtable.h
> > +++ b/arch/arm64/include/asm/pgtable.h
> > @@ -740,6 +740,11 @@ static inline bool pud_table(pud_t pud) { return true; }
> >                                PUD_TYPE_TABLE)
> >  #endif
> >
> > +static inline long sign_of(unsigned long addr)
> > +{
> > +     return (int)(addr >> 24) >> 31L; // bit 55 is the sign bit
> > +}
>
> That's a pretty generic name that trickles into the core code. It should
> be renamed to something that suggests arm64 addresses (and maybe some
> underscores to imply private).

Fair enough.

> Also, this assumes untagged addresses but
> I haven't checked whether that's the case for all call sites.
>

Isn't bit #55 always guaranteed to be the sign bit?

> >  extern pgd_t init_pg_dir[];
> >  extern pgd_t init_pg_end[];
> >  extern pgd_t swapper_pg_dir[];
> > @@ -932,6 +937,8 @@ static inline phys_addr_t p4d_page_paddr(p4d_t p4d)
> >
> >  static inline pud_t *p4d_to_folded_pud(p4d_t *p4dp, unsigned long addr)
> >  {
> > +     VM_BUG_ON(((u64)p4dp / sizeof(p4d_t) - sign_of(addr)) % PTRS_PER_P4D);
> > +
> >       return (pud_t *)PTR_ALIGN_DOWN(p4dp, PAGE_SIZE) + pud_index(addr);
> >  }
>
> I think I get it but please add a comment in the code, otherwise in a
> week time I'll wonder what this is.
>
> Even better if it could be written in a less cryptic way ;).
>

Yeah, I tried to keep it concise so the code for the VM_BUG_ON()
doesn't bloat the function.