Re: [PATCH] powerpc/mm: Support execute-only memory on the Radix MMU

[Russell Currey <ruscur@russell.cc>]

On Mon, 2022-08-08 at 18:28 +0530, Aneesh Kumar K V wrote:
> On 8/8/22 5:28 PM, Russell Currey wrote:
> > The Hash MMU already supports XOM (i.e. mmap with PROT_EXEC only)
> > through the execute-only pkey.  A PROT_ONLY mapping will actually
> > map to
> > RX, and then the pkey will be applied on top of it.
> > 
> > Radix doesn't have pkeys, but it does have execute permissions
> > built-in
> > to the MMU, so all we have to do to support XOM is expose it.
> > 
> > Signed-off-by: Russell Currey <ruscur@russell.cc>
> > ---
> > quick test:
> > https://raw.githubusercontent.com/ruscur/junkcode/main/mmap_test.c
> > I can make it a selftest.
> > 
> >  arch/powerpc/include/asm/book3s/64/radix.h |  3 +++
> >  arch/powerpc/mm/book3s64/radix_pgtable.c   |  4 ++++
> >  arch/powerpc/mm/fault.c                    | 10 ++++++++++
> >  3 files changed, 17 insertions(+)
> > 
> > diff --git a/arch/powerpc/include/asm/book3s/64/radix.h
> > b/arch/powerpc/include/asm/book3s/64/radix.h
> > index 686001eda936..bf316b773d73 100644
> > --- a/arch/powerpc/include/asm/book3s/64/radix.h
> > +++ b/arch/powerpc/include/asm/book3s/64/radix.h
> > @@ -19,6 +19,9 @@
> >  #include <asm/cpu_has_feature.h>
> >  #endif
> >  
> > +/* Execute-only page protections, Hash can use RX + execute-only
> > pkey */
> > +#define PAGE_EXECONLY  __pgprot(_PAGE_BASE | _PAGE_EXEC)
> > +
> >  /* An empty PTE can still have a R or C writeback */
> >  #define RADIX_PTE_NONE_MASK            (_PAGE_DIRTY |
> > _PAGE_ACCESSED)
> >  
> > diff --git a/arch/powerpc/mm/book3s64/radix_pgtable.c
> > b/arch/powerpc/mm/book3s64/radix_pgtable.c
> > index 698274109c91..2edb56169805 100644
> > --- a/arch/powerpc/mm/book3s64/radix_pgtable.c
> > +++ b/arch/powerpc/mm/book3s64/radix_pgtable.c
> > @@ -617,6 +617,10 @@ void __init radix__early_init_mmu(void)
> >         __pmd_frag_nr = RADIX_PMD_FRAG_NR;
> >         __pmd_frag_size_shift = RADIX_PMD_FRAG_SIZE_SHIFT;
> >  
> > +       /* Radix directly supports execute-only page protections */
> > +       protection_map[VM_EXEC] = PAGE_EXECONLY;
> > +       protection_map[VM_EXEC | VM_SHARED] = PAGE_EXECONLY;
> > +
> >         radix_init_pgtable();
> >  
> >         if (!firmware_has_feature(FW_FEATURE_LPAR)) {
> > diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
> > index 014005428687..887c0cc45ca6 100644
> > --- a/arch/powerpc/mm/fault.c
> > +++ b/arch/powerpc/mm/fault.c
> > @@ -270,6 +270,16 @@ static bool access_error(bool is_write, bool
> > is_exec, struct vm_area_struct *vma
> >                 return false;
> >         }
> >  
> > +       if (unlikely(!(vma->vm_flags & VM_READ))) {
> > +               /*
> > +                * If we're on Radix, then this could be a read
> > attempt on
> > +                * execute-only memory.  On other MMUs, an "exec-
> > only" page
> > +                * will be given RX flags, so this might be
> > redundant.
> > +                */
> > +               if (radix_enabled())
> > +                       return true;
> > +       }
> > +
> 
> 
> should we do 
> 
> /* This cover both PROT_NONE (due to check above) and exec only
> mapping */
> if (radix_enabled() && !(vma->vm_flags & VM_READ)) {
>     return true;
> /* PROT_NONE check */
> else if (!vma_is_accessible(vma)) 
>    return true;
> 
> return false;

That is better, thanks.

- Russell

> 
> 
> 
> >         if (unlikely(!vma_is_accessible(vma)))
> >                 return true;
> >         /*
> 
> -aneesh