diff mbox series

arm64: bti: Require clang >= 10.0.1 for in-kernel BTI support

Message ID 20200616183630.2445-1-will@kernel.org (mailing list archive)
State Mainlined
Commit b9249cba25a5dce5de87e5404503a5e11832c2dd
Headers show
Series arm64: bti: Require clang >= 10.0.1 for in-kernel BTI support | expand

Commit Message

Will Deacon June 16, 2020, 6:36 p.m. UTC
Unfortunately, most versions of clang that support BTI are capable of
miscompiling the kernel when converting a switch statement into a jump
table. As an example, attempting to spawn a KVM guest results in a panic:

[   56.253312] Kernel panic - not syncing: bad mode
[   56.253834] CPU: 0 PID: 279 Comm: lkvm Not tainted 5.8.0-rc1 #2
[   56.254225] Hardware name: QEMU QEMU Virtual Machine, BIOS 0.0.0 02/06/2015
[   56.254712] Call trace:
[   56.254952]  dump_backtrace+0x0/0x1d4
[   56.255305]  show_stack+0x1c/0x28
[   56.255647]  dump_stack+0xc4/0x128
[   56.255905]  panic+0x16c/0x35c
[   56.256146]  bad_el0_sync+0x0/0x58
[   56.256403]  el1_sync_handler+0xb4/0xe0
[   56.256674]  el1_sync+0x7c/0x100
[   56.256928]  kvm_vm_ioctl_check_extension_generic+0x74/0x98
[   56.257286]  __arm64_sys_ioctl+0x94/0xcc
[   56.257569]  el0_svc_common+0x9c/0x150
[   56.257836]  do_el0_svc+0x84/0x90
[   56.258083]  el0_sync_handler+0xf8/0x298
[   56.258361]  el0_sync+0x158/0x180

This is because the switch in kvm_vm_ioctl_check_extension_generic()
is executed as an indirect branch to tail-call through a jump table:

ffff800010032dc8:       3869694c        ldrb    w12, [x10, x9]
ffff800010032dcc:       8b0c096b        add     x11, x11, x12, lsl #2
ffff800010032dd0:       d61f0160        br      x11

However, where the target case uses the stack, the landing pad is elided
due to the presence of a paciasp instruction:

ffff800010032e14:       d503233f        paciasp
ffff800010032e18:       a9bf7bfd        stp     x29, x30, [sp, #-16]!
ffff800010032e1c:       910003fd        mov     x29, sp
ffff800010032e20:       aa0803e0        mov     x0, x8
ffff800010032e24:       940017c0        bl      ffff800010038d24 <kvm_vm_ioctl_check_extension>
ffff800010032e28:       93407c00        sxtw    x0, w0
ffff800010032e2c:       a8c17bfd        ldp     x29, x30, [sp], #16
ffff800010032e30:       d50323bf        autiasp
ffff800010032e34:       d65f03c0        ret

Unfortunately, this results in a fatal exception because paciasp is
compatible only with branch-and-link (call) instructions and not simple
indirect branches.

A fix is being merged into Clang 10.0.1 so that a 'bti j' instruction is
emitted as an explicit landing pad in this situation. Make in-kernel
BTI depend on that compiler version when building with clang.

Cc: Nick Desaulniers <ndesaulniers@google.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Nathan Chancellor <natechancellor@gmail.com>
Cc: Tom Stellard <tstellar@redhat.com>
Cc: Daniel Kiss <daniel.kiss@arm.com>
Link: https://lore.kernel.org/r/20200615105524.GA2694@willie-the-truck
Signed-off-by: Will Deacon <will@kernel.org>
---
 arch/arm64/Kconfig | 2 ++
 1 file changed, 2 insertions(+)

Comments

Nathan Chancellor June 16, 2020, 6:40 p.m. UTC | #1
On Tue, Jun 16, 2020 at 07:36:30PM +0100, Will Deacon wrote:
> Unfortunately, most versions of clang that support BTI are capable of
> miscompiling the kernel when converting a switch statement into a jump
> table. As an example, attempting to spawn a KVM guest results in a panic:
> 
> [   56.253312] Kernel panic - not syncing: bad mode
> [   56.253834] CPU: 0 PID: 279 Comm: lkvm Not tainted 5.8.0-rc1 #2
> [   56.254225] Hardware name: QEMU QEMU Virtual Machine, BIOS 0.0.0 02/06/2015
> [   56.254712] Call trace:
> [   56.254952]  dump_backtrace+0x0/0x1d4
> [   56.255305]  show_stack+0x1c/0x28
> [   56.255647]  dump_stack+0xc4/0x128
> [   56.255905]  panic+0x16c/0x35c
> [   56.256146]  bad_el0_sync+0x0/0x58
> [   56.256403]  el1_sync_handler+0xb4/0xe0
> [   56.256674]  el1_sync+0x7c/0x100
> [   56.256928]  kvm_vm_ioctl_check_extension_generic+0x74/0x98
> [   56.257286]  __arm64_sys_ioctl+0x94/0xcc
> [   56.257569]  el0_svc_common+0x9c/0x150
> [   56.257836]  do_el0_svc+0x84/0x90
> [   56.258083]  el0_sync_handler+0xf8/0x298
> [   56.258361]  el0_sync+0x158/0x180
> 
> This is because the switch in kvm_vm_ioctl_check_extension_generic()
> is executed as an indirect branch to tail-call through a jump table:
> 
> ffff800010032dc8:       3869694c        ldrb    w12, [x10, x9]
> ffff800010032dcc:       8b0c096b        add     x11, x11, x12, lsl #2
> ffff800010032dd0:       d61f0160        br      x11
> 
> However, where the target case uses the stack, the landing pad is elided
> due to the presence of a paciasp instruction:
> 
> ffff800010032e14:       d503233f        paciasp
> ffff800010032e18:       a9bf7bfd        stp     x29, x30, [sp, #-16]!
> ffff800010032e1c:       910003fd        mov     x29, sp
> ffff800010032e20:       aa0803e0        mov     x0, x8
> ffff800010032e24:       940017c0        bl      ffff800010038d24 <kvm_vm_ioctl_check_extension>
> ffff800010032e28:       93407c00        sxtw    x0, w0
> ffff800010032e2c:       a8c17bfd        ldp     x29, x30, [sp], #16
> ffff800010032e30:       d50323bf        autiasp
> ffff800010032e34:       d65f03c0        ret
> 
> Unfortunately, this results in a fatal exception because paciasp is
> compatible only with branch-and-link (call) instructions and not simple
> indirect branches.
> 
> A fix is being merged into Clang 10.0.1 so that a 'bti j' instruction is
> emitted as an explicit landing pad in this situation. Make in-kernel
> BTI depend on that compiler version when building with clang.
> 
> Cc: Nick Desaulniers <ndesaulniers@google.com>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Nathan Chancellor <natechancellor@gmail.com>
> Cc: Tom Stellard <tstellar@redhat.com>
> Cc: Daniel Kiss <daniel.kiss@arm.com>
> Link: https://lore.kernel.org/r/20200615105524.GA2694@willie-the-truck
> Signed-off-by: Will Deacon <will@kernel.org>

Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>

> ---
>  arch/arm64/Kconfig | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 31380da53689..4ae2419c14a8 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -1630,6 +1630,8 @@ config ARM64_BTI_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
> +	# https://reviews.llvm.org/rGb8ae3fdfa579dbf366b1bb1cbfdbf8c51db7fa55
> +	depends on !CC_IS_CLANG || CLANG_VERSION >= 100001
>  	depends on !(CC_IS_CLANG && GCOV_KERNEL)
>  	depends on (!FUNCTION_GRAPH_TRACER || DYNAMIC_FTRACE_WITH_REGS)
>  	help
> -- 
> 2.27.0.290.gba653c62da-goog
>
Dave Martin June 17, 2020, 9:06 a.m. UTC | #2
On Tue, Jun 16, 2020 at 07:36:30PM +0100, Will Deacon wrote:
> Unfortunately, most versions of clang that support BTI are capable of
> miscompiling the kernel when converting a switch statement into a jump
> table. As an example, attempting to spawn a KVM guest results in a panic:
> 
> [   56.253312] Kernel panic - not syncing: bad mode
> [   56.253834] CPU: 0 PID: 279 Comm: lkvm Not tainted 5.8.0-rc1 #2
> [   56.254225] Hardware name: QEMU QEMU Virtual Machine, BIOS 0.0.0 02/06/2015
> [   56.254712] Call trace:
> [   56.254952]  dump_backtrace+0x0/0x1d4
> [   56.255305]  show_stack+0x1c/0x28
> [   56.255647]  dump_stack+0xc4/0x128
> [   56.255905]  panic+0x16c/0x35c
> [   56.256146]  bad_el0_sync+0x0/0x58
> [   56.256403]  el1_sync_handler+0xb4/0xe0
> [   56.256674]  el1_sync+0x7c/0x100
> [   56.256928]  kvm_vm_ioctl_check_extension_generic+0x74/0x98
> [   56.257286]  __arm64_sys_ioctl+0x94/0xcc
> [   56.257569]  el0_svc_common+0x9c/0x150
> [   56.257836]  do_el0_svc+0x84/0x90
> [   56.258083]  el0_sync_handler+0xf8/0x298
> [   56.258361]  el0_sync+0x158/0x180
> 
> This is because the switch in kvm_vm_ioctl_check_extension_generic()
> is executed as an indirect branch to tail-call through a jump table:
> 
> ffff800010032dc8:       3869694c        ldrb    w12, [x10, x9]
> ffff800010032dcc:       8b0c096b        add     x11, x11, x12, lsl #2
> ffff800010032dd0:       d61f0160        br      x11
> 
> However, where the target case uses the stack, the landing pad is elided
> due to the presence of a paciasp instruction:
> 
> ffff800010032e14:       d503233f        paciasp
> ffff800010032e18:       a9bf7bfd        stp     x29, x30, [sp, #-16]!
> ffff800010032e1c:       910003fd        mov     x29, sp
> ffff800010032e20:       aa0803e0        mov     x0, x8
> ffff800010032e24:       940017c0        bl      ffff800010038d24 <kvm_vm_ioctl_check_extension>
> ffff800010032e28:       93407c00        sxtw    x0, w0
> ffff800010032e2c:       a8c17bfd        ldp     x29, x30, [sp], #16
> ffff800010032e30:       d50323bf        autiasp
> ffff800010032e34:       d65f03c0        ret
> 
> Unfortunately, this results in a fatal exception because paciasp is
> compatible only with branch-and-link (call) instructions and not simple
> indirect branches.
> 
> A fix is being merged into Clang 10.0.1 so that a 'bti j' instruction is
> emitted as an explicit landing pad in this situation. Make in-kernel
> BTI depend on that compiler version when building with clang.
> 
> Cc: Nick Desaulniers <ndesaulniers@google.com>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Nathan Chancellor <natechancellor@gmail.com>
> Cc: Tom Stellard <tstellar@redhat.com>
> Cc: Daniel Kiss <daniel.kiss@arm.com>
> Link: https://lore.kernel.org/r/20200615105524.GA2694@willie-the-truck
> Signed-off-by: Will Deacon <will@kernel.org>
> ---
>  arch/arm64/Kconfig | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 31380da53689..4ae2419c14a8 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -1630,6 +1630,8 @@ config ARM64_BTI_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
> +	# https://reviews.llvm.org/rGb8ae3fdfa579dbf366b1bb1cbfdbf8c51db7fa55
> +	depends on !CC_IS_CLANG || CLANG_VERSION >= 100001
>  	depends on !(CC_IS_CLANG && GCOV_KERNEL)
>  	depends on (!FUNCTION_GRAPH_TRACER || DYNAMIC_FTRACE_WITH_REGS)
>  	help

FWIW:

Acked-by: Dave Martin <Dave.Martin@arm.com>

I've not tried to reproduce this myself, but the problem description and
proposed solution seem perfectly plausible, given how BTI works.

Cheers
---Dave
Mark Brown June 17, 2020, 9:09 a.m. UTC | #3
On Tue, Jun 16, 2020 at 07:36:30PM +0100, Will Deacon wrote:
> Unfortunately, most versions of clang that support BTI are capable of
> miscompiling the kernel when converting a switch statement into a jump
> table. As an example, attempting to spawn a KVM guest results in a panic:

Reviewed-by: Mark Brown <broonie@kernel.org>
Daniel Kiss June 18, 2020, 11:25 a.m. UTC | #4
Acked-by: Daniel Kiss <daniel.kiss@arm.com>

> On 17 Jun 2020, at 11:09, Mark Brown <broonie@kernel.org> wrote:
> 
> On Tue, Jun 16, 2020 at 07:36:30PM +0100, Will Deacon wrote:
>> Unfortunately, most versions of clang that support BTI are capable of
>> miscompiling the kernel when converting a switch statement into a jump
>> table. As an example, attempting to spawn a KVM guest results in a panic:
> 
> Reviewed-by: Mark Brown <broonie@kernel.org>
diff mbox series

Patch

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 31380da53689..4ae2419c14a8 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -1630,6 +1630,8 @@  config ARM64_BTI_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
+	# https://reviews.llvm.org/rGb8ae3fdfa579dbf366b1bb1cbfdbf8c51db7fa55
+	depends on !CC_IS_CLANG || CLANG_VERSION >= 100001
 	depends on !(CC_IS_CLANG && GCOV_KERNEL)
 	depends on (!FUNCTION_GRAPH_TRACER || DYNAMIC_FTRACE_WITH_REGS)
 	help