diff mbox series

[v2,13/14] arm64: compile the kernel with ptrauth return address signing

Message ID 1574166746-27197-14-git-send-email-amit.kachhap@arm.com (mailing list archive)
State New, archived
Headers show
Series arm64: return address signing | expand

Commit Message

Amit Daniel Kachhap Nov. 19, 2019, 12:32 p.m. UTC 
From: Kristina Martsenko <kristina.martsenko@arm.com>

Compile all functions with two ptrauth instructions: PACIASP in the
prologue to sign the return address, and AUTIASP in the epilogue to
authenticate the return address (from the stack). If authentication
fails, the return will cause an instruction abort to be taken, followed
by an oops and killing the task.

This should help protect the kernel against attacks using
return-oriented programming. As ptrauth protects the return address, it
can also serve as a replacement for CONFIG_STACKPROTECTOR, although note
that it does not protect other parts of the stack.

The new instructions are in the HINT encoding space, so on a system
without ptrauth they execute as NOPs.

CONFIG_ARM64_PTR_AUTH now not only enables ptrauth for userspace and KVM
guests, but also automatically builds the kernel with ptrauth
instructions if the compiler supports it. If there is no compiler
support, we do not warn that the kernel was built without ptrauth
instructions.

GCC 7 and 8 support the -msign-return-address option, while GCC 9
deprecates that option and replaces it with -mbranch-protection. Support
both options.

Reviewed-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Kristina Martsenko <kristina.martsenko@arm.com>
[Amit: Cover leaf function, comments]
Signed-off-by: Amit Daniel Kachhap <amit.kachhap@arm.com>
---
Change since last version:
 * Comments for different behaviour while booting secondary cores.

 arch/arm64/Kconfig  | 16 +++++++++++++++-
 arch/arm64/Makefile |  6 ++++++
 2 files changed, 21 insertions(+), 1 deletion(-)

Comments

Mark Brown Nov. 21, 2019, 3:06 p.m. UTC | #1 
On Tue, Nov 19, 2019 at 06:02:25PM +0530, Amit Daniel Kachhap wrote:

> +config CC_HAS_BRANCH_PROT_PAC_RET
> +	# GCC 9 or later
> +	def_bool $(cc-option,-mbranch-protection=pac-ret+leaf)

clang also supports this option, as of clang-8 I think.

> +ifeq ($(CONFIG_ARM64_PTR_AUTH),y)
> +pac-flags-$(CONFIG_CC_HAS_SIGN_RETURN_ADDRESS) := -msign-return-address=all
> +pac-flags-$(CONFIG_CC_HAS_BRANCH_PROT_PAC_RET) := -mbranch-protection=pac-ret+leaf
> +KBUILD_CFLAGS += $(pac-flags-y)
> +endif

This is going to be a bit annoying with BTI as we need to set
-mbranch-protection=bti too.  This means we end up with type
bti+pac-ret+leaf which is annoying to arrange.  There is the convenient
branch protection type standard which does enable both in one option but
that only enables non-leaf pac-ret so you need to explicitly spell out
pac-ret so you can turn on leaf as well.  I'm not sure I can think of
anything much better than adding another case for BTI at the top so we
end up with something along the lines of:

ifeq ($(CONFIG_ARM64_BTI_KERNEL),y)
branch-prot-flags-$(CONFIG_CC_HAS_BRANCH_PROT_BTI) := -mbranch-protection=bti+pac-ret+leaf
else ifeq ($(CONFIG_ARM64_PTR_AUTH),y)
branch-prot-flags-$(CONFIG_CC_HAS_BRANCH_PROT_PAC_RET) := -mbranch-protection=pac-ret+leaf
endif
KBUILD_CFLAGS += $(branch-prot-flags-y)

with a separate section for the signed return address bit.  It would be
helpful to avoid the immediate refactoring when adding BTI by splitting
things up with a more generic name.
Mark Brown Nov. 25, 2019, 5:35 p.m. UTC | #2 
On Tue, Nov 19, 2019 at 06:02:25PM +0530, Amit Daniel Kachhap wrote:

> +config CC_HAS_BRANCH_PROT_PAC_RET
> +	# GCC 9 or later
> +	def_bool $(cc-option,-mbranch-protection=pac-ret+leaf)

This breaks the build for me with CC=clang-9:

  CC      arch/arm64/kernel/vdso/vgettimeofday.o
/tmp/vgettimeofday-1a520b.s: Assembler messages:
/tmp/vgettimeofday-1a520b.s:25: Error: selected processor does not support `paciasp'
/tmp/vgettimeofday-1a520b.s:26: Error: unknown pseudo-op: `.cfi_negate_ra_state'
/tmp/vgettimeofday-1a520b.s:120: Error: selected processor does not support `autiasp'

(and various other errors with the assembler not understanding stuff).
This happens because clang is using the system assembler (that from
Debian stable in my case, 2.31.1) and it requires additional options to
enable newer instructions.  We need to pass -mcpu=all or similar to the
assembler (eg, with -Wa,-mcpu=all in CC).  This'd be fine if the
cc-option check detected the assembler issues but sadly it doesn't get
that far.
Amit Daniel Kachhap Nov. 26, 2019, 7 a.m. UTC | #3 
Hi Mark,

On 11/21/19 8:36 PM, Mark Brown wrote:
> On Tue, Nov 19, 2019 at 06:02:25PM +0530, Amit Daniel Kachhap wrote:
> 
>> +config CC_HAS_BRANCH_PROT_PAC_RET
>> +	# GCC 9 or later
>> +	def_bool $(cc-option,-mbranch-protection=pac-ret+leaf)
> 
> clang also supports this option, as of clang-8 I think.

ok. I will test and update the comment here.
> 
>> +ifeq ($(CONFIG_ARM64_PTR_AUTH),y)
>> +pac-flags-$(CONFIG_CC_HAS_SIGN_RETURN_ADDRESS) := -msign-return-address=all
>> +pac-flags-$(CONFIG_CC_HAS_BRANCH_PROT_PAC_RET) := -mbranch-protection=pac-ret+leaf
>> +KBUILD_CFLAGS += $(pac-flags-y)
>> +endif
> 
> This is going to be a bit annoying with BTI as we need to set
> -mbranch-protection=bti too.  This means we end up with type
> bti+pac-ret+leaf which is annoying to arrange.  There is the convenient
> branch protection type standard which does enable both in one option but
> that only enables non-leaf pac-ret so you need to explicitly spell out
> pac-ret so you can turn on leaf as well.  I'm not sure I can think of
> anything much better than adding another case for BTI at the top so we
> end up with something along the lines of:

Yes. The reason for keeping pac-ret+leaf is to cover all functions in 
the earlier offline discussions. As you pointed out I can add 
CC_HAS_BRANCH_PROT_PAC_RET_LEAF config name to make it more meaningful
> 
> ifeq ($(CONFIG_ARM64_BTI_KERNEL),y)
> branch-prot-flags-$(CONFIG_CC_HAS_BRANCH_PROT_BTI) := -mbranch-protection=bti+pac-ret+leaf
> else ifeq ($(CONFIG_ARM64_PTR_AUTH),y)
> branch-prot-flags-$(CONFIG_CC_HAS_BRANCH_PROT_PAC_RET) := -mbranch-protection=pac-ret+leaf
> endif
> KBUILD_CFLAGS += $(branch-prot-flags-y)
> 
> with a separate section for the signed return address bit.  It would be
> helpful to avoid the immediate refactoring when adding BTI by splitting
> things up with a more generic name.

I agree with your concern for separate section when BTI support is 
added. I will do it in my next iteration.

//Amit

>
diff mbox series

Patch

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index c1844de..44d13fe 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -1427,11 +1427,17 @@  config ARM64_PTR_AUTH
 	  and other attacks.
 
 	  This option enables these instructions at EL0 (i.e. for userspace).
-
 	  Choosing this option will cause the kernel to initialise secret keys
 	  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. However, KVM guest also require VHE mode and hence
@@ -1442,6 +1448,14 @@  config ARM64_PTR_AUTH
 	  have address auth and the late CPU has then system panic will occur.
 	  On such a system, this option should not be selected.
 
+config CC_HAS_BRANCH_PROT_PAC_RET
+	# GCC 9 or later
+	def_bool $(cc-option,-mbranch-protection=pac-ret+leaf)
+
+config CC_HAS_SIGN_RETURN_ADDRESS
+	# GCC 7, 8
+	def_bool $(cc-option,-msign-return-address=all)
+
 endmenu
 
 config ARM64_SVE
diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
index 2c0238c..031462b 100644
--- a/arch/arm64/Makefile
+++ b/arch/arm64/Makefile
@@ -72,6 +72,12 @@  stack_protector_prepare: prepare0
 					include/generated/asm-offsets.h))
 endif
 
+ifeq ($(CONFIG_ARM64_PTR_AUTH),y)
+pac-flags-$(CONFIG_CC_HAS_SIGN_RETURN_ADDRESS) := -msign-return-address=all
+pac-flags-$(CONFIG_CC_HAS_BRANCH_PROT_PAC_RET) := -mbranch-protection=pac-ret+leaf
+KBUILD_CFLAGS += $(pac-flags-y)
+endif
+
 ifeq ($(CONFIG_CPU_BIG_ENDIAN), y)
 KBUILD_CPPFLAGS	+= -mbig-endian
 CHECKFLAGS	+= -D__AARCH64EB__