Re: [PATCH 2/2] arm64: Configure kernel's PTR_AUTH key when it is built with PTR_AUTH.

[Will Deacon <will@kernel.org>]

On Tue, Dec 08, 2020 at 11:33:33AM -0800, Peter Collingbourne wrote:
> On Tue, Dec 8, 2020 at 3:00 AM Catalin Marinas <catalin.marinas@arm.com> wrote:
> >
> > On Mon, Dec 07, 2020 at 03:07:07PM -0800, Peter Collingbourne wrote:
> > > On Mon, Dec 7, 2020 at 2:46 PM Daniel Kiss <daniel.kiss@arm.com> wrote:
> > > > If the kernel is not compiled with CONFIG_ARM64_PTR_AUTH_KERNEL,
> > > > then the kernel does not need a key and kernel's key could be disabled.
> > > >
> > > > Signed-off-by: Daniel Kiss <daniel.kiss@arm.com>
> > > > ---
> > > >  arch/arm64/include/asm/asm_pointer_auth.h | 68 ++++++++++++++++-------
> > > >  arch/arm64/include/asm/processor.h        |  2 +
> > > >  arch/arm64/kernel/asm-offsets.c           |  4 ++
> > > >  3 files changed, 55 insertions(+), 19 deletions(-)
> > > >
> > > > diff --git a/arch/arm64/include/asm/asm_pointer_auth.h b/arch/arm64/include/asm/asm_pointer_auth.h
> > > > index 52dead2a8640..af3d16027e8f 100644
> > > > --- a/arch/arm64/include/asm/asm_pointer_auth.h
> > > > +++ b/arch/arm64/include/asm/asm_pointer_auth.h
> > > > @@ -14,6 +14,12 @@
> > > >   * thread.keys_user.ap*.
> > > >   */
> > > >         .macro ptrauth_keys_install_user tsk, tmp1, tmp2, tmp3
> > > > +#ifndef CONFIG_ARM64_PTR_AUTH_KERNEL
> > > > +       /* Reenable A key */
> > > > +       mrs     \tmp1, sctlr_el1
> > > > +       orr     \tmp1, \tmp1, SCTLR_ELx_ENIA
> > > > +       msr     sctlr_el1, \tmp1
> > > > +#endif
> > >
> > > We should avoid an unconditional MSR on exit like this as it is
> > > expensive (for my PR_PAC_SET_ENABLED_KEYS series I measured the cost
> > > of entry/exit MSR as 43.7ns on Cortex-A75 and 33.0ns on Apple M1). In
> > > that series I take care not to touch SCTLR_EL1 unless necessary.
> > > Likewise for the MSRs on entry below.
> >
> > I think that's how Daniel attempted the first (internal) version of
> > these patches. In theory you don't need to touch SCTLR_ELx_EN* at all as
> > long as the kernel does not use any PAC instructions. However, I was
> > a bit concerned about this and thought it's safer if, when
> > !CONFIG_ARM64_PTR_AUTH_KERNEL, the EnIA bit is cleared while in the
> > kernel.
> >
> > If we can guarantee that the compiler does not generate any PAC
> > instructions (it may assume they are no-ops) and vendor modules don't
> > have such instructions either, we may be able to relax this.
> 
> The way I see it it isn't too different from the current prohibition
> on using IB in the kernel (and to a lesser extent DA/DB/GA since those
> can't be accessed from nop-space as far as I'm aware), or NEON
> instructions in most parts of the kernel, or the stack protector
> cookie when building with -fno-stack-protector etc. i.e. if you do
> that then you're breaking the ABI.
> 
> Is your concern that distributions may default to enabling
> -mbranch-protection which would result in the PAC instructions being
> used? To address that I think it is reasonable to expect the compiler
> not to use PAC instructions when passing -mbranch-protection=none, and
> if the compiler does so then that is a bug in the compiler.

I'm inclined to agree. At the very least, I think we should start from a
position where we assume the compiler doesn't randomly emit these
instructions, and then we can revisit that decision in future if it turns
out to be wrong.

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel