arm64: split ARM64_PTR_AUTH option to userspace and kernel configs.

arm64: split ARM64_PTR_AUTH option to userspace and kernel configs.

[Daniel Kiss]

In some situation it is useful to build the kernel without pointer
authentication. This series breaks the config option into two flags, 
one for the user space and one for the kernel.
The default config remains the same after the patches.



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

[PATCH 1/2] arm64: Add ARM64_PTR_AUTH_KERNEL config option

[Daniel Kiss]

This new option makes possible to build the kernel with pointer
authentication support for the user space while the kernel is not built
with the pointer authentication. We have similar config structure for
BTI.

The default configuration will be the same after this patch.

Signed-off-by: Daniel Kiss <daniel.kiss@arm.com>
---
 arch/arm64/Kconfig        | 26 +++++++++++++++++---------
 arch/arm64/Makefile       |  2 +-
 drivers/misc/lkdtm/bugs.c |  6 +++---
 3 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index a6b5b7ef40ae..4e88dbbb16d9 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -1501,7 +1501,6 @@ config ARM64_PTR_AUTH
 	# 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
@@ -1513,13 +1512,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.
@@ -1530,6 +1522,22 @@ 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 "Enable support for pointer authentication for kernel"
+	default y
+	depends on ARM64_PTR_AUTH
+	depends on (!FUNCTION_GRAPH_TRACER || DYNAMIC_FTRACE_WITH_REGS)
+	help
+	  Build the kernel with return address protection by
+	  pointer authentication.
+
+	  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.
 
@@ -1618,7 +1626,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 6a87d592bd00..6e5d9de8c2b3 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/drivers/misc/lkdtm/bugs.c b/drivers/misc/lkdtm/bugs.c
index a0675d4154d2..439fa33ae413 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.17.1


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

[PATCH 2/2] arm64: Configure kernel's PTR_AUTH key when it is built with PTR_AUTH.

[Daniel Kiss]

If the kernel is not compiled with CONFIG_ARM64_PTR_AUTH_KERNEL,
then the kernel does not need a key and kernel's key could be disabled.

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

diff --git a/arch/arm64/include/asm/asm_pointer_auth.h b/arch/arm64/include/asm/asm_pointer_auth.h
index 52dead2a8640..af3d16027e8f 100644
--- a/arch/arm64/include/asm/asm_pointer_auth.h
+++ b/arch/arm64/include/asm/asm_pointer_auth.h
@@ -14,6 +14,12 @@
  * thread.keys_user.ap*.
  */
 	.macro ptrauth_keys_install_user tsk, tmp1, tmp2, tmp3
+#ifndef CONFIG_ARM64_PTR_AUTH_KERNEL
+	/* Reenable A key */
+	mrs	\tmp1, sctlr_el1
+	orr	\tmp1, \tmp1, SCTLR_ELx_ENIA
+	msr	sctlr_el1, \tmp1
+#endif
 	mov	\tmp1, #THREAD_KEYS_USER
 	add	\tmp1, \tsk, \tmp1
 alternative_if_not ARM64_HAS_ADDRESS_AUTH
@@ -39,6 +45,36 @@ alternative_if ARM64_HAS_GENERIC_AUTH
 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
+	cbz	\tmp1, .Lno_addr_auth\@
+#ifdef CONFIG_ARM64_PTR_AUTH_KERNEL
+	mov_q	\tmp1, (SCTLR_ELx_ENIA | SCTLR_ELx_ENIB | \
+			SCTLR_ELx_ENDA | SCTLR_ELx_ENDB)
+#else
+	mov_q	\tmp1, (SCTLR_ELx_ENIB | \
+			SCTLR_ELx_ENDA | SCTLR_ELx_ENDB)
+#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
+
+	.macro ptrauth_keys_init_cpu tsk, tmp1, tmp2, tmp3
+alternative_if_not ARM64_HAS_ADDRESS_AUTH
+	b	.Lno_addr_auth\@
+alternative_else_nop_endif
+	__ptrauth_keys_init_cpu \tsk, \tmp1, \tmp2, \tmp3
+.Lno_addr_auth\@:
+	.endm
+
+#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
@@ -60,29 +96,23 @@ alternative_if ARM64_HAS_ADDRESS_AUTH
 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
-	cbz	\tmp1, .Lno_addr_auth\@
-	mov_q	\tmp1, (SCTLR_ELx_ENIA | SCTLR_ELx_ENIB | \
-			SCTLR_ELx_ENDA | SCTLR_ELx_ENDB)
-	mrs	\tmp2, sctlr_el1
-	orr	\tmp2, \tmp2, \tmp1
-	msr	sctlr_el1, \tmp2
-	__ptrauth_keys_install_kernel_nosync \tsk, \tmp1, \tmp2, \tmp3
-	isb
-.Lno_addr_auth\@:
+#else /* CONFIG_ARM64_PTR_AUTH_KERNEL */
+
+	.macro ptrauth_keys_install_kernel_nosync tsk, tmp1, tmp2, tmp3
+	mrs	\tmp1, sctlr_el1
+	and	\tmp1, \tmp1, ~SCTLR_ELx_ENIA
+	msr	sctlr_el1, \tmp1
 	.endm
 
-	.macro ptrauth_keys_init_cpu tsk, tmp1, tmp2, tmp3
-alternative_if_not ARM64_HAS_ADDRESS_AUTH
-	b	.Lno_addr_auth\@
-alternative_else_nop_endif
-	__ptrauth_keys_init_cpu \tsk, \tmp1, \tmp2, \tmp3
-.Lno_addr_auth\@:
+	.macro ptrauth_keys_install_kernel tsk, tmp1, tmp2, tmp3
+	mrs	\tmp1, sctlr_el1
+	and	\tmp1, \tmp1, ~SCTLR_ELx_ENIA
+	msr	sctlr_el1, \tmp1
 	.endm
 
-#else /* CONFIG_ARM64_PTR_AUTH */
+#endif /* CONFIG_ARM64_PTR_AUTH_KERNEL */
+
+#else /* !CONFIG_ARM64_PTR_AUTH */
 
 	.macro ptrauth_keys_install_user tsk, tmp1, tmp2, tmp3
 	.endm
diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h
index fce8cbecd6bc..e20888b321e3 100644
--- a/arch/arm64/include/asm/processor.h
+++ b/arch/arm64/include/asm/processor.h
@@ -150,8 +150,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_incl;
diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
index 7d32fc959b1a..cb7965a9f505 100644
--- a/arch/arm64/kernel/asm-offsets.c
+++ b/arch/arm64/kernel/asm-offsets.c
@@ -46,7 +46,9 @@ 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));
+#ifdef CONFIG_ARM64_PTR_AUTH_KERNEL
   DEFINE(THREAD_KEYS_KERNEL,	offsetof(struct task_struct, thread.keys_kernel));
+#endif
 #endif
   BLANK();
   DEFINE(S_X0,			offsetof(struct pt_regs, regs[0]));
@@ -141,7 +143,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.17.1


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

Re: [PATCH 2/2] arm64: Configure kernel's PTR_AUTH key when it is built with PTR_AUTH.

[Peter Collingbourne]

On Mon, Dec 7, 2020 at 2:46 PM Daniel Kiss <daniel.kiss@arm.com> wrote:
>
> If the kernel is not compiled with CONFIG_ARM64_PTR_AUTH_KERNEL,
> then the kernel does not need a key and kernel's key could be disabled.
>
> Signed-off-by: Daniel Kiss <daniel.kiss@arm.com>
> ---
>  arch/arm64/include/asm/asm_pointer_auth.h | 68 ++++++++++++++++-------
>  arch/arm64/include/asm/processor.h        |  2 +
>  arch/arm64/kernel/asm-offsets.c           |  4 ++
>  3 files changed, 55 insertions(+), 19 deletions(-)
>
> diff --git a/arch/arm64/include/asm/asm_pointer_auth.h b/arch/arm64/include/asm/asm_pointer_auth.h
> index 52dead2a8640..af3d16027e8f 100644
> --- a/arch/arm64/include/asm/asm_pointer_auth.h
> +++ b/arch/arm64/include/asm/asm_pointer_auth.h
> @@ -14,6 +14,12 @@
>   * thread.keys_user.ap*.
>   */
>         .macro ptrauth_keys_install_user tsk, tmp1, tmp2, tmp3
> +#ifndef CONFIG_ARM64_PTR_AUTH_KERNEL
> +       /* Reenable A key */
> +       mrs     \tmp1, sctlr_el1
> +       orr     \tmp1, \tmp1, SCTLR_ELx_ENIA
> +       msr     sctlr_el1, \tmp1
> +#endif

We should avoid an unconditional MSR on exit like this as it is
expensive (for my PR_PAC_SET_ENABLED_KEYS series I measured the cost
of entry/exit MSR as 43.7ns on Cortex-A75 and 33.0ns on Apple M1). In
that series I take care not to touch SCTLR_EL1 unless necessary.
Likewise for the MSRs on entry below.

>         mov     \tmp1, #THREAD_KEYS_USER
>         add     \tmp1, \tsk, \tmp1
>  alternative_if_not ARM64_HAS_ADDRESS_AUTH
> @@ -39,6 +45,36 @@ alternative_if ARM64_HAS_GENERIC_AUTH
>  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
> +       cbz     \tmp1, .Lno_addr_auth\@
> +#ifdef CONFIG_ARM64_PTR_AUTH_KERNEL
> +       mov_q   \tmp1, (SCTLR_ELx_ENIA | SCTLR_ELx_ENIB | \
> +                       SCTLR_ELx_ENDA | SCTLR_ELx_ENDB)
> +#else
> +       mov_q   \tmp1, (SCTLR_ELx_ENIB | \
> +                       SCTLR_ELx_ENDA | SCTLR_ELx_ENDB)
> +#endif

If you leave IA enabled here then you shouldn't need to MSR on entry
and exit. If no PAC instructions are used in the kernel then it
shouldn't matter if it is left enabled.

Peter

> +       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
> +
> +       .macro ptrauth_keys_init_cpu tsk, tmp1, tmp2, tmp3
> +alternative_if_not ARM64_HAS_ADDRESS_AUTH
> +       b       .Lno_addr_auth\@
> +alternative_else_nop_endif
> +       __ptrauth_keys_init_cpu \tsk, \tmp1, \tmp2, \tmp3
> +.Lno_addr_auth\@:
> +       .endm
> +
> +#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
> @@ -60,29 +96,23 @@ alternative_if ARM64_HAS_ADDRESS_AUTH
>  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
> -       cbz     \tmp1, .Lno_addr_auth\@
> -       mov_q   \tmp1, (SCTLR_ELx_ENIA | SCTLR_ELx_ENIB | \
> -                       SCTLR_ELx_ENDA | SCTLR_ELx_ENDB)
> -       mrs     \tmp2, sctlr_el1
> -       orr     \tmp2, \tmp2, \tmp1
> -       msr     sctlr_el1, \tmp2
> -       __ptrauth_keys_install_kernel_nosync \tsk, \tmp1, \tmp2, \tmp3
> -       isb
> -.Lno_addr_auth\@:
> +#else /* CONFIG_ARM64_PTR_AUTH_KERNEL */
> +
> +       .macro ptrauth_keys_install_kernel_nosync tsk, tmp1, tmp2, tmp3
> +       mrs     \tmp1, sctlr_el1
> +       and     \tmp1, \tmp1, ~SCTLR_ELx_ENIA
> +       msr     sctlr_el1, \tmp1
>         .endm
>
> -       .macro ptrauth_keys_init_cpu tsk, tmp1, tmp2, tmp3
> -alternative_if_not ARM64_HAS_ADDRESS_AUTH
> -       b       .Lno_addr_auth\@
> -alternative_else_nop_endif
> -       __ptrauth_keys_init_cpu \tsk, \tmp1, \tmp2, \tmp3
> -.Lno_addr_auth\@:
> +       .macro ptrauth_keys_install_kernel tsk, tmp1, tmp2, tmp3
> +       mrs     \tmp1, sctlr_el1
> +       and     \tmp1, \tmp1, ~SCTLR_ELx_ENIA
> +       msr     sctlr_el1, \tmp1
>         .endm
>
> -#else /* CONFIG_ARM64_PTR_AUTH */
> +#endif /* CONFIG_ARM64_PTR_AUTH_KERNEL */
> +
> +#else /* !CONFIG_ARM64_PTR_AUTH */
>
>         .macro ptrauth_keys_install_user tsk, tmp1, tmp2, tmp3
>         .endm
> diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h
> index fce8cbecd6bc..e20888b321e3 100644
> --- a/arch/arm64/include/asm/processor.h
> +++ b/arch/arm64/include/asm/processor.h
> @@ -150,8 +150,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_incl;
> diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
> index 7d32fc959b1a..cb7965a9f505 100644
> --- a/arch/arm64/kernel/asm-offsets.c
> +++ b/arch/arm64/kernel/asm-offsets.c
> @@ -46,7 +46,9 @@ 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));
> +#ifdef CONFIG_ARM64_PTR_AUTH_KERNEL
>    DEFINE(THREAD_KEYS_KERNEL,   offsetof(struct task_struct, thread.keys_kernel));
> +#endif
>  #endif
>    BLANK();
>    DEFINE(S_X0,                 offsetof(struct pt_regs, regs[0]));
> @@ -141,7 +143,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.17.1
>

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

Re: [PATCH 2/2] arm64: Configure kernel's PTR_AUTH key when it is built with PTR_AUTH.

[Catalin Marinas]

On Mon, Dec 07, 2020 at 03:07:07PM -0800, Peter Collingbourne wrote:
> On Mon, Dec 7, 2020 at 2:46 PM Daniel Kiss <daniel.kiss@arm.com> wrote:
> > If the kernel is not compiled with CONFIG_ARM64_PTR_AUTH_KERNEL,
> > then the kernel does not need a key and kernel's key could be disabled.
> >
> > Signed-off-by: Daniel Kiss <daniel.kiss@arm.com>
> > ---
> >  arch/arm64/include/asm/asm_pointer_auth.h | 68 ++++++++++++++++-------
> >  arch/arm64/include/asm/processor.h        |  2 +
> >  arch/arm64/kernel/asm-offsets.c           |  4 ++
> >  3 files changed, 55 insertions(+), 19 deletions(-)
> >
> > diff --git a/arch/arm64/include/asm/asm_pointer_auth.h b/arch/arm64/include/asm/asm_pointer_auth.h
> > index 52dead2a8640..af3d16027e8f 100644
> > --- a/arch/arm64/include/asm/asm_pointer_auth.h
> > +++ b/arch/arm64/include/asm/asm_pointer_auth.h
> > @@ -14,6 +14,12 @@
> >   * thread.keys_user.ap*.
> >   */
> >         .macro ptrauth_keys_install_user tsk, tmp1, tmp2, tmp3
> > +#ifndef CONFIG_ARM64_PTR_AUTH_KERNEL
> > +       /* Reenable A key */
> > +       mrs     \tmp1, sctlr_el1
> > +       orr     \tmp1, \tmp1, SCTLR_ELx_ENIA
> > +       msr     sctlr_el1, \tmp1
> > +#endif
> 
> We should avoid an unconditional MSR on exit like this as it is
> expensive (for my PR_PAC_SET_ENABLED_KEYS series I measured the cost
> of entry/exit MSR as 43.7ns on Cortex-A75 and 33.0ns on Apple M1). In
> that series I take care not to touch SCTLR_EL1 unless necessary.
> Likewise for the MSRs on entry below.

I think that's how Daniel attempted the first (internal) version of
these patches. In theory you don't need to touch SCTLR_ELx_EN* at all as
long as the kernel does not use any PAC instructions. However, I was
a bit concerned about this and thought it's safer if, when
!CONFIG_ARM64_PTR_AUTH_KERNEL, the EnIA bit is cleared while in the
kernel.

If we can guarantee that the compiler does not generate any PAC
instructions (it may assume they are no-ops) and vendor modules don't
have such instructions either, we may be able to relax this.

-- 
Catalin

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

Re: [PATCH 2/2] arm64: Configure kernel's PTR_AUTH key when it is built with PTR_AUTH.

[Peter Collingbourne]

On Tue, Dec 8, 2020 at 3:00 AM Catalin Marinas <catalin.marinas@arm.com> wrote:
>
> On Mon, Dec 07, 2020 at 03:07:07PM -0800, Peter Collingbourne wrote:
> > On Mon, Dec 7, 2020 at 2:46 PM Daniel Kiss <daniel.kiss@arm.com> wrote:
> > > If the kernel is not compiled with CONFIG_ARM64_PTR_AUTH_KERNEL,
> > > then the kernel does not need a key and kernel's key could be disabled.
> > >
> > > Signed-off-by: Daniel Kiss <daniel.kiss@arm.com>
> > > ---
> > >  arch/arm64/include/asm/asm_pointer_auth.h | 68 ++++++++++++++++-------
> > >  arch/arm64/include/asm/processor.h        |  2 +
> > >  arch/arm64/kernel/asm-offsets.c           |  4 ++
> > >  3 files changed, 55 insertions(+), 19 deletions(-)
> > >
> > > diff --git a/arch/arm64/include/asm/asm_pointer_auth.h b/arch/arm64/include/asm/asm_pointer_auth.h
> > > index 52dead2a8640..af3d16027e8f 100644
> > > --- a/arch/arm64/include/asm/asm_pointer_auth.h
> > > +++ b/arch/arm64/include/asm/asm_pointer_auth.h
> > > @@ -14,6 +14,12 @@
> > >   * thread.keys_user.ap*.
> > >   */
> > >         .macro ptrauth_keys_install_user tsk, tmp1, tmp2, tmp3
> > > +#ifndef CONFIG_ARM64_PTR_AUTH_KERNEL
> > > +       /* Reenable A key */
> > > +       mrs     \tmp1, sctlr_el1
> > > +       orr     \tmp1, \tmp1, SCTLR_ELx_ENIA
> > > +       msr     sctlr_el1, \tmp1
> > > +#endif
> >
> > We should avoid an unconditional MSR on exit like this as it is
> > expensive (for my PR_PAC_SET_ENABLED_KEYS series I measured the cost
> > of entry/exit MSR as 43.7ns on Cortex-A75 and 33.0ns on Apple M1). In
> > that series I take care not to touch SCTLR_EL1 unless necessary.
> > Likewise for the MSRs on entry below.
>
> I think that's how Daniel attempted the first (internal) version of
> these patches. In theory you don't need to touch SCTLR_ELx_EN* at all as
> long as the kernel does not use any PAC instructions. However, I was
> a bit concerned about this and thought it's safer if, when
> !CONFIG_ARM64_PTR_AUTH_KERNEL, the EnIA bit is cleared while in the
> kernel.
>
> If we can guarantee that the compiler does not generate any PAC
> instructions (it may assume they are no-ops) and vendor modules don't
> have such instructions either, we may be able to relax this.

The way I see it it isn't too different from the current prohibition
on using IB in the kernel (and to a lesser extent DA/DB/GA since those
can't be accessed from nop-space as far as I'm aware), or NEON
instructions in most parts of the kernel, or the stack protector
cookie when building with -fno-stack-protector etc. i.e. if you do
that then you're breaking the ABI.

Is your concern that distributions may default to enabling
-mbranch-protection which would result in the PAC instructions being
used? To address that I think it is reasonable to expect the compiler
not to use PAC instructions when passing -mbranch-protection=none, and
if the compiler does so then that is a bug in the compiler.

Peter

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

Re: [PATCH 2/2] arm64: Configure kernel's PTR_AUTH key when it is built with PTR_AUTH.

[Will Deacon]

On Tue, Dec 08, 2020 at 11:33:33AM -0800, Peter Collingbourne wrote:
> On Tue, Dec 8, 2020 at 3:00 AM Catalin Marinas <catalin.marinas@arm.com> wrote:
> >
> > On Mon, Dec 07, 2020 at 03:07:07PM -0800, Peter Collingbourne wrote:
> > > On Mon, Dec 7, 2020 at 2:46 PM Daniel Kiss <daniel.kiss@arm.com> wrote:
> > > > If the kernel is not compiled with CONFIG_ARM64_PTR_AUTH_KERNEL,
> > > > then the kernel does not need a key and kernel's key could be disabled.
> > > >
> > > > Signed-off-by: Daniel Kiss <daniel.kiss@arm.com>
> > > > ---
> > > >  arch/arm64/include/asm/asm_pointer_auth.h | 68 ++++++++++++++++-------
> > > >  arch/arm64/include/asm/processor.h        |  2 +
> > > >  arch/arm64/kernel/asm-offsets.c           |  4 ++
> > > >  3 files changed, 55 insertions(+), 19 deletions(-)
> > > >
> > > > diff --git a/arch/arm64/include/asm/asm_pointer_auth.h b/arch/arm64/include/asm/asm_pointer_auth.h
> > > > index 52dead2a8640..af3d16027e8f 100644
> > > > --- a/arch/arm64/include/asm/asm_pointer_auth.h
> > > > +++ b/arch/arm64/include/asm/asm_pointer_auth.h
> > > > @@ -14,6 +14,12 @@
> > > >   * thread.keys_user.ap*.
> > > >   */
> > > >         .macro ptrauth_keys_install_user tsk, tmp1, tmp2, tmp3
> > > > +#ifndef CONFIG_ARM64_PTR_AUTH_KERNEL
> > > > +       /* Reenable A key */
> > > > +       mrs     \tmp1, sctlr_el1
> > > > +       orr     \tmp1, \tmp1, SCTLR_ELx_ENIA
> > > > +       msr     sctlr_el1, \tmp1
> > > > +#endif
> > >
> > > We should avoid an unconditional MSR on exit like this as it is
> > > expensive (for my PR_PAC_SET_ENABLED_KEYS series I measured the cost
> > > of entry/exit MSR as 43.7ns on Cortex-A75 and 33.0ns on Apple M1). In
> > > that series I take care not to touch SCTLR_EL1 unless necessary.
> > > Likewise for the MSRs on entry below.
> >
> > I think that's how Daniel attempted the first (internal) version of
> > these patches. In theory you don't need to touch SCTLR_ELx_EN* at all as
> > long as the kernel does not use any PAC instructions. However, I was
> > a bit concerned about this and thought it's safer if, when
> > !CONFIG_ARM64_PTR_AUTH_KERNEL, the EnIA bit is cleared while in the
> > kernel.
> >
> > If we can guarantee that the compiler does not generate any PAC
> > instructions (it may assume they are no-ops) and vendor modules don't
> > have such instructions either, we may be able to relax this.
> 
> The way I see it it isn't too different from the current prohibition
> on using IB in the kernel (and to a lesser extent DA/DB/GA since those
> can't be accessed from nop-space as far as I'm aware), or NEON
> instructions in most parts of the kernel, or the stack protector
> cookie when building with -fno-stack-protector etc. i.e. if you do
> that then you're breaking the ABI.
> 
> Is your concern that distributions may default to enabling
> -mbranch-protection which would result in the PAC instructions being
> used? To address that I think it is reasonable to expect the compiler
> not to use PAC instructions when passing -mbranch-protection=none, and
> if the compiler does so then that is a bug in the compiler.

I'm inclined to agree. At the very least, I think we should start from a
position where we assume the compiler doesn't randomly emit these
instructions, and then we can revisit that decision in future if it turns
out to be wrong.

Will

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

Re: [PATCH 2/2] arm64: Configure kernel's PTR_AUTH key when it is built with PTR_AUTH.

[Daniel Kiss]

> On 9 Dec 2020, at 11:51, Will Deacon <will@kernel.org> wrote:
> 
> On Tue, Dec 08, 2020 at 11:33:33AM -0800, Peter Collingbourne wrote:
>> On Tue, Dec 8, 2020 at 3:00 AM Catalin Marinas <catalin.marinas@arm.com> wrote:
>>> 
>>> On Mon, Dec 07, 2020 at 03:07:07PM -0800, Peter Collingbourne wrote:
>>>> On Mon, Dec 7, 2020 at 2:46 PM Daniel Kiss <daniel.kiss@arm.com> wrote:
>>>>> If the kernel is not compiled with CONFIG_ARM64_PTR_AUTH_KERNEL,
>>>>> then the kernel does not need a key and kernel's key could be disabled.
>>>>> 
>>>>> Signed-off-by: Daniel Kiss <daniel.kiss@arm.com>
>>>>> ---
>>>>> arch/arm64/include/asm/asm_pointer_auth.h | 68 ++++++++++++++++-------
>>>>> arch/arm64/include/asm/processor.h        |  2 +
>>>>> arch/arm64/kernel/asm-offsets.c           |  4 ++
>>>>> 3 files changed, 55 insertions(+), 19 deletions(-)
>>>>> 
>>>>> diff --git a/arch/arm64/include/asm/asm_pointer_auth.h b/arch/arm64/include/asm/asm_pointer_auth.h
>>>>> index 52dead2a8640..af3d16027e8f 100644
>>>>> --- a/arch/arm64/include/asm/asm_pointer_auth.h
>>>>> +++ b/arch/arm64/include/asm/asm_pointer_auth.h
>>>>> @@ -14,6 +14,12 @@
>>>>>  * thread.keys_user.ap*.
>>>>>  */
>>>>>        .macro ptrauth_keys_install_user tsk, tmp1, tmp2, tmp3
>>>>> +#ifndef CONFIG_ARM64_PTR_AUTH_KERNEL
>>>>> +       /* Reenable A key */
>>>>> +       mrs     \tmp1, sctlr_el1
>>>>> +       orr     \tmp1, \tmp1, SCTLR_ELx_ENIA
>>>>> +       msr     sctlr_el1, \tmp1
>>>>> +#endif
>>>> 
>>>> We should avoid an unconditional MSR on exit like this as it is
>>>> expensive (for my PR_PAC_SET_ENABLED_KEYS series I measured the cost
>>>> of entry/exit MSR as 43.7ns on Cortex-A75 and 33.0ns on Apple M1). In
>>>> that series I take care not to touch SCTLR_EL1 unless necessary.
>>>> Likewise for the MSRs on entry below.
>>> 
>>> I think that's how Daniel attempted the first (internal) version of
>>> these patches. In theory you don't need to touch SCTLR_ELx_EN* at all as
>>> long as the kernel does not use any PAC instructions. However, I was
>>> a bit concerned about this and thought it's safer if, when
>>> !CONFIG_ARM64_PTR_AUTH_KERNEL, the EnIA bit is cleared while in the
>>> kernel.
>>> 
>>> If we can guarantee that the compiler does not generate any PAC
>>> instructions (it may assume they are no-ops) and vendor modules don't
>>> have such instructions either, we may be able to relax this.
>> 
>> The way I see it it isn't too different from the current prohibition
>> on using IB in the kernel (and to a lesser extent DA/DB/GA since those
>> can't be accessed from nop-space as far as I'm aware), or NEON
>> instructions in most parts of the kernel, or the stack protector
>> cookie when building with -fno-stack-protector etc. i.e. if you do
>> that then you're breaking the ABI.
>> 
>> Is your concern that distributions may default to enabling
>> -mbranch-protection which would result in the PAC instructions being
>> used? To address that I think it is reasonable to expect the compiler
>> not to use PAC instructions when passing -mbranch-protection=none, and
>> if the compiler does so then that is a bug in the compiler.
> 
> I'm inclined to agree. At the very least, I think we should start from a
> position where we assume the compiler doesn't randomly emit these
> instructions, and then we can revisit that decision in future if it turns
> out to be wrong.
> 

I agree the compiler shall not emit these instructions when not requested.
I have two corner cases to consider:
Assembly code may contain pac/aut instructions unconditionally, like:
https://elixir.bootlin.com/linux/v5.10-rc7/source/arch/arm64/crypto/poly1305-armv8.pl#L348

A module may be compiled against a kernel with CONFIG_ARM64_PTR_AUTH_KERNEL=y
but later it is loaded on a kernel which is built with CONFIG_ARM64_PTR_AUTH_KERNEL=n.
If the key is not disabled here, the CONFIG_ARM64_PTR_AUTH_KERNEL is 
part of the KMI otherwise not.

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

arm64: split ARM64_PTR_AUTH option to userspace and kernel

[Daniel Kiss]

As discussed the A-key left enabled, this makes the patch simpler too.
arch/arm64/crypto/poly1305-core.S_shipped contains PACISP/AUTISP
instructions but this code is called while the preeption is disabled,
therefore it won't cause any trouble.

v2:
- dropped the keychange/enablement for the kernel keys.



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

[PATCH v2 1/2] arm64: Add ARM64_PTR_AUTH_KERNEL config option

[Daniel Kiss]

This new option makes possible to build the kernel with pointer
authentication support for the user space while the kernel is not built
with the pointer authentication. There is a similar config structure for BTI.

The default configuration will be the same after this patch.

Signed-off-by: Daniel Kiss <daniel.kiss@arm.com>
---
 arch/arm64/Kconfig        | 26 +++++++++++++++++---------
 arch/arm64/Makefile       |  2 +-
 drivers/misc/lkdtm/bugs.c |  6 +++---
 3 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 75aefc9990ea..b8af3297425a 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -1501,7 +1501,6 @@ config ARM64_PTR_AUTH
 	# 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
@@ -1513,13 +1512,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.
@@ -1530,6 +1522,22 @@ 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 "Enable support for pointer authentication for kernel"
+	default y
+	depends on ARM64_PTR_AUTH
+	depends on (!FUNCTION_GRAPH_TRACER || DYNAMIC_FTRACE_WITH_REGS)
+	help
+	  Build the kernel with return address protection by
+	  pointer authentication.
+
+	  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.
 
@@ -1618,7 +1626,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 6a87d592bd00..6e5d9de8c2b3 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/drivers/misc/lkdtm/bugs.c b/drivers/misc/lkdtm/bugs.c
index a0675d4154d2..439fa33ae413 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.29.2


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

[PATCH v2 2/2] arm64: Do not configure kernel's PTR_AUTH key when it not needed.

[Daniel Kiss]

If the kernel is not compiled with CONFIG_ARM64_PTR_AUTH_KERNEL,
then no need to install dedicated key for the kernel, user's key
could be left enabled because no PACI/AUTI instructions are expected..

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

diff --git a/arch/arm64/include/asm/asm_pointer_auth.h b/arch/arm64/include/asm/asm_pointer_auth.h
index 52dead2a8640..b2572a943f59 100644
--- a/arch/arm64/include/asm/asm_pointer_auth.h
+++ b/arch/arm64/include/asm/asm_pointer_auth.h
@@ -39,27 +39,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 +48,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 +63,43 @@ 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
 
+#endif /* CONFIG_ARM64_PTR_AUTH */
+
+#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
 
-#endif /* CONFIG_ARM64_PTR_AUTH */
+#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 */
 
 #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 9c5efcc6e7f1..c78d63be5bd1 100644
--- a/arch/arm64/include/asm/processor.h
+++ b/arch/arm64/include/asm/processor.h
@@ -153,8 +153,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_incl;
diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
index 7d32fc959b1a..cb7965a9f505 100644
--- a/arch/arm64/kernel/asm-offsets.c
+++ b/arch/arm64/kernel/asm-offsets.c
@@ -46,7 +46,9 @@ 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));
+#ifdef CONFIG_ARM64_PTR_AUTH_KERNEL
   DEFINE(THREAD_KEYS_KERNEL,	offsetof(struct task_struct, thread.keys_kernel));
+#endif
 #endif
   BLANK();
   DEFINE(S_X0,			offsetof(struct pt_regs, regs[0]));
@@ -141,7 +143,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.29.2


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

Re: arm64: split ARM64_PTR_AUTH option to userspace and kernel

[Will Deacon]

On Fri, Dec 18, 2020 at 12:56:30PM +0100, Daniel Kiss wrote:
> As discussed the A-key left enabled, this makes the patch simpler too.
> arch/arm64/crypto/poly1305-core.S_shipped contains PACISP/AUTISP
> instructions but this code is called while the preeption is disabled,
> therefore it won't cause any trouble.

Please use the --cover-letter option to git format-patch for generating your
cover letter. It's also best to send new versions out as a new series,
rather than replying to the previous one.

Thanks,

Will

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

Re: [PATCH v2 1/2] arm64: Add ARM64_PTR_AUTH_KERNEL config option

[Will Deacon]

On Fri, Dec 18, 2020 at 12:56:31PM +0100, Daniel Kiss wrote:
> This new option makes possible to build the kernel with pointer
> authentication support for the user space while the kernel is not built
> with the pointer authentication. There is a similar config structure for BTI.
> 
> The default configuration will be the same after this patch.

Please read the "Describe your changes" section of
Documentation/process/submitting-patches.rst for some guidance on writing
commit messages.

> Signed-off-by: Daniel Kiss <daniel.kiss@arm.com>
> ---
>  arch/arm64/Kconfig        | 26 +++++++++++++++++---------
>  arch/arm64/Makefile       |  2 +-
>  drivers/misc/lkdtm/bugs.c |  6 +++---
>  3 files changed, 21 insertions(+), 13 deletions(-)
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 75aefc9990ea..b8af3297425a 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -1501,7 +1501,6 @@ config ARM64_PTR_AUTH
>  	# 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

Why do we need to keep all the toolchain checks here if this option doesn't
enable PAC in the kernel?

> -	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
> @@ -1513,13 +1512,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.
> @@ -1530,6 +1522,22 @@ 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 "Enable support for pointer authentication for kernel"

Maybe "Use pointer authentication for kernel" for parity with the BTI
description.

> +	default y
> +	depends on ARM64_PTR_AUTH
> +	depends on (!FUNCTION_GRAPH_TRACER || DYNAMIC_FTRACE_WITH_REGS)
> +	help
> +	  Build the kernel with return address protection by
> +	  pointer authentication.

I don't think these two lines add anything ^^

Will

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

Re: [PATCH v2 2/2] arm64: Do not configure kernel's PTR_AUTH key when it not needed.

[Will Deacon]

On Fri, Dec 18, 2020 at 12:56:32PM +0100, Daniel Kiss wrote:
> If the kernel is not compiled with CONFIG_ARM64_PTR_AUTH_KERNEL,
> then no need to install dedicated key for the kernel, user's key
> could be left enabled because no PACI/AUTI instructions are expected..

(same comment as before re commit message)

> Signed-off-by: Daniel Kiss <daniel.kiss@arm.com>
> ---
>  arch/arm64/include/asm/asm_pointer_auth.h | 53 +++++++++++++----------
>  arch/arm64/include/asm/pointer_auth.h     | 20 ++++++---
>  arch/arm64/include/asm/processor.h        |  2 +
>  arch/arm64/kernel/asm-offsets.c           |  4 ++
>  4 files changed, 51 insertions(+), 28 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/asm_pointer_auth.h b/arch/arm64/include/asm/asm_pointer_auth.h
> index 52dead2a8640..b2572a943f59 100644
> --- a/arch/arm64/include/asm/asm_pointer_auth.h
> +++ b/arch/arm64/include/asm/asm_pointer_auth.h
> @@ -39,27 +39,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 +48,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 +63,43 @@ 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
>  
> +#endif /* CONFIG_ARM64_PTR_AUTH */
> +
> +#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
> +

nit: can you please define these macros at the top of the file, so that
they are defined before the macros which use them?

Will

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

Re: [PATCH v2 1/2] arm64: Add ARM64_PTR_AUTH_KERNEL config option

[Daniel Kiss]

> On 26 Jan 2021, at 14:27, Will Deacon <will@kernel.org> wrote:
> 
> On Fri, Dec 18, 2020 at 12:56:31PM +0100, Daniel Kiss wrote:
>> This new option makes possible to build the kernel with pointer
>> authentication support for the user space while the kernel is not built
>> with the pointer authentication. There is a similar config structure for BTI.
>> 
>> The default configuration will be the same after this patch.
> 
> Please read the "Describe your changes" section of
> Documentation/process/submitting-patches.rst for some guidance on writing
> commit messages.
WIll do, thanks.
I’ll send a new patch series according to it with the fixes.

>> Signed-off-by: Daniel Kiss <daniel.kiss@arm.com>
>> ---
>> arch/arm64/Kconfig        | 26 +++++++++++++++++---------
>> arch/arm64/Makefile       |  2 +-
>> drivers/misc/lkdtm/bugs.c |  6 +++---
>> 3 files changed, 21 insertions(+), 13 deletions(-)
>> 
>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>> index 75aefc9990ea..b8af3297425a 100644
>> --- a/arch/arm64/Kconfig
>> +++ b/arch/arm64/Kconfig
>> @@ -1501,7 +1501,6 @@ config ARM64_PTR_AUTH
>> 	# 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
> 
> Why do we need to keep all the toolchain checks here if this option doesn't
> enable PAC in the kernel?
No need for that, can be moved to under ARM64_PTR_AUTH_KERNEL.

> 
>> -	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
>> @@ -1513,13 +1512,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.
>> @@ -1530,6 +1522,22 @@ 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 "Enable support for pointer authentication for kernel"
> 
> Maybe "Use pointer authentication for kernel" for parity with the BTI
> description.
Done
> 
>> +	default y
>> +	depends on ARM64_PTR_AUTH
>> +	depends on (!FUNCTION_GRAPH_TRACER || DYNAMIC_FTRACE_WITH_REGS)
>> +	help
>> +	  Build the kernel with return address protection by
>> +	  pointer authentication.
> 
> I don't think these two lines add anything ^^
Done.
> 
> Will

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