Re: [PATCH v6 11/18] arm64: initialize ptrauth keys for kernel booting task

From: Vincenzo Frascino <vincenzo.frascino@arm.com>
To: Amit Daniel Kachhap <amit.kachhap@arm.com>,
	linux-arm-kernel@lists.infradead.org
Cc: Mark Rutland <mark.rutland@arm.com>,
	Kees Cook <keescook@chromium.org>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Kristina Martsenko <kristina.martsenko@arm.com>,
	Dave Martin <Dave.Martin@arm.com>,
	Mark Brown <broonie@kernel.org>,
	James Morse <james.morse@arm.com>,
	Ramana Radhakrishnan <ramana.radhakrishnan@arm.com>,
	Will Deacon <will@kernel.org>, Ard Biesheuvel <ardb@kernel.org>
Subject: Re: [PATCH v6 11/18] arm64: initialize ptrauth keys for kernel booting task
Date: Tue, 10 Mar 2020 15:09:04 +0000	[thread overview]
Message-ID: <0480bdeb-d4b4-c8f9-0a1b-3cf07caeb050@arm.com> (raw)
In-Reply-To: <1583476525-13505-12-git-send-email-amit.kachhap@arm.com>

On 3/6/20 6:35 AM, Amit Daniel Kachhap wrote:
> This patch uses the existing boot_init_stack_canary arch function
> to initialize the ptrauth keys for the booting task in the primary
> core. The requirement here is that it should be always inline and
> the caller must never return.
> 
> As pointer authentication too detects a subset of stack corruption
> so it makes sense to place this code here.
> 
> Both pointer authentication and stack canary codes are protected
> by their respective config option.
>

Reviewed-by: Vincenzo Frascino <Vincenzo.Frascino@arm.com>

> Suggested-by: Ard Biesheuvel <ardb@kernel.org>
> Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
> Signed-off-by: Amit Daniel Kachhap <amit.kachhap@arm.com>
> ---
>  arch/arm64/include/asm/pointer_auth.h   | 11 ++++++++++-
>  arch/arm64/include/asm/stackprotector.h |  5 +++++
>  include/linux/stackprotector.h          |  2 +-
>  3 files changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/pointer_auth.h b/arch/arm64/include/asm/pointer_auth.h
> index aa956ca..833d3f9 100644
> --- a/arch/arm64/include/asm/pointer_auth.h
> +++ b/arch/arm64/include/asm/pointer_auth.h
> @@ -54,12 +54,18 @@ do {								\
>  	write_sysreg_s(__pki_v.hi, SYS_ ## k ## KEYHI_EL1);	\
>  } while (0)
>  
> -static 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));
>  }
>  
> +static __always_inline void ptrauth_keys_switch_kernel(struct ptrauth_keys_kernel *keys)
> +{
> +	if (system_supports_address_auth())
> +		__ptrauth_key_install(APIA, keys->apia);
> +}
> +
>  extern int ptrauth_prctl_reset_keys(struct task_struct *tsk, unsigned long arg);
>  
>  /*
> @@ -78,12 +84,15 @@ static inline unsigned long ptrauth_strip_insn_pac(unsigned long ptr)
>  	ptrauth_keys_init_user(&(tsk)->thread.keys_user)
>  #define ptrauth_thread_init_kernel(tsk)					\
>  	ptrauth_keys_init_kernel(&(tsk)->thread.keys_kernel)
> +#define ptrauth_thread_switch_kernel(tsk)				\
> +	ptrauth_keys_switch_kernel(&(tsk)->thread.keys_kernel)
>  
>  #else /* CONFIG_ARM64_PTR_AUTH */
>  #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_kernel(tsk)
>  #endif /* CONFIG_ARM64_PTR_AUTH */
>  
>  #endif /* __ASM_POINTER_AUTH_H */
> diff --git a/arch/arm64/include/asm/stackprotector.h b/arch/arm64/include/asm/stackprotector.h
> index 5884a2b..7263e0b 100644
> --- a/arch/arm64/include/asm/stackprotector.h
> +++ b/arch/arm64/include/asm/stackprotector.h
> @@ -15,6 +15,7 @@
>  
>  #include <linux/random.h>
>  #include <linux/version.h>
> +#include <asm/pointer_auth.h>
>  
>  extern unsigned long __stack_chk_guard;
>  
> @@ -26,6 +27,7 @@ extern unsigned long __stack_chk_guard;
>   */
>  static __always_inline void boot_init_stack_canary(void)
>  {
> +#if defined(CONFIG_STACKPROTECTOR)
>  	unsigned long canary;
>  
>  	/* Try to get a semi random initial value. */
> @@ -36,6 +38,9 @@ static __always_inline void boot_init_stack_canary(void)
>  	current->stack_canary = canary;
>  	if (!IS_ENABLED(CONFIG_STACKPROTECTOR_PER_TASK))
>  		__stack_chk_guard = current->stack_canary;
> +#endif
> +	ptrauth_thread_init_kernel(current);
> +	ptrauth_thread_switch_kernel(current);
>  }
>  
>  #endif	/* _ASM_STACKPROTECTOR_H */
> diff --git a/include/linux/stackprotector.h b/include/linux/stackprotector.h
> index 6b792d0..4c678c4 100644
> --- a/include/linux/stackprotector.h
> +++ b/include/linux/stackprotector.h
> @@ -6,7 +6,7 @@
>  #include <linux/sched.h>
>  #include <linux/random.h>
>  
> -#ifdef CONFIG_STACKPROTECTOR
> +#if defined(CONFIG_STACKPROTECTOR) || defined(CONFIG_ARM64_PTR_AUTH)
>  # include <asm/stackprotector.h>
>  #else
>  static inline void boot_init_stack_canary(void)
> 

-- 
Regards,
Vincenzo

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