Subject: Re: [PATCH] arm64: add prctl(PR_PAC_SET_ENABLED_KEYS)

From: Dave Martin <[email-masked]>
Date: Mon, 24 Aug 2020 15:55:30 +0100

On Wed, Aug 19, 2020 at 02:25:45PM -0700, Peter Collingbourne wrote:
> On Wed, Aug 19, 2020 at 3:18 AM Dave Martin <[email-masked]> wrote:
> >
> > On Fri, Jul 31, 2020 at 06:11:52PM -0700, Peter Collingbourne wrote:
> > > This prctl allows the user program to control which PAC keys are enabled
> > > in a particular task. The main reason why this is useful is to enable a
> > > userspace ABI that uses PAC to sign and authenticate function pointers
> > > and other pointers exposed outside of the function, while still allowing
> > > binaries conforming to the ABI to interoperate with legacy binaries that
> > > do not sign or authenticate pointers.
> > >
> > > The idea is that a dynamic loader or early startup code would issue
> > > this prctl very early after establishing that a process may load legacy
> > > binaries, but before executing any PAC instructions.
> >
> > Apologies for the slow response on this, I'd had it on my list for a
> > while...

A couple more comments I accidentally left out of my last reply, below.

> >
> > > ---
> > >  .../arm64/pointer-authentication.rst          | 27 +++++++++++++++
> > >  arch/arm64/include/asm/asm_pointer_auth.h     | 19 +++++++++++
> > >  arch/arm64/include/asm/pointer_auth.h         | 10 ++++--
> > >  arch/arm64/include/asm/processor.h            |  5 +++
> > >  arch/arm64/kernel/asm-offsets.c               |  1 +
> > >  arch/arm64/kernel/pointer_auth.c              | 34 +++++++++++++++++++
> > >  include/uapi/linux/prctl.h                    |  3 ++
> > >  kernel/sys.c                                  |  8 +++++
> > >  8 files changed, 105 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/Documentation/arm64/pointer-authentication.rst b/Documentation/arm64/pointer-authentication.rst
> > > index 30b2ab06526b..1f7e064deeb3 100644
> > > --- a/Documentation/arm64/pointer-authentication.rst
> > > +++ b/Documentation/arm64/pointer-authentication.rst
> > > @@ -107,3 +107,30 @@ filter out the Pointer Authentication system key registers from
> > >  KVM_GET/SET_REG_* ioctls and mask those features from cpufeature ID
> > >  register. Any attempt to use the Pointer Authentication instructions will
> > >  result in an UNDEFINED exception being injected into the guest.
> > > +
> > > +
> > > +Enabling and disabling keys
> > > +---------------------------
> > > +
> > > +The prctl PR_PAC_SET_ENABLED_KEYS allows the user program to control which
> > > +PAC keys are enabled in a particular task. It takes two arguments, the
> > > +first being a bitmask of PR_PAC_APIAKEY, PR_PAC_APIBKEY, PR_PAC_APDAKEY
> > > +and PR_PAC_APDBKEY specifying which keys shall be affected by this prctl,
> > > +and the second being a bitmask of the same bits specifying whether the key
> > > +should be enabled or disabled. For example::
> > > +
> > > +  prctl(PR_PAC_SET_ENABLED_KEYS,
> > > +        PR_PAC_APIAKEY | PR_PAC_APIBKEY | PR_PAC_APDAKEY | PR_PAC_APDBKEY,
> > > +        PR_PAC_APIBKEY, 0, 0);
> > > +
> > > +disables all keys except the IB key.
> > > +
> > > +The main reason why this is useful is to enable a userspace ABI that uses PAC
> > > +instructions to sign and authenticate function pointers and other pointers
> > > +exposed outside of the function, while still allowing binaries conforming to
> > > +the ABI to interoperate with legacy binaries that do not sign or authenticate
> > > +pointers.
> >
> > What actually breaks without this?
> >
> > Since the keys are all enabled by default, the only purpose of this
> > prctl seems to be to disable keys.  I'm not sure what this is giving us.
> 
> Yes, the purpose is to disable keys. Let's consider the function
> pointer signing userspace ABI use case. An example is Apple's arm64e
> ABI, and I have a prototype branch of LLVM [0] that implements a
> similar ABI in Linux userspace, based on Apple's implementation of
> their ABI.
> 
> Here's an example of a function that returns a function pointer, and a
> function that calls a function pointer of the same type:
> 
> static void f(void) {}
> 
> void *return_fp(void) {
>   return f;
> }
> 
> void call_fp(void (*p)(void)) {
>   p();
> }
> 
> If I compile this with my prototype compiler I get:
> 
> $ clang --target=aarch64-android   -fptrauth-calls  fptr.c -S -o - -O3
> -march=armv8.3a
> [...]
> return_fp:                              // @return_fp
> // %bb.0:                               // %entry
>         adrp    x16, f
>         add     x16, x16, :lo12:f
>         mov     x17, #16277
>         pacia   x16, x17
>         mov     x0, x16
>         ret
> [...]
> call_fp:                                // @call_fp
> // %bb.0:                               // %entry
>         mov     w1, #16277
>         braa    x0, x1
> [...]
> 
> In this code snippet the function pointer is signed with the IA key
> and discriminator 16277 before being returned. When the function is
> called, the pointer is first authenticated with the same key and
> discriminator.
> 
> Now imagine that this code lives in a shared library used both by
> programs that use the function pointer signing ABI and by legacy
> binaries (i.e. programs that use the existing ABI), and we have a
> legacy binary that calls return_fp. If the legacy binary then calls
> the function pointer returned by return_fp, that code will not
> authenticate the pointer before calling it, it will just use a br or
> blr instruction to call it directly, which will lead to a crash if the
> signature bits are set in the function pointer. In order to prevent
> the crash, we need a way to cause the pacia instruction in return_fp
> to become a no-op when running inside the process hosting the legacy
> binary, so that the signature bits will remain clear and the br or blr
> instruction in the legacy binary will successfully call the function
> f. That can be done by disabling the IA key, which is exactly what
> this prctl() lets us do.

Ack, I think there has been past discussion on this, but it's been
waiting for a usecase since it does impose a bit of extra overhead.

(I'd somehow assumes that you were actually wanting to get SIGILLs,
rather then compatibly executing some ptrauth insns as NOPs -- the
latter makes much more sense.)

Is this going to land for real, or is it just something people have
been experimenting with?

(I can guess the answer from the fact that you've proposed this patch
in the first place, but I have to ask...)

[...]

Cheers
---Dave

_______________________________________________
linux-arm-kernel mailing list
[email-masked]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel