All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kees Cook <keescook@chromium.org>
To: Kristina Martsenko <kristina.martsenko@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>,
	Ard Biesheuvel <ard.biesheuvel@linaro.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	Will Deacon <will.deacon@arm.com>,
	Ramana Radhakrishnan <ramana.radhakrishnan@arm.com>,
	Amit Kachhap <Amit.Kachhap@arm.com>,
	Dave Martin <Dave.Martin@arm.com>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [RFC v2 5/7] arm64: initialize and switch ptrauth kernel keys
Date: Wed, 29 May 2019 20:34:50 -0700	[thread overview]
Message-ID: <201905292027.B439EF2CA@keescook> (raw)
In-Reply-To: <20190529190332.29753-6-kristina.martsenko@arm.com>

On Wed, May 29, 2019 at 08:03:30PM +0100, Kristina Martsenko wrote:
> Set up keys to use pointer authentication within the kernel. The kernel
> will be compiled with APIAKey instructions, the other keys are currently
> unused. Each task is given its own APIAKey, which is initialized during
> fork. The key is changed during context switch and on kernel entry from
> EL0.
> 
> A function that changes the key cannot return, so compile such functions
> without pointer auth (using __no_ptrauth which will be defined to a
> compiler function attribute later).
> 
> Signed-off-by: Kristina Martsenko <kristina.martsenko@arm.com>

Just so I'm reading this right: the kernel is only using APIAKey?

Reviewed-by: Kees Cook <keescook@chromium.org>

-Kees

> ---
> 
> Changes since RFC v1:
>  - Updated ptrauth_keys_init to not randomly initialize the other 4 (unused)
>    kernel-space keys, to save entropy
>  - Replaced __always_inline with __no_ptrauth as it turns out __always_inline
>    is only a hint (and could therefore result in crashes)
>  - Moved ptrauth_keys_install_kernel earlier in kernel_entry, in case in the
>    future C function calls are added in kernel_entry
>  - Added ISB after key install in kernel_exit, in case in the future C function
>    calls are added after the macro
>  - Dropped an outdated comment
>  - Updated the commit message
> 
>  arch/arm64/include/asm/asm_pointer_auth.h | 16 +++++++++++++++
>  arch/arm64/include/asm/pointer_auth.h     | 33 +++++++++++++++++++++----------
>  arch/arm64/include/asm/processor.h        |  1 +
>  arch/arm64/kernel/asm-offsets.c           |  1 +
>  arch/arm64/kernel/entry.S                 |  1 +
>  arch/arm64/kernel/pointer_auth.c          |  2 +-
>  arch/arm64/kernel/process.c               |  3 +++
>  arch/arm64/kernel/smp.c                   |  3 +++
>  8 files changed, 49 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/asm_pointer_auth.h b/arch/arm64/include/asm/asm_pointer_auth.h
> index e3bfddfe80b6..f595da9661a4 100644
> --- a/arch/arm64/include/asm/asm_pointer_auth.h
> +++ b/arch/arm64/include/asm/asm_pointer_auth.h
> @@ -25,11 +25,24 @@ alternative_if ARM64_HAS_ADDRESS_AUTH
>  	ldp	\tmp2, \tmp3, [\tmp1, #PTRAUTH_KEY_APDB]
>  	msr_s	SYS_APDBKEYLO_EL1, \tmp2
>  	msr_s	SYS_APDBKEYHI_EL1, \tmp3
> +	isb
>  alternative_else_nop_endif
>  alternative_if ARM64_HAS_GENERIC_AUTH
>  	ldp	\tmp2, \tmp3, [\tmp1, #PTRAUTH_KEY_APGA]
>  	msr_s	SYS_APGAKEYLO_EL1, \tmp2
>  	msr_s	SYS_APGAKEYHI_EL1, \tmp3
> +	isb
> +alternative_else_nop_endif
> +	.endm
> +
> +	.macro ptrauth_keys_install_kernel tsk, tmp1, tmp2, tmp3
> +	mov	\tmp1, #THREAD_KEYS_KERNEL
> +	add	\tmp1, \tsk, \tmp1
> +alternative_if ARM64_HAS_ADDRESS_AUTH
> +	ldp	\tmp2, \tmp3, [\tmp1, #PTRAUTH_KEY_APIA]
> +	msr_s	SYS_APIAKEYLO_EL1, \tmp2
> +	msr_s	SYS_APIAKEYHI_EL1, \tmp3
> +	isb
>  alternative_else_nop_endif
>  	.endm
>  
> @@ -38,6 +51,9 @@ alternative_else_nop_endif
>  	.macro ptrauth_keys_install_user tsk, tmp1, tmp2, tmp3
>  	.endm
>  
> +	.macro ptrauth_keys_install_kernel tsk, tmp1, tmp2, tmp3
> +	.endm
> +
>  #endif /* CONFIG_ARM64_PTR_AUTH */
>  
>  #endif /* __ASM_ASM_POINTER_AUTH_H */
> diff --git a/arch/arm64/include/asm/pointer_auth.h b/arch/arm64/include/asm/pointer_auth.h
> index a97b7dc10bdb..79f35f5ecff5 100644
> --- a/arch/arm64/include/asm/pointer_auth.h
> +++ b/arch/arm64/include/asm/pointer_auth.h
> @@ -25,10 +25,6 @@ struct ptrauth_key {
>  	unsigned long lo, hi;
>  };
>  
> -/*
> - * 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_key apia;
>  	struct ptrauth_key apib;
> @@ -37,16 +33,18 @@ struct ptrauth_keys {
>  	struct ptrauth_key apga;
>  };
>  
> -static inline void ptrauth_keys_init(struct ptrauth_keys *keys)
> +static inline void ptrauth_keys_init(struct ptrauth_keys *keys, bool user)
>  {
>  	if (system_supports_address_auth()) {
>  		get_random_bytes(&keys->apia, sizeof(keys->apia));
> -		get_random_bytes(&keys->apib, sizeof(keys->apib));
> -		get_random_bytes(&keys->apda, sizeof(keys->apda));
> -		get_random_bytes(&keys->apdb, sizeof(keys->apdb));
> +		if (user) {
> +			get_random_bytes(&keys->apib, sizeof(keys->apib));
> +			get_random_bytes(&keys->apda, sizeof(keys->apda));
> +			get_random_bytes(&keys->apdb, sizeof(keys->apdb));
> +		}
>  	}
>  
> -	if (system_supports_generic_auth())
> +	if (system_supports_generic_auth() && user)
>  		get_random_bytes(&keys->apga, sizeof(keys->apga));
>  }
>  
> @@ -57,6 +55,15 @@ do {								\
>  	write_sysreg_s(__pki_v.hi, SYS_ ## k ## KEYHI_EL1);	\
>  } while (0)
>  
> +static inline void __no_ptrauth ptrauth_keys_switch(struct ptrauth_keys *keys)
> +{
> +	if (!system_supports_address_auth())
> +		return;
> +
> +	__ptrauth_key_install(APIA, keys->apia);
> +	isb();
> +}
> +
>  static inline void __no_ptrauth ptrauth_cpu_enable(void)
>  {
>  	if (!system_supports_address_auth())
> @@ -82,7 +89,11 @@ 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(&(tsk)->thread.keys_user, true)
> +#define ptrauth_thread_init_kernel(tsk)					\
> +	ptrauth_keys_init(&(tsk)->thread.keys_kernel, false)
> +#define ptrauth_thread_switch(tsk)					\
> +	ptrauth_keys_switch(&(tsk)->thread.keys_kernel)
>  
>  #else /* CONFIG_ARM64_PTR_AUTH */
>  #define __no_ptrauth
> @@ -90,6 +101,8 @@ static inline unsigned long ptrauth_strip_insn_pac(unsigned long ptr)
>  #define ptrauth_prctl_reset_keys(tsk, arg)	(-EINVAL)
>  #define ptrauth_strip_insn_pac(lr)	(lr)
>  #define ptrauth_thread_init_user(tsk)
> +#define ptrauth_thread_init_kernel(tsk)
> +#define ptrauth_thread_switch(tsk)
>  #endif /* CONFIG_ARM64_PTR_AUTH */
>  
>  #endif /* __ASM_POINTER_AUTH_H */
> diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h
> index 5d9ce62bdebd..f7684a021ca2 100644
> --- a/arch/arm64/include/asm/processor.h
> +++ b/arch/arm64/include/asm/processor.h
> @@ -149,6 +149,7 @@ struct thread_struct {
>  	struct debug_info	debug;		/* debugging */
>  #ifdef CONFIG_ARM64_PTR_AUTH
>  	struct ptrauth_keys	keys_user;
> +	struct ptrauth_keys	keys_kernel;
>  #endif
>  };
>  
> diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
> index edc471e4acb1..7dfebecd387d 100644
> --- a/arch/arm64/kernel/asm-offsets.c
> +++ b/arch/arm64/kernel/asm-offsets.c
> @@ -53,6 +53,7 @@ int main(void)
>    DEFINE(THREAD_CPU_CONTEXT,	offsetof(struct task_struct, thread.cpu_context));
>  #ifdef CONFIG_ARM64_PTR_AUTH
>    DEFINE(THREAD_KEYS_USER,	offsetof(struct task_struct, thread.keys_user));
> +  DEFINE(THREAD_KEYS_KERNEL,	offsetof(struct task_struct, thread.keys_kernel));
>  #endif
>    BLANK();
>    DEFINE(S_X0,			offsetof(struct pt_regs, regs[0]));
> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
> index 73a28d88f78d..001d508cd63f 100644
> --- a/arch/arm64/kernel/entry.S
> +++ b/arch/arm64/kernel/entry.S
> @@ -184,6 +184,7 @@ alternative_cb_end
>  
>  	apply_ssbd 1, x22, x23
>  
> +	ptrauth_keys_install_kernel tsk, x20, x22, x23
>  	.else
>  	add	x21, sp, #S_FRAME_SIZE
>  	get_current_task tsk
> diff --git a/arch/arm64/kernel/pointer_auth.c b/arch/arm64/kernel/pointer_auth.c
> index 95985be67891..4ca141b8c581 100644
> --- a/arch/arm64/kernel/pointer_auth.c
> +++ b/arch/arm64/kernel/pointer_auth.c
> @@ -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(keys, true);
>  		return 0;
>  	}
>  
> diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
> index a9b30111b725..890d7185641b 100644
> --- a/arch/arm64/kernel/process.c
> +++ b/arch/arm64/kernel/process.c
> @@ -378,6 +378,8 @@ int copy_thread(unsigned long clone_flags, unsigned long stack_start,
>  	 */
>  	fpsimd_flush_task_state(p);
>  
> +	ptrauth_thread_init_kernel(p);
> +
>  	if (likely(!(p->flags & PF_KTHREAD))) {
>  		*childregs = *current_pt_regs();
>  		childregs->regs[0] = 0;
> @@ -481,6 +483,7 @@ __notrace_funcgraph struct task_struct *__switch_to(struct task_struct *prev,
>  	contextidr_thread_switch(next);
>  	entry_task_switch(next);
>  	uao_thread_switch(next);
> +	ptrauth_thread_switch(next);
>  
>  	/*
>  	 * Complete any pending TLB or cache maintenance on this CPU in case
> diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
> index eca6aa05ac66..c5a6f3e8660b 100644
> --- a/arch/arm64/kernel/smp.c
> +++ b/arch/arm64/kernel/smp.c
> @@ -239,6 +239,7 @@ asmlinkage notrace void secondary_start_kernel(void)
>  	 */
>  	check_local_cpu_capabilities();
>  
> +	ptrauth_thread_switch(current);
>  	ptrauth_cpu_enable();
>  
>  	if (cpu_ops[cpu]->cpu_postboot)
> @@ -456,6 +457,8 @@ void __init __no_ptrauth smp_prepare_boot_cpu(void)
>  	if (system_uses_irq_prio_masking())
>  		init_gic_priority_masking();
>  
> +	ptrauth_thread_init_kernel(current);
> +	ptrauth_thread_switch(current);
>  	ptrauth_cpu_enable();
>  }
>  
> -- 
> 2.11.0
> 

-- 
Kees Cook

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

  reply	other threads:[~2019-05-30  3:35 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-29 19:03 [RFC v2 0/7] arm64: return address signing Kristina Martsenko
2019-05-29 19:03 ` [RFC v2 1/7] arm64: cpufeature: add pointer auth meta-capabilities Kristina Martsenko
2019-05-30  1:58   ` Kees Cook
2019-05-30 10:50   ` Suzuki K Poulose
2019-06-13 16:13     ` Suzuki K Poulose
2019-05-29 19:03 ` [RFC v2 2/7] arm64: install user ptrauth keys at kernel exit time Kristina Martsenko
2019-05-30  2:04   ` Kees Cook
2019-06-06 16:26   ` Catalin Marinas
2019-05-29 19:03 ` [RFC v2 3/7] arm64: cpufeature: handle conflicts based on capability Kristina Martsenko
2019-05-30  2:49   ` Kees Cook
2019-05-30 14:16   ` Suzuki K Poulose
2019-05-31 14:00     ` Kristina Martsenko
2019-05-31 15:08       ` Suzuki K Poulose
2019-05-29 19:03 ` [RFC v2 4/7] arm64: enable ptrauth earlier Kristina Martsenko
2019-05-30  3:11   ` Kees Cook
2019-06-13 15:41   ` Suzuki K Poulose
2019-05-29 19:03 ` [RFC v2 5/7] arm64: initialize and switch ptrauth kernel keys Kristina Martsenko
2019-05-30  3:34   ` Kees Cook [this message]
2019-05-30 16:26     ` Kristina Martsenko
2019-06-04 10:03   ` Dave Martin
2019-06-06 16:44   ` Catalin Marinas
2019-06-12 16:21     ` Kristina Martsenko
2019-06-13 10:44       ` Catalin Marinas
2019-05-29 19:03 ` [RFC v2 6/7] arm64: unwind: strip PAC from kernel addresses Kristina Martsenko
2019-05-30  3:36   ` Kees Cook
2019-05-29 19:03 ` [RFC v2 7/7] arm64: compile the kernel with ptrauth return address signing Kristina Martsenko
2019-05-30  3:45   ` Kees Cook
2019-05-30  3:09 ` [RFC v2 0/7] arm64: " Kees Cook
2019-05-30  7:25   ` Will Deacon
2019-05-30  8:39     ` Ard Biesheuvel
2019-05-30  9:11       ` Ramana Radhakrishnan
2019-05-30  9:12   ` Ramana Radhakrishnan
2019-06-06 17:44     ` Kristina Martsenko
2019-06-08  4:09       ` Kees Cook
     [not found]   ` <DB7PR08MB3865C4AA36C9C465B2A687DABF180@DB7PR08MB3865.eurprd08.prod.outlook.com>
2019-05-30 15:57     ` Kees Cook
     [not found]       ` <DB7PR08MB3865A83066179CE419D171EDBF180@DB7PR08MB3865.eurprd08.prod.outlook.com>
2019-05-30 18:05         ` Kees Cook
2019-05-31  9:22           ` Will Deacon
2019-06-02 15:43             ` Kees Cook
2019-06-03 10:40               ` Will Deacon
2019-06-04 13:52                 ` Luke Cheeseman
2019-06-06 17:43                   ` Kristina Martsenko

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=201905292027.B439EF2CA@keescook \
    --to=keescook@chromium.org \
    --cc=Amit.Kachhap@arm.com \
    --cc=Dave.Martin@arm.com \
    --cc=ard.biesheuvel@linaro.org \
    --cc=catalin.marinas@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.