public inbox for linux-arm-kernel@lists.infradead.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox