From: Kees Cook <keescook@chromium.org>
To: Amit Daniel Kachhap <amit.kachhap@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>,
Suzuki K Poulose <suzuki.poulose@arm.com>,
Catalin Marinas <catalin.marinas@arm.com>,
Ard Biesheuvel <ard.biesheuvel@linaro.org>,
Will Deacon <will.deacon@arm.com>,
Kristina Martsenko <kristina.martsenko@arm.com>,
James Morse <james.morse@arm.com>,
Ramana Radhakrishnan <ramana.radhakrishnan@arm.com>,
Vincenzo Frascino <Vincenzo.Frascino@arm.com>,
Dave Martin <Dave.Martin@arm.com>,
linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 06/11] arm64: rename ptrauth key structures to be user-specific
Date: Tue, 29 Oct 2019 16:27:51 -0700 [thread overview]
Message-ID: <201910291627.0823B87@keescook> (raw)
In-Reply-To: <1571300065-10236-7-git-send-email-amit.kachhap@arm.com>
On Thu, Oct 17, 2019 at 01:44:20PM +0530, Amit Daniel Kachhap wrote:
> From: Kristina Martsenko <kristina.martsenko@arm.com>
>
> We currently enable ptrauth for userspace, but do not use it within the
> kernel. We're going to enable it for the kernel, and will need to manage
> a separate set of ptrauth keys for the kernel.
>
> We currently keep all 5 keys in struct ptrauth_keys. However, as the
> kernel will only need to use 1 key, it is a bit wasteful to allocate a
> whole ptrauth_keys struct for every thread.
>
> Therefore, a subsequent patch will define a separate struct, with only 1
> key, for the kernel. In preparation for that, rename the existing struct
> (and associated macros and functions) to reflect that they are specific
> to userspace.
>
> Signed-off-by: Kristina Martsenko <kristina.martsenko@arm.com>
This appears very mechanical; easy to review! ;)
Reviewed-by: Kees Cook <keescook@chromium.org>
-Kees
> Signed-off-by: Amit Daniel Kachhap <amit.kachhap@arm.com>
> ---
> Changes since RFC v2:
> - New patch in this version, to optimize struct usage [Dave]
>
> arch/arm64/include/asm/asm_pointer_auth.h | 10 +++++-----
> arch/arm64/include/asm/pointer_auth.h | 6 +++---
> arch/arm64/include/asm/processor.h | 2 +-
> arch/arm64/kernel/asm-offsets.c | 10 +++++-----
> arch/arm64/kernel/pointer_auth.c | 4 ++--
> arch/arm64/kernel/ptrace.c | 16 ++++++++--------
> 6 files changed, 24 insertions(+), 24 deletions(-)
>
> diff --git a/arch/arm64/include/asm/asm_pointer_auth.h b/arch/arm64/include/asm/asm_pointer_auth.h
> index cb21a06..3d39788 100644
> --- a/arch/arm64/include/asm/asm_pointer_auth.h
> +++ b/arch/arm64/include/asm/asm_pointer_auth.h
> @@ -15,21 +15,21 @@
> alternative_if_not ARM64_HAS_ADDRESS_AUTH
> b .Laddr_auth_skip_\@
> alternative_else_nop_endif
> - ldp \tmp2, \tmp3, [\tmp1, #PTRAUTH_KEY_APIA]
> + ldp \tmp2, \tmp3, [\tmp1, #PTRAUTH_USER_KEY_APIA]
> msr_s SYS_APIAKEYLO_EL1, \tmp2
> msr_s SYS_APIAKEYHI_EL1, \tmp3
> - ldp \tmp2, \tmp3, [\tmp1, #PTRAUTH_KEY_APIB]
> + ldp \tmp2, \tmp3, [\tmp1, #PTRAUTH_USER_KEY_APIB]
> msr_s SYS_APIBKEYLO_EL1, \tmp2
> msr_s SYS_APIBKEYHI_EL1, \tmp3
> - ldp \tmp2, \tmp3, [\tmp1, #PTRAUTH_KEY_APDA]
> + ldp \tmp2, \tmp3, [\tmp1, #PTRAUTH_USER_KEY_APDA]
> msr_s SYS_APDAKEYLO_EL1, \tmp2
> msr_s SYS_APDAKEYHI_EL1, \tmp3
> - ldp \tmp2, \tmp3, [\tmp1, #PTRAUTH_KEY_APDB]
> + ldp \tmp2, \tmp3, [\tmp1, #PTRAUTH_USER_KEY_APDB]
> msr_s SYS_APDBKEYLO_EL1, \tmp2
> msr_s SYS_APDBKEYHI_EL1, \tmp3
> .Laddr_auth_skip_\@:
> alternative_if ARM64_HAS_GENERIC_AUTH
> - ldp \tmp2, \tmp3, [\tmp1, #PTRAUTH_KEY_APGA]
> + ldp \tmp2, \tmp3, [\tmp1, #PTRAUTH_USER_KEY_APGA]
> msr_s SYS_APGAKEYLO_EL1, \tmp2
> msr_s SYS_APGAKEYHI_EL1, \tmp3
> alternative_else_nop_endif
> diff --git a/arch/arm64/include/asm/pointer_auth.h b/arch/arm64/include/asm/pointer_auth.h
> index 21c2115..cc42145 100644
> --- a/arch/arm64/include/asm/pointer_auth.h
> +++ b/arch/arm64/include/asm/pointer_auth.h
> @@ -22,7 +22,7 @@ struct ptrauth_key {
> * We give each process its own keys, which are shared by all threads. The keys
> * are inherited upon fork(), and reinitialised upon exec*().
> */
> -struct ptrauth_keys {
> +struct ptrauth_keys_user {
> struct ptrauth_key apia;
> struct ptrauth_key apib;
> struct ptrauth_key apda;
> @@ -30,7 +30,7 @@ struct ptrauth_keys {
> struct ptrauth_key apga;
> };
>
> -static inline void ptrauth_keys_init(struct ptrauth_keys *keys)
> +static inline void ptrauth_keys_init_user(struct ptrauth_keys_user *keys)
> {
> if (system_supports_address_auth()) {
> get_random_bytes(&keys->apia, sizeof(keys->apia));
> @@ -58,7 +58,7 @@ static inline unsigned long ptrauth_strip_insn_pac(unsigned long ptr)
> }
>
> #define ptrauth_thread_init_user(tsk) \
> - ptrauth_keys_init(&(tsk)->thread.keys_user)
> + ptrauth_keys_init_user(&(tsk)->thread.keys_user)
>
> #else /* CONFIG_ARM64_PTR_AUTH */
> #define ptrauth_prctl_reset_keys(tsk, arg) (-EINVAL)
> diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h
> index 5623685..8ec792d 100644
> --- a/arch/arm64/include/asm/processor.h
> +++ b/arch/arm64/include/asm/processor.h
> @@ -144,7 +144,7 @@ struct thread_struct {
> unsigned long fault_code; /* ESR_EL1 value */
> struct debug_info debug; /* debugging */
> #ifdef CONFIG_ARM64_PTR_AUTH
> - struct ptrauth_keys keys_user;
> + struct ptrauth_keys_user keys_user;
> #endif
> };
>
> diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
> index ef0c24b..cf15182 100644
> --- a/arch/arm64/kernel/asm-offsets.c
> +++ b/arch/arm64/kernel/asm-offsets.c
> @@ -131,11 +131,11 @@ int main(void)
> DEFINE(SDEI_EVENT_PRIORITY, offsetof(struct sdei_registered_event, priority));
> #endif
> #ifdef CONFIG_ARM64_PTR_AUTH
> - DEFINE(PTRAUTH_KEY_APIA, offsetof(struct ptrauth_keys, apia));
> - DEFINE(PTRAUTH_KEY_APIB, offsetof(struct ptrauth_keys, apib));
> - DEFINE(PTRAUTH_KEY_APDA, offsetof(struct ptrauth_keys, apda));
> - DEFINE(PTRAUTH_KEY_APDB, offsetof(struct ptrauth_keys, apdb));
> - DEFINE(PTRAUTH_KEY_APGA, offsetof(struct ptrauth_keys, apga));
> + DEFINE(PTRAUTH_USER_KEY_APIA, offsetof(struct ptrauth_keys_user, apia));
> + DEFINE(PTRAUTH_USER_KEY_APIB, offsetof(struct ptrauth_keys_user, apib));
> + DEFINE(PTRAUTH_USER_KEY_APDA, offsetof(struct ptrauth_keys_user, apda));
> + DEFINE(PTRAUTH_USER_KEY_APDB, offsetof(struct ptrauth_keys_user, apdb));
> + DEFINE(PTRAUTH_USER_KEY_APGA, offsetof(struct ptrauth_keys_user, apga));
> BLANK();
> #endif
> return 0;
> diff --git a/arch/arm64/kernel/pointer_auth.c b/arch/arm64/kernel/pointer_auth.c
> index 95985be..1e77736 100644
> --- a/arch/arm64/kernel/pointer_auth.c
> +++ b/arch/arm64/kernel/pointer_auth.c
> @@ -9,7 +9,7 @@
>
> int ptrauth_prctl_reset_keys(struct task_struct *tsk, unsigned long arg)
> {
> - struct ptrauth_keys *keys = &tsk->thread.keys_user;
> + struct ptrauth_keys_user *keys = &tsk->thread.keys_user;
> unsigned long addr_key_mask = PR_PAC_APIAKEY | PR_PAC_APIBKEY |
> PR_PAC_APDAKEY | PR_PAC_APDBKEY;
> unsigned long key_mask = addr_key_mask | PR_PAC_APGAKEY;
> @@ -18,7 +18,7 @@ int ptrauth_prctl_reset_keys(struct task_struct *tsk, unsigned long arg)
> return -EINVAL;
>
> if (!arg) {
> - ptrauth_keys_init(keys);
> + ptrauth_keys_init_user(keys);
> return 0;
> }
>
> diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
> index 21176d0..0793739 100644
> --- a/arch/arm64/kernel/ptrace.c
> +++ b/arch/arm64/kernel/ptrace.c
> @@ -986,7 +986,7 @@ static struct ptrauth_key pac_key_from_user(__uint128_t ukey)
> }
>
> static void pac_address_keys_to_user(struct user_pac_address_keys *ukeys,
> - const struct ptrauth_keys *keys)
> + const struct ptrauth_keys_user *keys)
> {
> ukeys->apiakey = pac_key_to_user(&keys->apia);
> ukeys->apibkey = pac_key_to_user(&keys->apib);
> @@ -994,7 +994,7 @@ static void pac_address_keys_to_user(struct user_pac_address_keys *ukeys,
> ukeys->apdbkey = pac_key_to_user(&keys->apdb);
> }
>
> -static void pac_address_keys_from_user(struct ptrauth_keys *keys,
> +static void pac_address_keys_from_user(struct ptrauth_keys_user *keys,
> const struct user_pac_address_keys *ukeys)
> {
> keys->apia = pac_key_from_user(ukeys->apiakey);
> @@ -1008,7 +1008,7 @@ static int pac_address_keys_get(struct task_struct *target,
> unsigned int pos, unsigned int count,
> void *kbuf, void __user *ubuf)
> {
> - struct ptrauth_keys *keys = &target->thread.keys_user;
> + struct ptrauth_keys_user *keys = &target->thread.keys_user;
> struct user_pac_address_keys user_keys;
>
> if (!system_supports_address_auth())
> @@ -1025,7 +1025,7 @@ static int pac_address_keys_set(struct task_struct *target,
> unsigned int pos, unsigned int count,
> const void *kbuf, const void __user *ubuf)
> {
> - struct ptrauth_keys *keys = &target->thread.keys_user;
> + struct ptrauth_keys_user *keys = &target->thread.keys_user;
> struct user_pac_address_keys user_keys;
> int ret;
>
> @@ -1043,12 +1043,12 @@ static int pac_address_keys_set(struct task_struct *target,
> }
>
> static void pac_generic_keys_to_user(struct user_pac_generic_keys *ukeys,
> - const struct ptrauth_keys *keys)
> + const struct ptrauth_keys_user *keys)
> {
> ukeys->apgakey = pac_key_to_user(&keys->apga);
> }
>
> -static void pac_generic_keys_from_user(struct ptrauth_keys *keys,
> +static void pac_generic_keys_from_user(struct ptrauth_keys_user *keys,
> const struct user_pac_generic_keys *ukeys)
> {
> keys->apga = pac_key_from_user(ukeys->apgakey);
> @@ -1059,7 +1059,7 @@ static int pac_generic_keys_get(struct task_struct *target,
> unsigned int pos, unsigned int count,
> void *kbuf, void __user *ubuf)
> {
> - struct ptrauth_keys *keys = &target->thread.keys_user;
> + struct ptrauth_keys_user *keys = &target->thread.keys_user;
> struct user_pac_generic_keys user_keys;
>
> if (!system_supports_generic_auth())
> @@ -1076,7 +1076,7 @@ static int pac_generic_keys_set(struct task_struct *target,
> unsigned int pos, unsigned int count,
> const void *kbuf, const void __user *ubuf)
> {
> - struct ptrauth_keys *keys = &target->thread.keys_user;
> + struct ptrauth_keys_user *keys = &target->thread.keys_user;
> struct user_pac_generic_keys user_keys;
> int ret;
>
> --
> 2.7.4
>
--
Kees Cook
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2019-10-29 23:27 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-17 8:14 [PATCH 00/11] arm64: return address signing Amit Daniel Kachhap
2019-10-17 8:14 ` [PATCH 01/11] arm64: cpufeature: add pointer auth meta-capabilities Amit Daniel Kachhap
2019-10-17 8:14 ` [PATCH 02/11] arm64: install user ptrauth keys at kernel exit time Amit Daniel Kachhap
2019-10-17 8:14 ` [PATCH 03/11] arm64: cpufeature: handle conflicts based on capability Amit Daniel Kachhap
2019-10-17 14:06 ` Suzuki K Poulose
2019-10-18 9:59 ` Amit Kachhap
2019-10-17 8:14 ` [PATCH 04/11] arm64: create macro to park cpu in an infinite loop Amit Daniel Kachhap
2019-10-17 8:14 ` [PATCH 05/11] arm64: enable ptrauth earlier Amit Daniel Kachhap
2019-10-17 14:13 ` Suzuki K Poulose
2019-10-18 10:07 ` Amit Kachhap
2019-10-23 17:32 ` James Morse
2019-10-30 4:01 ` Amit Daniel Kachhap
2019-10-17 8:14 ` [PATCH 06/11] arm64: rename ptrauth key structures to be user-specific Amit Daniel Kachhap
2019-10-29 23:27 ` Kees Cook [this message]
2019-10-17 8:14 ` [PATCH 07/11] arm64: initialize and switch ptrauth kernel keys Amit Daniel Kachhap
2019-10-23 17:35 ` James Morse
2019-10-30 4:02 ` Amit Daniel Kachhap
2019-10-17 8:14 ` [PATCH 08/11] arm64: unwind: strip PAC from kernel addresses Amit Daniel Kachhap
2019-10-23 17:36 ` James Morse
2019-10-30 4:02 ` Amit Daniel Kachhap
2019-10-17 8:14 ` [RFC PATCH 09/11] arm64: suspend: restore the kernel ptrauth keys Amit Daniel Kachhap
2019-10-17 8:14 ` [RFC PATCH 10/11] arm64: mask PAC bits of __builtin_return_address Amit Daniel Kachhap
2019-10-17 8:14 ` [PATCH 11/11] arm64: compile the kernel with ptrauth return address signing Amit Daniel Kachhap
2019-10-23 17:31 ` [PATCH 00/11] arm64: " James Morse
2019-10-30 3:59 ` Amit Daniel Kachhap
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=201910291627.0823B87@keescook \
--to=keescook@chromium.org \
--cc=Dave.Martin@arm.com \
--cc=Vincenzo.Frascino@arm.com \
--cc=amit.kachhap@arm.com \
--cc=ard.biesheuvel@linaro.org \
--cc=catalin.marinas@arm.com \
--cc=james.morse@arm.com \
--cc=kristina.martsenko@arm.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=mark.rutland@arm.com \
--cc=ramana.radhakrishnan@arm.com \
--cc=suzuki.poulose@arm.com \
--cc=will.deacon@arm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.