* PAC key changes when kernel code is preempted @ 2021-04-30 14:40 Derrick McKee 2021-04-30 15:04 ` Mark Rutland 0 siblings, 1 reply; 9+ messages in thread From: Derrick McKee @ 2021-04-30 14:40 UTC (permalink / raw) To: Yianni Giannaris, Linux ARM, mark.rutland, suzuki.poulose, Catalin Marinas, james.morse, amit.kachhap, will Hi, I am noticing that when kernel code is preempted, PAC keys seem to change when resuming execution. For instance, when I read APDAKeyHi_EL1 and APDAKeyLo_EL1, sleep, and read them again, the values are different. Is this the intended behavior? If so, how can I ensure that the keys do not change? The different keys are causing PAC authentication to fail on pointers signed using the stale key. Thanks. -- Derrick McKee Email: derrick.mckee@gmail.com _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: PAC key changes when kernel code is preempted 2021-04-30 14:40 PAC key changes when kernel code is preempted Derrick McKee @ 2021-04-30 15:04 ` Mark Rutland 2021-05-07 20:24 ` Derrick McKee 2021-05-20 15:18 ` Derrick McKee 0 siblings, 2 replies; 9+ messages in thread From: Mark Rutland @ 2021-04-30 15:04 UTC (permalink / raw) To: Derrick McKee Cc: Yianni Giannaris, Linux ARM, suzuki.poulose, Catalin Marinas, james.morse, amit.kachhap, will On Fri, Apr 30, 2021 at 10:40:04AM -0400, Derrick McKee wrote: > Hi, > > I am noticing that when kernel code is preempted, PAC keys seem to > change when resuming execution. For instance, when I read > APDAKeyHi_EL1 and APDAKeyLo_EL1, sleep, and read them again, the > values are different. Is this the intended behavior? This is expected; kernel-side we only use the IA keys (which should stay the same from a kernel task's PoV), and the other keys (IB, DA, DB, GA) are not supposed to be used within the kernel. Up to and including v5.12, the other keys are switched at entry to/from userspace, and so may change from the PoV of a kernel thread across preemption. With the patches merged for v5.13, the other keys will be switched with the task, but userspace can reset these at any time, and they are still not supposed to be used within the kernel. > If so, how can I ensure that the keys do not change? The different > keys are causing PAC authentication to fail on pointers signed using > the stale key. Thanks. I take it this is non-mainline code? We shouldn't be using the other keys today. If you want to use the other keys, you'll need to alter the context-switch and userspace entry/exit logic to have kernel versions of the other keys, and switch them at the same points we switch the IA keys. For reference, see commit: b90e483938ce387c ("arm64: pac: Optimize kernel entry/exit key installation code paths") Thank, Mark. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: PAC key changes when kernel code is preempted 2021-04-30 15:04 ` Mark Rutland @ 2021-05-07 20:24 ` Derrick McKee 2021-05-20 15:18 ` Derrick McKee 1 sibling, 0 replies; 9+ messages in thread From: Derrick McKee @ 2021-05-07 20:24 UTC (permalink / raw) To: Mark Rutland Cc: Yianni Giannaris, Linux ARM, suzuki.poulose, Catalin Marinas, james.morse, amit.kachhap, will On Fri, Apr 30, 2021 at 11:04 AM Mark Rutland <mark.rutland@arm.com> wrote: > > On Fri, Apr 30, 2021 at 10:40:04AM -0400, Derrick McKee wrote: > > Hi, > > > > I am noticing that when kernel code is preempted, PAC keys seem to > > change when resuming execution. For instance, when I read > > APDAKeyHi_EL1 and APDAKeyLo_EL1, sleep, and read them again, the > > values are different. Is this the intended behavior? > > This is expected; kernel-side we only use the IA keys (which should stay > the same from a kernel task's PoV), and the other keys (IB, DA, DB, GA) > are not supposed to be used within the kernel. > > Up to and including v5.12, the other keys are switched at entry to/from > userspace, and so may change from the PoV of a kernel thread across > preemption. Thanks for the response. When was the preservation and restoration of the IA key added to the kernel, because in my 5.10 code base, I am seeing the IA key change for the kernel between system calls. > With the patches merged for v5.13, the other keys will be > switched with the task, but userspace can reset these at any time, and > they are still not supposed to be used within the kernel. > > > If so, how can I ensure that the keys do not change? The different > > keys are causing PAC authentication to fail on pointers signed using > > the stale key. Thanks. > > I take it this is non-mainline code? We shouldn't be using the other > keys today. Yeah, I'm a doctorate researcher attempting to combine PAC and MTE to harden kernel modules, which is why I am trying to make sure PAC keys are the same. Pointers to kernel data are being signed and authenticated using different keys. It actually doesn't matter for our purposes what key is used, just that the key remains the same whenever kernel code gets executed. Thanks. -- Derrick McKee _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] Ensure kernel AI key is not changed on fork 2021-04-30 15:04 ` Mark Rutland @ 2021-05-20 15:18 ` Derrick McKee 2021-05-20 15:18 ` Derrick McKee 1 sibling, 0 replies; 9+ messages in thread From: Derrick McKee @ 2021-05-20 15:18 UTC (permalink / raw) To: derrick.mckee Cc: Nathan.Burow, Yianni Giannaris, Catalin Marinas, Will Deacon, linux-arm-kernel, linux-kernel 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 <derrick.mckee@gmail.com> Signed-off-by: Yianni Giannaris <yiannig@mit.edu> --- 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH] Ensure kernel AI key is not changed on fork @ 2021-05-20 15:18 ` Derrick McKee 0 siblings, 0 replies; 9+ messages in thread From: Derrick McKee @ 2021-05-20 15:18 UTC (permalink / raw) To: derrick.mckee Cc: Nathan.Burow, Yianni Giannaris, Catalin Marinas, Will Deacon, linux-arm-kernel, linux-kernel 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 <derrick.mckee@gmail.com> Signed-off-by: Yianni Giannaris <yiannig@mit.edu> --- 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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] Ensure kernel AI key is not changed on fork 2021-05-20 15:18 ` Derrick McKee @ 2021-05-20 16:00 ` Mark Rutland -1 siblings, 0 replies; 9+ messages in thread From: Mark Rutland @ 2021-05-20 16:00 UTC (permalink / raw) To: Derrick McKee Cc: Nathan.Burow, Yianni Giannaris, Catalin Marinas, Will Deacon, linux-arm-kernel, linux-kernel 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 <derrick.mckee@gmail.com> > Signed-off-by: Yianni Giannaris <yiannig@mit.edu> 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 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Ensure kernel AI key is not changed on fork @ 2021-05-20 16:00 ` Mark Rutland 0 siblings, 0 replies; 9+ messages in thread From: Mark Rutland @ 2021-05-20 16:00 UTC (permalink / raw) To: Derrick McKee Cc: Nathan.Burow, Yianni Giannaris, Catalin Marinas, Will Deacon, linux-arm-kernel, linux-kernel 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 <derrick.mckee@gmail.com> > Signed-off-by: Yianni Giannaris <yiannig@mit.edu> 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 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Ensure kernel AI key is not changed on fork 2021-05-20 16:00 ` Mark Rutland @ 2021-05-20 18:24 ` Derrick McKee -1 siblings, 0 replies; 9+ messages in thread From: Derrick McKee @ 2021-05-20 18:24 UTC (permalink / raw) To: Mark Rutland Cc: Nathan Harrison Burow, Yianni Giannaris, Catalin Marinas, Will Deacon, Linux ARM, LKML On Thu, May 20, 2021 at 12:00 PM Mark Rutland <mark.rutland@arm.com> 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: derrick.mckee@gmail.com ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Ensure kernel AI key is not changed on fork @ 2021-05-20 18:24 ` Derrick McKee 0 siblings, 0 replies; 9+ messages in thread From: Derrick McKee @ 2021-05-20 18:24 UTC (permalink / raw) To: Mark Rutland Cc: Nathan Harrison Burow, Yianni Giannaris, Catalin Marinas, Will Deacon, Linux ARM, LKML On Thu, May 20, 2021 at 12:00 PM Mark Rutland <mark.rutland@arm.com> 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: derrick.mckee@gmail.com _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-05-20 18:26 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-04-30 14:40 PAC key changes when kernel code is preempted Derrick McKee 2021-04-30 15:04 ` Mark Rutland 2021-05-07 20:24 ` Derrick McKee 2021-05-20 15:18 ` [PATCH] Ensure kernel AI key is not changed on fork Derrick McKee 2021-05-20 15:18 ` Derrick McKee 2021-05-20 16:00 ` Mark Rutland 2021-05-20 16:00 ` Mark Rutland 2021-05-20 18:24 ` Derrick McKee 2021-05-20 18:24 ` Derrick McKee
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.