> Date: Fri, 14 Aug 2020 12:29:51 +1000
> From: Jonathan Gray <j...@jsg.id.au>
> 
> On Thu, Aug 13, 2020 at 09:17:41PM +0200, Mark Kettenis wrote:
> > ARMv8.1 introduced PAN (Priviliged Access Never) which prevents the
> > kernel from accessing userland data.  This can be bypassed by using
> > special instructions which we already use in copyin(9) and friends.
> > So we can simply turn this feature on if the CPU supports it.
> > 
> > Tested on an Odroid-C4 which has Cortex-A55 cores that have PAN
> > support.
> > 
> > ok?
> 
> This should be changing PSTATE.PAN as well.  Can you force an
> acess of this type to be sure the permission fault occurs?
> 
> >From the Arm ARM:
> 
> "When the value of PSTATE.PAN is 1, any privileged data access from
> EL1, or EL2 when HCR_EL2.E2H is 1, to a virtual memory address that
> is accessible at EL0, generates a Permission fault.
> 
> When the value of PSTATE.PAN is 0, the translation system is the
> same as in Armv8.0.
> 
> When ARMv8.1-PAN is implemented, the SPSR_EL1.PAN, SPSR_EL2.PAN, and
> SPSR_EL3.PAN bits are used for exception returns, and the DSPSR_EL0
> register is used for entry to or exit from Debug state.
> 
> When ARMv8.1-PAN is implemented, the SCTLR_EL1.SPAN and SCTLR_EL2.SPAN
> bits are used to control whether the PAN bit is set on an exception
> to EL1 or EL2."

By clearing SCTRL_EL0.SPAN, PSTATE.PAN will be set on an exception to
EL1.  Yes, this does mean that PSTATE.PAN won't be set until a CPU
returns to userland for the first time.  But from then on PSTATE.PAN
will be set.

I suppose a way to test this properly is to pick a system call and
replace a copyin() with a direct access?  That will succeed without
PAN but should fail with PAN enabled right?

> > Index: arch/arm64/arm64/cpu.c
> > ===================================================================
> > RCS file: /cvs/src/sys/arch/arm64/arm64/cpu.c,v
> > retrieving revision 1.38
> > diff -u -p -r1.38 cpu.c
> > --- arch/arm64/arm64/cpu.c  4 Jun 2020 21:18:16 -0000       1.38
> > +++ arch/arm64/arm64/cpu.c  13 Aug 2020 19:12:30 -0000
> > @@ -321,6 +321,7 @@ cpu_attach(struct device *parent, struct
> >     struct fdt_attach_args *faa = aux;
> >     struct cpu_info *ci;
> >     uint64_t mpidr = READ_SPECIALREG(mpidr_el1);
> > +   uint64_t id_aa64mmfr1, sctlr;
> >     uint32_t opp;
> >  
> >     KASSERT(faa->fa_nreg > 0);
> > @@ -393,6 +394,14 @@ cpu_attach(struct device *parent, struct
> >                     cpu_cpuspeed = cpu_clockspeed;
> >             }
> >  
> > +           /* Enable PAN. */
> > +           id_aa64mmfr1 = READ_SPECIALREG(id_aa64mmfr1_el1);
> > +           if (ID_AA64MMFR1_PAN(id_aa64mmfr1) != ID_AA64MMFR1_PAN_NONE) {
> > +                   sctlr = READ_SPECIALREG(sctlr_el1);
> > +                   sctlr &= ~SCTLR_SPAN;
> > +                   WRITE_SPECIALREG(sctlr_el1, sctlr);
> > +           }
> > +
> >             /* Initialize debug registers. */
> >             WRITE_SPECIALREG(mdscr_el1, DBG_MDSCR_TDCC);
> >             WRITE_SPECIALREG(oslar_el1, 0);
> > @@ -522,6 +531,7 @@ cpu_boot_secondary(struct cpu_info *ci)
> >  void
> >  cpu_start_secondary(struct cpu_info *ci)
> >  {
> > +   uint64_t id_aa64mmfr1, sctlr;
> >     uint64_t tcr;
> >     int s;
> >  
> > @@ -543,6 +553,14 @@ cpu_start_secondary(struct cpu_info *ci)
> >     tcr |= TCR_T0SZ(64 - USER_SPACE_BITS);
> >     tcr |= TCR_A1;
> >     WRITE_SPECIALREG(tcr_el1, tcr);
> > +
> > +   /* Enable PAN. */
> > +   id_aa64mmfr1 = READ_SPECIALREG(id_aa64mmfr1_el1);
> > +   if (ID_AA64MMFR1_PAN(id_aa64mmfr1) != ID_AA64MMFR1_PAN_NONE) {
> > +           sctlr = READ_SPECIALREG(sctlr_el1);
> > +           sctlr &= ~SCTLR_SPAN;
> > +           WRITE_SPECIALREG(sctlr_el1, sctlr);
> > +   }
> >  
> >     /* Initialize debug registers. */
> >     WRITE_SPECIALREG(mdscr_el1, DBG_MDSCR_TDCC);
> > Index: arch/arm64/include/armreg.h
> > ===================================================================
> > RCS file: /cvs/src/sys/arch/arm64/include/armreg.h,v
> > retrieving revision 1.11
> > diff -u -p -r1.11 armreg.h
> > --- arch/arm64/include/armreg.h     5 Jun 2020 22:14:25 -0000       1.11
> > +++ arch/arm64/include/armreg.h     13 Aug 2020 19:12:30 -0000
> > @@ -451,6 +451,7 @@
> >  #define    SCTLR_nTWI      0x00010000
> >  #define    SCTLR_nTWE      0x00040000
> >  #define    SCTLR_WXN       0x00080000
> > +#define    SCTLR_SPAN      0x00800000
> >  #define    SCTLR_EOE       0x01000000
> >  #define    SCTLR_EE        0x02000000
> >  #define    SCTLR_UCI       0x04000000
> > @@ -478,6 +479,7 @@
> >  #define    PSR_D           0x00000200
> >  #define    PSR_IL          0x00100000
> >  #define    PSR_SS          0x00200000
> > +#define    PSR_PAN         0x00400000
> >  #define    PSR_V           0x10000000
> >  #define    PSR_C           0x20000000
> >  #define    PSR_Z           0x40000000
> > 
> > 
> 

Reply via email to