All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/2] arm64: split ARM64_PTR_AUTH option to userspace and kernel
@ 2021-02-08 14:55 Daniel Kiss
  2021-02-08 14:55 ` [PATCH v3 1/2] arm64: Add ARM64_PTR_AUTH_KERNEL config option Daniel Kiss
  2021-02-08 14:55 ` [PATCH v3 2/2] arm64: Conditionally configure PTR_AUTH key of the kernel Daniel Kiss
  0 siblings, 2 replies; 6+ messages in thread
From: Daniel Kiss @ 2021-02-08 14:55 UTC (permalink / raw)
  To: linux-arm-kernel, will; +Cc: Catalin.Marinas, pcc, Ard.Biesheuvel, Daniel Kiss

This patch series is to support pointer authentication in userspace
independently from the kernel use.

Use of pointer authentication in kernel depends on toolchain suppport
and might not be desired in production due to deployment problems.
Debugging the kernel also a bit harder with PAC. Let the distributions
choose where the pointer authentication is used. 
Config structure for BTI is similar.

This patch series can be cleanly applied on v5.12.rc6

The patch series has been tested on Arm FVP in all three
config combinations.

Changes from v2:
* Moved the toolchain version check under ARM64_PTR_AUTH_KERNEL.

Changes from v1:
* Keep the A key in register while running in the kernel.

Daniel Kiss (2):
  arm64: Add ARM64_PTR_AUTH_KERNEL config option
  arm64: Conditionally configure PTR_AUTH key of the kernel.

 arch/arm64/Kconfig                        | 33 +++++++-----
 arch/arm64/Makefile                       |  2 +-
 arch/arm64/include/asm/asm_pointer_auth.h | 63 +++++++++++++----------
 arch/arm64/include/asm/pointer_auth.h     | 20 +++++--
 arch/arm64/include/asm/processor.h        |  2 +
 arch/arm64/kernel/asm-offsets.c           |  4 ++
 drivers/misc/lkdtm/bugs.c                 |  6 +--
 7 files changed, 79 insertions(+), 51 deletions(-)

-- 
2.25.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] 6+ messages in thread

* [PATCH v3 1/2] arm64: Add ARM64_PTR_AUTH_KERNEL config option
  2021-02-08 14:55 [PATCH v3 0/2] arm64: split ARM64_PTR_AUTH option to userspace and kernel Daniel Kiss
@ 2021-02-08 14:55 ` Daniel Kiss
  2021-03-29 14:51   ` Will Deacon
  2021-02-08 14:55 ` [PATCH v3 2/2] arm64: Conditionally configure PTR_AUTH key of the kernel Daniel Kiss
  1 sibling, 1 reply; 6+ messages in thread
From: Daniel Kiss @ 2021-02-08 14:55 UTC (permalink / raw)
  To: linux-arm-kernel, will; +Cc: Catalin.Marinas, pcc, Ard.Biesheuvel, Daniel Kiss

This patch add the ARM64_PTR_AUTH_KERNEL config and deals with the
build aspect of it.

Userspace support has no dependency on the toolchain therefore all
toolchain checks and build flags are controlled the new config
option.
The default config behavior will not be changed.

Signed-off-by: Daniel Kiss <daniel.kiss@arm.com>
---
 arch/arm64/Kconfig              | 33 +++++++++++++++++++--------------
 arch/arm64/Makefile             |  2 +-
 arch/arm64/kernel/asm-offsets.c |  2 ++
 drivers/misc/lkdtm/bugs.c       |  6 +++---
 4 files changed, 25 insertions(+), 18 deletions(-)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index f39568b28ec1..c1268cdd6964 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -1474,12 +1474,6 @@ menu "ARMv8.3 architectural features"
 config ARM64_PTR_AUTH
 	bool "Enable support for pointer authentication"
 	default y
-	depends on (CC_HAS_SIGN_RETURN_ADDRESS || CC_HAS_BRANCH_PROT_PAC_RET) && AS_HAS_PAC
-	# Modern compilers insert a .note.gnu.property section note for PAC
-	# which is only understood by binutils starting with version 2.33.1.
-	depends on LD_IS_LLD || LD_VERSION >= 233010000 || (CC_IS_GCC && GCC_VERSION < 90100)
-	depends on !CC_IS_CLANG || AS_HAS_CFI_NEGATE_RA_STATE
-	depends on (!FUNCTION_GRAPH_TRACER || DYNAMIC_FTRACE_WITH_REGS)
 	help
 	  Pointer authentication (part of the ARMv8.3 Extensions) provides
 	  instructions for signing and authenticating pointers against secret
@@ -1491,13 +1485,6 @@ config ARM64_PTR_AUTH
 	  for each process at exec() time, with these keys being
 	  context-switched along with the process.
 
-	  If the compiler supports the -mbranch-protection or
-	  -msign-return-address flag (e.g. GCC 7 or later), then this option
-	  will also cause the kernel itself to be compiled with return address
-	  protection. In this case, and if the target hardware is known to
-	  support pointer authentication, then CONFIG_STACKPROTECTOR can be
-	  disabled with minimal loss of protection.
-
 	  The feature is detected at runtime. If the feature is not present in
 	  hardware it will not be advertised to userspace/KVM guest nor will it
 	  be enabled.
@@ -1508,6 +1495,24 @@ config ARM64_PTR_AUTH
 	  but with the feature disabled. On such a system, this option should
 	  not be selected.
 
+config ARM64_PTR_AUTH_KERNEL
+	bool "Use pointer authentication for kernel"
+	default y
+	depends on ARM64_PTR_AUTH
+	depends on (CC_HAS_SIGN_RETURN_ADDRESS || CC_HAS_BRANCH_PROT_PAC_RET) && AS_HAS_PAC
+	# Modern compilers insert a .note.gnu.property section note for PAC
+	# which is only understood by binutils starting with version 2.33.1.
+	depends on LD_IS_LLD || LD_VERSION >= 233010000 || (CC_IS_GCC && GCC_VERSION < 90100)
+	depends on !CC_IS_CLANG || AS_HAS_CFI_NEGATE_RA_STATE
+	depends on (!FUNCTION_GRAPH_TRACER || DYNAMIC_FTRACE_WITH_REGS)
+	help
+	  If the compiler supports the -mbranch-protection or
+	  -msign-return-address flag (e.g. GCC 7 or later), then this option
+	  will cause the kernel itself to be compiled with return address
+	  protection. In this case, and if the target hardware is known to
+	  support pointer authentication, then CONFIG_STACKPROTECTOR can be
+	  disabled with minimal loss of protection.
+
 	  This feature works with FUNCTION_GRAPH_TRACER option only if
 	  DYNAMIC_FTRACE_WITH_REGS is enabled.
 
@@ -1599,7 +1604,7 @@ config ARM64_BTI_KERNEL
 	bool "Use Branch Target Identification for kernel"
 	default y
 	depends on ARM64_BTI
-	depends on ARM64_PTR_AUTH
+	depends on ARM64_PTR_AUTH_KERNEL
 	depends on CC_HAS_BRANCH_PROT_PAC_RET_BTI
 	# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94697
 	depends on !CC_IS_GCC || GCC_VERSION >= 100100
diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
index 90309208bb28..a50b5e8231a7 100644
--- a/arch/arm64/Makefile
+++ b/arch/arm64/Makefile
@@ -70,7 +70,7 @@ endif
 # off, this will be overridden if we are using branch protection.
 branch-prot-flags-y += $(call cc-option,-mbranch-protection=none)
 
-ifeq ($(CONFIG_ARM64_PTR_AUTH),y)
+ifeq ($(CONFIG_ARM64_PTR_AUTH_KERNEL),y)
 branch-prot-flags-$(CONFIG_CC_HAS_SIGN_RETURN_ADDRESS) := -msign-return-address=all
 # We enable additional protection for leaf functions as there is some
 # narrow potential for ROP protection benefits and no substantial
diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
index 301784463587..21c9f98f868d 100644
--- a/arch/arm64/kernel/asm-offsets.c
+++ b/arch/arm64/kernel/asm-offsets.c
@@ -45,6 +45,8 @@ 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));
+#endif
+#ifdef CONFIG_ARM64_PTR_AUTH_KERNEL
   DEFINE(THREAD_KEYS_KERNEL,	offsetof(struct task_struct, thread.keys_kernel));
 #endif
 #ifdef CONFIG_ARM64_MTE
diff --git a/drivers/misc/lkdtm/bugs.c b/drivers/misc/lkdtm/bugs.c
index 110f5a8538e9..b5b809b3e1ef 100644
--- a/drivers/misc/lkdtm/bugs.c
+++ b/drivers/misc/lkdtm/bugs.c
@@ -446,7 +446,7 @@ void lkdtm_DOUBLE_FAULT(void)
 #ifdef CONFIG_ARM64
 static noinline void change_pac_parameters(void)
 {
-	if (IS_ENABLED(CONFIG_ARM64_PTR_AUTH)) {
+	if (IS_ENABLED(CONFIG_ARM64_PTR_AUTH_KERNEL)) {
 		/* Reset the keys of current task */
 		ptrauth_thread_init_kernel(current);
 		ptrauth_thread_switch_kernel(current);
@@ -460,8 +460,8 @@ noinline void lkdtm_CORRUPT_PAC(void)
 #define CORRUPT_PAC_ITERATE	10
 	int i;
 
-	if (!IS_ENABLED(CONFIG_ARM64_PTR_AUTH))
-		pr_err("FAIL: kernel not built with CONFIG_ARM64_PTR_AUTH\n");
+	if (!IS_ENABLED(CONFIG_ARM64_PTR_AUTH_KERNEL))
+		pr_err("FAIL: kernel not built with CONFIG_ARM64_PTR_AUTH_KERNEL\n");
 
 	if (!system_supports_address_auth()) {
 		pr_err("FAIL: CPU lacks pointer authentication feature\n");
-- 
2.25.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] 6+ messages in thread

* [PATCH v3 2/2] arm64: Conditionally configure PTR_AUTH key of the kernel.
  2021-02-08 14:55 [PATCH v3 0/2] arm64: split ARM64_PTR_AUTH option to userspace and kernel Daniel Kiss
  2021-02-08 14:55 ` [PATCH v3 1/2] arm64: Add ARM64_PTR_AUTH_KERNEL config option Daniel Kiss
@ 2021-02-08 14:55 ` Daniel Kiss
  2021-03-29 14:51   ` Will Deacon
  1 sibling, 1 reply; 6+ messages in thread
From: Daniel Kiss @ 2021-02-08 14:55 UTC (permalink / raw)
  To: linux-arm-kernel, will; +Cc: Catalin.Marinas, pcc, Ard.Biesheuvel, Daniel Kiss

If the kernel is not compiled with CONFIG_ARM64_PTR_AUTH_KERNEL=y,
then no PACI/AUTI instructions are expected while the kernel is running
so the kernel's key will not be used. Write of a system regiters
is expensive therefore avoid it not required.

Signed-off-by: Daniel Kiss <daniel.kiss@arm.com>
---
 arch/arm64/include/asm/asm_pointer_auth.h | 63 +++++++++++++----------
 arch/arm64/include/asm/pointer_auth.h     | 20 +++++--
 arch/arm64/include/asm/processor.h        |  2 +
 arch/arm64/kernel/asm-offsets.c           |  2 +
 4 files changed, 54 insertions(+), 33 deletions(-)

diff --git a/arch/arm64/include/asm/asm_pointer_auth.h b/arch/arm64/include/asm/asm_pointer_auth.h
index 52dead2a8640..413cc36fdd5c 100644
--- a/arch/arm64/include/asm/asm_pointer_auth.h
+++ b/arch/arm64/include/asm/asm_pointer_auth.h
@@ -7,6 +7,38 @@
 #include <asm/cpufeature.h>
 #include <asm/sysreg.h>
 
+#ifdef CONFIG_ARM64_PTR_AUTH_KERNEL
+	.macro __ptrauth_keys_install_kernel_nosync tsk, tmp1, tmp2, tmp3
+	mov	\tmp1, #THREAD_KEYS_KERNEL
+	add	\tmp1, \tsk, \tmp1
+	ldp	\tmp2, \tmp3, [\tmp1, #PTRAUTH_KERNEL_KEY_APIA]
+	msr_s	SYS_APIAKEYLO_EL1, \tmp2
+	msr_s	SYS_APIAKEYHI_EL1, \tmp3
+	.endm
+
+	.macro ptrauth_keys_install_kernel_nosync tsk, tmp1, tmp2, tmp3
+alternative_if ARM64_HAS_ADDRESS_AUTH
+	__ptrauth_keys_install_kernel_nosync \tsk, \tmp1, \tmp2, \tmp3
+alternative_else_nop_endif
+	.endm
+
+	.macro ptrauth_keys_install_kernel tsk, tmp1, tmp2, tmp3
+alternative_if ARM64_HAS_ADDRESS_AUTH
+	__ptrauth_keys_install_kernel_nosync \tsk, \tmp1, \tmp2, \tmp3
+	isb
+alternative_else_nop_endif
+	.endm
+
+#else /* CONFIG_ARM64_PTR_AUTH_KERNEL */
+
+	.macro ptrauth_keys_install_kernel_nosync tsk, tmp1, tmp2, tmp3
+	.endm
+
+	.macro ptrauth_keys_install_kernel tsk, tmp1, tmp2, tmp3
+	.endm
+
+#endif /* CONFIG_ARM64_PTR_AUTH_KERNEL */
+
 #ifdef CONFIG_ARM64_PTR_AUTH
 /*
  * thread.keys_user.ap* as offset exceeds the #imm offset range
@@ -39,27 +71,6 @@ alternative_if ARM64_HAS_GENERIC_AUTH
 alternative_else_nop_endif
 	.endm
 
-	.macro __ptrauth_keys_install_kernel_nosync tsk, tmp1, tmp2, tmp3
-	mov	\tmp1, #THREAD_KEYS_KERNEL
-	add	\tmp1, \tsk, \tmp1
-	ldp	\tmp2, \tmp3, [\tmp1, #PTRAUTH_KERNEL_KEY_APIA]
-	msr_s	SYS_APIAKEYLO_EL1, \tmp2
-	msr_s	SYS_APIAKEYHI_EL1, \tmp3
-	.endm
-
-	.macro ptrauth_keys_install_kernel_nosync tsk, tmp1, tmp2, tmp3
-alternative_if ARM64_HAS_ADDRESS_AUTH
-	__ptrauth_keys_install_kernel_nosync \tsk, \tmp1, \tmp2, \tmp3
-alternative_else_nop_endif
-	.endm
-
-	.macro ptrauth_keys_install_kernel tsk, tmp1, tmp2, tmp3
-alternative_if ARM64_HAS_ADDRESS_AUTH
-	__ptrauth_keys_install_kernel_nosync \tsk, \tmp1, \tmp2, \tmp3
-	isb
-alternative_else_nop_endif
-	.endm
-
 	.macro __ptrauth_keys_init_cpu tsk, tmp1, tmp2, tmp3
 	mrs	\tmp1, id_aa64isar1_el1
 	ubfx	\tmp1, \tmp1, #ID_AA64ISAR1_APA_SHIFT, #8
@@ -69,7 +80,9 @@ alternative_else_nop_endif
 	mrs	\tmp2, sctlr_el1
 	orr	\tmp2, \tmp2, \tmp1
 	msr	sctlr_el1, \tmp2
+#ifdef CONFIG_ARM64_PTR_AUTH_KERNEL
 	__ptrauth_keys_install_kernel_nosync \tsk, \tmp1, \tmp2, \tmp3
+#endif
 	isb
 .Lno_addr_auth\@:
 	.endm
@@ -82,17 +95,11 @@ alternative_else_nop_endif
 .Lno_addr_auth\@:
 	.endm
 
-#else /* CONFIG_ARM64_PTR_AUTH */
+#else /* !CONFIG_ARM64_PTR_AUTH */
 
 	.macro ptrauth_keys_install_user tsk, tmp1, tmp2, tmp3
 	.endm
 
-	.macro ptrauth_keys_install_kernel_nosync 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 c6b4f0603024..b34aebb95757 100644
--- a/arch/arm64/include/asm/pointer_auth.h
+++ b/arch/arm64/include/asm/pointer_auth.h
@@ -30,9 +30,11 @@ struct ptrauth_keys_user {
 	struct ptrauth_key apga;
 };
 
+#ifdef CONFIG_ARM64_PTR_AUTH_KERNEL
 struct ptrauth_keys_kernel {
 	struct ptrauth_key apia;
 };
+#endif
 
 static inline void ptrauth_keys_init_user(struct ptrauth_keys_user *keys)
 {
@@ -54,6 +56,8 @@ do {								\
 	write_sysreg_s(__pki_v.hi, SYS_ ## k ## KEYHI_EL1);	\
 } while (0)
 
+#ifdef CONFIG_ARM64_PTR_AUTH_KERNEL
+
 static __always_inline void ptrauth_keys_init_kernel(struct ptrauth_keys_kernel *keys)
 {
 	if (system_supports_address_auth())
@@ -69,6 +73,8 @@ static __always_inline void ptrauth_keys_switch_kernel(struct ptrauth_keys_kerne
 	isb();
 }
 
+#endif /* CONFIG_ARM64_PTR_AUTH_KERNEL */
+
 extern int ptrauth_prctl_reset_keys(struct task_struct *tsk, unsigned long arg);
 
 static inline unsigned long ptrauth_strip_insn_pac(unsigned long ptr)
@@ -78,17 +84,21 @@ static inline unsigned long ptrauth_strip_insn_pac(unsigned long ptr)
 
 #define ptrauth_thread_init_user(tsk)					\
 	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)
+#endif /* CONFIG_ARM64_PTR_AUTH */
+
+#ifdef CONFIG_ARM64_PTR_AUTH_KERNEL
+#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
 #define ptrauth_thread_init_kernel(tsk)
 #define ptrauth_thread_switch_kernel(tsk)
-#endif /* CONFIG_ARM64_PTR_AUTH */
+#endif /* CONFIG_ARM64_PTR_AUTH_KERNEL */
 
 #endif /* __ASM_POINTER_AUTH_H */
diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h
index ca2cd75d3286..318e0e7cf9e1 100644
--- a/arch/arm64/include/asm/processor.h
+++ b/arch/arm64/include/asm/processor.h
@@ -148,8 +148,10 @@ struct thread_struct {
 	struct debug_info	debug;		/* debugging */
 #ifdef CONFIG_ARM64_PTR_AUTH
 	struct ptrauth_keys_user	keys_user;
+#ifdef CONFIG_ARM64_PTR_AUTH_KERNEL
 	struct ptrauth_keys_kernel	keys_kernel;
 #endif
+#endif
 #ifdef CONFIG_ARM64_MTE
 	u64			sctlr_tcf0;
 	u64			gcr_user_excl;
diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
index 21c9f98f868d..3b58804e4224 100644
--- a/arch/arm64/kernel/asm-offsets.c
+++ b/arch/arm64/kernel/asm-offsets.c
@@ -150,7 +150,9 @@ int main(void)
   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));
+#ifdef CONFIG_ARM64_PTR_AUTH_KERNEL
   DEFINE(PTRAUTH_KERNEL_KEY_APIA,	offsetof(struct ptrauth_keys_kernel, apia));
+#endif
   BLANK();
 #endif
   return 0;
-- 
2.25.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] 6+ messages in thread

* Re: [PATCH v3 1/2] arm64: Add ARM64_PTR_AUTH_KERNEL config option
  2021-02-08 14:55 ` [PATCH v3 1/2] arm64: Add ARM64_PTR_AUTH_KERNEL config option Daniel Kiss
@ 2021-03-29 14:51   ` Will Deacon
  0 siblings, 0 replies; 6+ messages in thread
From: Will Deacon @ 2021-03-29 14:51 UTC (permalink / raw)
  To: Daniel Kiss; +Cc: linux-arm-kernel, Ard.Biesheuvel, Catalin.Marinas, pcc

On Mon, Feb 08, 2021 at 03:55:53PM +0100, Daniel Kiss wrote:
> This patch add the ARM64_PTR_AUTH_KERNEL config and deals with the
> build aspect of it.
> 
> Userspace support has no dependency on the toolchain therefore all
> toolchain checks and build flags are controlled the new config
> option.
> The default config behavior will not be changed.
> 
> Signed-off-by: Daniel Kiss <daniel.kiss@arm.com>
> ---
>  arch/arm64/Kconfig              | 33 +++++++++++++++++++--------------
>  arch/arm64/Makefile             |  2 +-
>  arch/arm64/kernel/asm-offsets.c |  2 ++
>  drivers/misc/lkdtm/bugs.c       |  6 +++---
>  4 files changed, 25 insertions(+), 18 deletions(-)

Looks good to me:

Acked-by: Will Deacon <will@kernel.org>

Will

_______________________________________________
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] 6+ messages in thread

* Re: [PATCH v3 2/2] arm64: Conditionally configure PTR_AUTH key of the kernel.
  2021-02-08 14:55 ` [PATCH v3 2/2] arm64: Conditionally configure PTR_AUTH key of the kernel Daniel Kiss
@ 2021-03-29 14:51   ` Will Deacon
  2021-04-12 20:17     ` Daniel Kiss
  0 siblings, 1 reply; 6+ messages in thread
From: Will Deacon @ 2021-03-29 14:51 UTC (permalink / raw)
  To: Daniel Kiss; +Cc: linux-arm-kernel, Ard.Biesheuvel, Catalin.Marinas, pcc

On Mon, Feb 08, 2021 at 03:55:54PM +0100, Daniel Kiss wrote:
> If the kernel is not compiled with CONFIG_ARM64_PTR_AUTH_KERNEL=y,
> then no PACI/AUTI instructions are expected while the kernel is running
> so the kernel's key will not be used. Write of a system regiters
> is expensive therefore avoid it not required.

What happens if somebody tries to load a module built with PAC into a kernel
where CONFIG_ARM64_PTR_AUTH_KERNEL=n? Do we reject the module?

I'm not sure how much we care, but I'm a bit worried that it might not go
"obviously" wrong.

> Signed-off-by: Daniel Kiss <daniel.kiss@arm.com>
> ---
>  arch/arm64/include/asm/asm_pointer_auth.h | 63 +++++++++++++----------
>  arch/arm64/include/asm/pointer_auth.h     | 20 +++++--
>  arch/arm64/include/asm/processor.h        |  2 +
>  arch/arm64/kernel/asm-offsets.c           |  2 +
>  4 files changed, 54 insertions(+), 33 deletions(-)

[...]

> diff --git a/arch/arm64/include/asm/pointer_auth.h b/arch/arm64/include/asm/pointer_auth.h
> index c6b4f0603024..b34aebb95757 100644
> --- a/arch/arm64/include/asm/pointer_auth.h
> +++ b/arch/arm64/include/asm/pointer_auth.h
> @@ -30,9 +30,11 @@ struct ptrauth_keys_user {
>  	struct ptrauth_key apga;
>  };
>  
> +#ifdef CONFIG_ARM64_PTR_AUTH_KERNEL
>  struct ptrauth_keys_kernel {
>  	struct ptrauth_key apia;
>  };
> +#endif
>  
>  static inline void ptrauth_keys_init_user(struct ptrauth_keys_user *keys)
>  {
> @@ -54,6 +56,8 @@ do {								\
>  	write_sysreg_s(__pki_v.hi, SYS_ ## k ## KEYHI_EL1);	\
>  } while (0)
>  
> +#ifdef CONFIG_ARM64_PTR_AUTH_KERNEL
> +
>  static __always_inline void ptrauth_keys_init_kernel(struct ptrauth_keys_kernel *keys)
>  {
>  	if (system_supports_address_auth())
> @@ -69,6 +73,8 @@ static __always_inline void ptrauth_keys_switch_kernel(struct ptrauth_keys_kerne
>  	isb();
>  }
>  
> +#endif /* CONFIG_ARM64_PTR_AUTH_KERNEL */

Can you group this with the struct and avoid having two #ifdef blocks?

Will

_______________________________________________
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] 6+ messages in thread

* Re: [PATCH v3 2/2] arm64: Conditionally configure PTR_AUTH key of the kernel.
  2021-03-29 14:51   ` Will Deacon
@ 2021-04-12 20:17     ` Daniel Kiss
  0 siblings, 0 replies; 6+ messages in thread
From: Daniel Kiss @ 2021-04-12 20:17 UTC (permalink / raw)
  To: Will Deacon; +Cc: Linux ARM, Ard Biesheuvel, Catalin Marinas, pcc



> On 29 Mar 2021, at 16:51, Will Deacon <will@kernel.org> wrote:
> 
> On Mon, Feb 08, 2021 at 03:55:54PM +0100, Daniel Kiss wrote:
>> If the kernel is not compiled with CONFIG_ARM64_PTR_AUTH_KERNEL=y,
>> then no PACI/AUTI instructions are expected while the kernel is running
>> so the kernel's key will not be used. Write of a system regiters
>> is expensive therefore avoid it not required.
> 
> What happens if somebody tries to load a module built with PAC into a kernel
> where CONFIG_ARM64_PTR_AUTH_KERNEL=n? Do we reject the module?
I think it will be loaded, but actually nothing will happen because then the instruction
will be just NOPs. Other way will work as well.
Enforcement will be trick because module might sneak the compiler flag in that overrides
the kconfig.

> I'm not sure how much we care, but I'm a bit worried that it might not go
> "obviously" wrong.
> 
>> Signed-off-by: Daniel Kiss <daniel.kiss@arm.com>
>> ---
>> arch/arm64/include/asm/asm_pointer_auth.h | 63 +++++++++++++----------
>> arch/arm64/include/asm/pointer_auth.h     | 20 +++++--
>> arch/arm64/include/asm/processor.h        |  2 +
>> arch/arm64/kernel/asm-offsets.c           |  2 +
>> 4 files changed, 54 insertions(+), 33 deletions(-)
> 
> [...]
> 
>> diff --git a/arch/arm64/include/asm/pointer_auth.h b/arch/arm64/include/asm/pointer_auth.h
>> index c6b4f0603024..b34aebb95757 100644
>> --- a/arch/arm64/include/asm/pointer_auth.h
>> +++ b/arch/arm64/include/asm/pointer_auth.h
>> @@ -30,9 +30,11 @@ struct ptrauth_keys_user {
>> 	struct ptrauth_key apga;
>> };
>> 
>> +#ifdef CONFIG_ARM64_PTR_AUTH_KERNEL
>> struct ptrauth_keys_kernel {
>> 	struct ptrauth_key apia;
>> };
>> +#endif
>> 
>> static inline void ptrauth_keys_init_user(struct ptrauth_keys_user *keys)
>> {
>> @@ -54,6 +56,8 @@ do {								\
>> 	write_sysreg_s(__pki_v.hi, SYS_ ## k ## KEYHI_EL1);	\
>> } while (0)
>> 
>> +#ifdef CONFIG_ARM64_PTR_AUTH_KERNEL
>> +
>> static __always_inline void ptrauth_keys_init_kernel(struct ptrauth_keys_kernel *keys)
>> {
>> 	if (system_supports_address_auth())
>> @@ -69,6 +73,8 @@ static __always_inline void ptrauth_keys_switch_kernel(struct ptrauth_keys_kerne
>> 	isb();
>> }
>> 
>> +#endif /* CONFIG_ARM64_PTR_AUTH_KERNEL */
> 
> Can you group this with the struct and avoid having two #ifdef blocks?
Sure, I'll send a version.

> 
> Will


_______________________________________________
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] 6+ messages in thread

end of thread, other threads:[~2021-04-12 20:19 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-08 14:55 [PATCH v3 0/2] arm64: split ARM64_PTR_AUTH option to userspace and kernel Daniel Kiss
2021-02-08 14:55 ` [PATCH v3 1/2] arm64: Add ARM64_PTR_AUTH_KERNEL config option Daniel Kiss
2021-03-29 14:51   ` Will Deacon
2021-02-08 14:55 ` [PATCH v3 2/2] arm64: Conditionally configure PTR_AUTH key of the kernel Daniel Kiss
2021-03-29 14:51   ` Will Deacon
2021-04-12 20:17     ` Daniel Kiss

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.