2021-05-21 06:04:44

by Derrick McKee

Subject: [PATCH] Ensure kernel AI key is not changed on fork

The kernel uses the IA key for PAC signing,
and this key should remain unchanged from the kernel point of view.
This patch ensures that the IA key remains constant on fork,
if it has been previously set.
The software is provided on an as-is basis.

Signed-off-by: Derrick McKee <[email protected]>
Signed-off-by: Yianni Giannaris <[email protected]>
---
arch/arm64/include/asm/pointer_auth.h | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/include/asm/pointer_auth.h b/arch/arm64/include/asm/pointer_auth.h
index d50416be99be..9748413e72fd 100644
--- a/arch/arm64/include/asm/pointer_auth.h
+++ b/arch/arm64/include/asm/pointer_auth.h
@@ -69,10 +69,13 @@ static inline void ptrauth_keys_init_user(struct ptrauth_keys_user *keys)
ptrauth_keys_install_user(keys);
}

-static __always_inline void ptrauth_keys_init_kernel(struct ptrauth_keys_kernel *keys)
+static __always_inline void
+ptrauth_keys_init_kernel(struct ptrauth_keys_kernel *keys)
{
- if (system_supports_address_auth())
- get_random_bytes(&keys->apia, sizeof(keys->apia));
+ if (keys->apia.lo == 0 && keys->apia.hi == 0) {
+ if (system_supports_address_auth())
+ get_random_bytes(&keys->apia, sizeof(keys->apia));
+ }
}

static __always_inline void ptrauth_keys_switch_kernel(struct ptrauth_keys_kernel *keys)
--
2.31.1

2021-05-21 06:47:32

by Derrick McKee

Subject: Re: [PATCH] Ensure kernel AI key is not changed on fork

On Thu, May 20, 2021 at 12:00 PM Mark Rutland <[email protected]> wrote:

> On the kernel side, we use a unique IA key per kernel thread, and while
> this must remain the same *for that kernel thread*, the kernel IA key
> should differ across kernel threads when a fork() occurs.

Thank you for the clarification.

> I think you're trying to use the keys in a different way than upstream
> intends to, and we do not need this change as-is.
>
> So NAK to this patch as it stands.

Given the above discussion, I agree with the NAK.

>
> > ---
> > arch/arm64/include/asm/pointer_auth.h | 9 ++++++---
> > 1 file changed, 6 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/arm64/include/asm/pointer_auth.h b/arch/arm64/include/asm/pointer_auth.h
> > index d50416be99be..9748413e72fd 100644
> > --- a/arch/arm64/include/asm/pointer_auth.h
> > +++ b/arch/arm64/include/asm/pointer_auth.h
> > @@ -69,10 +69,13 @@ static inline void ptrauth_keys_init_user(struct ptrauth_keys_user *keys)
> > ptrauth_keys_install_user(keys);
> > }
> >
> > -static __always_inline void ptrauth_keys_init_kernel(struct ptrauth_keys_kernel *keys)
> > +static __always_inline void
> > +ptrauth_keys_init_kernel(struct ptrauth_keys_kernel *keys)
> > {
> > - if (system_supports_address_auth())
> > - get_random_bytes(&keys->apia, sizeof(keys->apia));
> > + if (keys->apia.lo == 0 && keys->apia.hi == 0) {
> > + if (system_supports_address_auth())
> > + get_random_bytes(&keys->apia, sizeof(keys->apia));
> > + }
> > }
> >
> > static __always_inline void ptrauth_keys_switch_kernel(struct ptrauth_keys_kernel *keys)
> > --
> > 2.31.1
> >



--
Derrick McKee
Phone: (703) 957-9362
Email: [email protected]

2021-05-21 15:04:22

by Mark Rutland

Subject: Re: [PATCH] Ensure kernel AI key is not changed on fork

On Thu, May 20, 2021 at 11:18:54AM -0400, Derrick McKee wrote:
> The kernel uses the IA key for PAC signing,
> and this key should remain unchanged from the kernel point of view.
> This patch ensures that the IA key remains constant on fork,
> if it has been previously set.
> The software is provided on an as-is basis.
>
> Signed-off-by: Derrick McKee <[email protected]>
> Signed-off-by: Yianni Giannaris <[email protected]>

On the kernel side, we use a unique IA key per kernel thread, and while
this must remain the same *for that kernel thread*, the kernel IA key
should differ across kernel threads when a fork() occurs.

I think you're trying to use the keys in a different way than upstream
intends to, and we do not need this change as-is.

So NAK to this patch as it stands.

Thanks,
Mark.

> ---
> arch/arm64/include/asm/pointer_auth.h | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm64/include/asm/pointer_auth.h b/arch/arm64/include/asm/pointer_auth.h
> index d50416be99be..9748413e72fd 100644
> --- a/arch/arm64/include/asm/pointer_auth.h
> +++ b/arch/arm64/include/asm/pointer_auth.h
> @@ -69,10 +69,13 @@ static inline void ptrauth_keys_init_user(struct ptrauth_keys_user *keys)
> ptrauth_keys_install_user(keys);
> }
>
> -static __always_inline void ptrauth_keys_init_kernel(struct ptrauth_keys_kernel *keys)
> +static __always_inline void
> +ptrauth_keys_init_kernel(struct ptrauth_keys_kernel *keys)
> {
> - if (system_supports_address_auth())
> - get_random_bytes(&keys->apia, sizeof(keys->apia));
> + if (keys->apia.lo == 0 && keys->apia.hi == 0) {
> + if (system_supports_address_auth())
> + get_random_bytes(&keys->apia, sizeof(keys->apia));
> + }
> }
>
> static __always_inline void ptrauth_keys_switch_kernel(struct ptrauth_keys_kernel *keys)
> --
> 2.31.1
>