Avoid speculative indirect calls in kernel

From: Dave Hansen <[email protected]>

From: David Woodhouse <[email protected]>

retpoline is a special sequence on Intel CPUs to stop speculation for indirect branches.

Provide assembler infrastructure to use retpoline by the compiler and for assembler. We add the out of line trampoline used by the compiler, and NOSPEC_JUMP / NOSPEC_CALL macros for assembler

[Originally from David and Tim, heavily hacked by AK]

v2: Add CONFIG_RETPOLINE option

Signed-off-by David Woodhouse <[email protected]>

Signed-off-by Tim Chen <[email protected]>

Signed-off-by Andi Kleen <[email protected]>

---
 arch/x86/Kconfig                |  8 +++++
 arch/x86/include/asm/jump-asm.h | 70 +++++++++++++++++++++++++++++++++++++++++
 arch/x86/kernel/vmlinux.lds.S   |  1 +
 arch/x86/lib/Makefile           |  1 +
 arch/x86/lib/retpoline.S        | 35 +++++++++++++++++++++
 5 files changed, 115 insertions(+)
 create mode 100644 arch/x86/include/asm/jump-asm.h
 create mode 100644 arch/x86/lib/retpoline.S

arch/x86/Kconfig +8 -0

index d4fc98c50378..8b0facfa35be 100644
@@ -429,6 +429,14 @@ config GOLDFISH
        def_bool y
        depends on X86_GOLDFISH
 
+config RETPOLINE
+       bool "Avoid speculative indirect branches in kernel"
+       default y
+       help
+         Compile kernel with the retpoline compiler options to guard against
+         kernel to user data leaks by avoiding speculative indirect
+         branches. Requires a new enough compiler. The kernel may run slower.
+
 config INTEL_RDT
        bool "Intel Resource Director Technology support"
        default n

arch/x86/include/asm/jump-asm.h new file +70 -0

new file mode 100644
index 000000000000..936fa620f346
@@ -0,0 +1,70 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef JUMP_ASM_H
+#define JUMP_ASM_H 1
+
+#ifdef __ASSEMBLY__
+
+#ifdef CONFIG_RETPOLINE
+
+/*
+ * Jump to an indirect pointer without speculation.
+ *
+ * The out of line __x86.indirect_thunk has special code sequences
+ * to stop speculation.
+ */
+
+.macro NOSPEC_JMP target
+       push    \target
+       jmp     __x86.indirect_thunk
+.endm
+
+
+/*
+ * Call an indirect pointer without speculation.
+ */
+
+.macro NOSPEC_CALL target
+       jmp     1221f
+1222:
+       push    \target
+       jmp     __x86.indirect_thunk
+1221:
+       call    1222b
+.endm
+
+#else /* CONFIG_RETPOLINE */
+
+.macro NOSPEC_JMP target
+       jmp *\target
+.endm
+
+.macro NOSPEC_CALL target
+       call *\target
+.endm
+
+#endif /* !CONFIG_RETPOLINE */
+
+#else /* __ASSEMBLY__ */
+
+#ifdef CONFIG_RETPOLINE
+
+#define NOSPEC_JMP(t) \
+       "push " t "; "                          \
+       "jmp __x86.indirect_thunk; "
+
+#define NOSPEC_CALL(t) \
+       "       jmp 1221f; "                    \
+       "1222:  push " t ";"                    \
+       "       jmp __x86.indirect_thunk;"      \
+       "1221:  call 1222b;"
+
+#else /* CONFIG_RETPOLINE */
+
+#define NOSPEC_JMP(t) "jmp *" t "; "
+#define NOSPEC_CALL(t) "call *" t "; "
+
+#endif /* !CONFIG_RETPOLINE */
+
+#endif /* !__ASSEMBLY */
+
+#endif

arch/x86/kernel/vmlinux.lds.S +1 -0

index 1e413a9326aa..2e64241a6664 100644
@@ -103,6 +103,7 @@ SECTIONS
                /* bootstrapping code */
                HEAD_TEXT
                . = ALIGN(8);
+               *(.text.__x86.indirect_thunk)
                TEXT_TEXT
                SCHED_TEXT
                CPUIDLE_TEXT

arch/x86/lib/Makefile +1 -0

index 7b181b61170e..f23934bbaf4e 100644
@@ -26,6 +26,7 @@ lib-y += memcpy_$(BITS).o
 lib-$(CONFIG_RWSEM_XCHGADD_ALGORITHM) += rwsem.o
 lib-$(CONFIG_INSTRUCTION_DECODER) += insn.o inat.o insn-eval.o
 lib-$(CONFIG_RANDOMIZE_BASE) += kaslr.o
+lib-$(CONFIG_RETPOLINE) += retpoline.o
 
 obj-y += msr.o msr-reg.o msr-reg-export.o hweight.o
 

arch/x86/lib/retpoline.S new file +35 -1

new file mode 100644
index 000000000000..cb40781adbfe
@@ -0,0 +1,35 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+/*
+ * Out of line jump trampoline for calls that disable speculation.
+ *
+ * This is a special sequence that prevents the CPU speculating
+ * for indirect calls.
+ *
+ * This can be called by gcc generated code, or with the asm macros
+ * in asm/jump-asm.h
+ */
+
+#include <linux/linkage.h>
+#include <asm/dwarf2.h>
+#include <asm/export.h>
+
+       .section        .text.__x86.indirect_thunk,"ax"
+
+ENTRY(__x86.indirect_thunk)
+       CFI_STARTPROC
+       call    retpoline_call_target
+2:
+       lfence          /* stop speculation */
+       jmp     2b
+retpoline_call_target:
+#ifdef CONFIG_64BIT
+       lea     8(%rsp), %rsp
+#else
+       lea     4(%esp), %esp
+#endif
+       ret
+       CFI_ENDPROC
+ENDPROC(__x86.indirect_thunk)
+
+       EXPORT_SYMBOL(__x86.indirect_thunk)
--

2.14.3

Quoted 14 lines

arch/x86/lib/retpoline.S new file +14 -0

+ENTRY(__x86.indirect_thunk)
+       CFI_STARTPROC
+       call    retpoline_call_target
+2:
+       lfence          /* stop speculation */
+       jmp     2b
+retpoline_call_target:
+#ifdef CONFIG_64BIT
+       lea     8(%rsp), %rsp
+#else
+       lea     4(%esp), %esp
+#endif
+       ret
+       CFI_ENDPROC

Quoted 6 lines

arch/x86/lib/retpoline.S new file +3 -1

+ENDPROC(__x86.indirect_thunk)
+
+       EXPORT_SYMBOL(__x86.indirect_thunk)
--

2.14.3

Can someone actually explain WTF this mess is trying to accomplish?

arch/x86/entry/entry_64.S +3 -2

@@ -269,8 +270,9 @@ entry_SYSCALL_64_fastpath:
          * This call instruction is handled specially in stub_ptregs_64.
          * It might end up jumping to the slow path.  If it jumps, RAX
          * and all argument registers are clobbered.
-        */
-       call    *sys_call_table(, %rax, 8)
+       */
+       movq    sys_call_table(, %rax, 8), %rax
+       NOSPEC_CALL     %rax

 .Lentry_SYSCALL_64_after_fastpath_call:

        movq    %rax, RAX(%rsp)

From: Andi Kleen <[email protected]>

Convert all indirect jumps in xen inline assembler code to use non speculative sequences.

Based on code from David Woodhouse and Tim Chen

Signed-off-by Andi Kleen <[email protected]>

---
 arch/x86/crypto/camellia-aesni-avx2-asm_64.S | 1 +
 arch/x86/include/asm/xen/hypercall.h         | 3 ++-
 2 files changed, 3 insertions(+), 1 deletion(-)

arch/x86/crypto/camellia-aesni-avx2-asm_64.S +0 -0

index 1743e6850e00..9cd8450a2050 100644

arch/x86/crypto/camellia-aesni-avx2-asm_64.S +0 -0

 #define CAMELLIA_TABLE_BYTE_LEN 272
 

arch/x86/include/asm/xen/hypercall.h +1 -0

index 7cb282e9e587..91de35bcce5e 100644
@@ -44,6 +44,7 @@
 #include <asm/page.h>
 #include <asm/pgtable.h>
 #include <asm/smap.h>
+#include <asm/jump-asm.h>
 
 #include <xen/interface/xen.h>
 #include <xen/interface/sched.h>

Quoted 6 lines

arch/x86/crypto/camellia-aesni-avx2-asm_64.S +0 -0

index 1743e6850e00..9cd8450a2050 100644
@@ -12,6 +12,7 @@
 

arch/x86/crypto/camellia-aesni-avx2-asm_64.S line 15

Quoted 3 lines

arch/x86/crypto/camellia-aesni-avx2-asm_64.S +1 -0

 #include <linux/linkage.h>
 #include <asm/frame.h>
+#include <asm/jump-asm.h>

I fail to connect this change to the patch title.

Maybe should be part of the crypto patch?

From: Andi Kleen <[email protected]>

With the indirect call thunk enabled compiler two objtool warnings are triggered very frequently and make the build very noisy.

I don't see a good way to avoid them, so just disable them for now.

Signed-off-by Andi Kleen <[email protected]>

---
 tools/objtool/check.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

tools/objtool/check.c +11 -0

index 9b341584eb1b..435c71f944dc 100644
@@ -503,8 +503,13 @@ static int add_call_destinations(struct objtool_file *file)
 			insn->call_dest = find_symbol_by_offset(insn->sec,
 								dest_off);
 			if (!insn->call_dest) {
+#if 0
+				/* Compilers with -mindirect-branch=thunk-extern trigger
+				 * this everywhere on x86. Disable for now.
+				 */
 				WARN_FUNC("can't find call dest symbol at offset 0x%lx",
 					  insn->sec, insn->offset, dest_off);
+#endif
 				return -1;
 			}
 		} else if (rela->sym->type == STT_SECTION) {

@@ -1716,8 +1721,14 @@ static int validate_branch(struct objtool_file *file, struct instruction *first,
 					return 1;
 
 			} else if (func && has_modified_stack_frame(&state)) {
+#if 0
+				/* Compilers with -mindirect-branch=thunk-extern trigger
+				 * this everywhere on x86. Disable for now.
+				 */
+
 				WARN_FUNC("sibling call from callable instruction with modified stack frame",
 					  sec, insn->offset);
+#endif
 				return 1;
 			}
 

Signature · 1 line

2.14.3

NAK.  We can't blindly disable objtool warnings, that will break livepatch and the ORC unwinder.  If you share a .o file (or the GCC code) I can look at adding retpoline support.

NAK. We can't blindly disable objtool warnings, that will break livepatch and the ORC unwinder. If you share a .o file (or the GCC code) I can look at adding retpoline support.

Quoted 3 lines

NAK. We can't blindly disable objtool warnings, that will break livepatch and the ORC unwinder. If you share a .o file (or the GCC code) I can look at adding retpoline support.

I don't think we can wait for that. We can disable livepatch and the unwinder for now. They are not essential. Frame pointers should work well enough for unwinding

and afaik nobody can use livepatch in mainline anyways.

Quoted 7 lines · Andi Kleen

Quoted 3 lines

NAK. We can't blindly disable objtool warnings, that will break livepatch and the ORC unwinder. If you share a .o file (or the GCC code) I can look at adding retpoline support.

I don't think we can wait for that. We can disable livepatch and the unwinder for now. They are not essential. Frame pointers should work well enough for unwinding

If you want to make this feature conflict with livepatch and ORC, silencing objtool warnings is not the way to do it.

Quoted 2 lines

and afaik nobody can use livepatch in mainline anyways.

Why not? The patch creation tooling is still out-of-tree, but livepatch itself is fully supported in mainline.

Quoted 11 lines · Josh Poimboeuf

Quoted 7 lines · Andi Kleen

Quoted 3 lines

NAK. We can't blindly disable objtool warnings, that will break livepatch and the ORC unwinder. If you share a .o file (or the GCC code) I can look at adding retpoline support.

I don't think we can wait for that. We can disable livepatch and the unwinder for now. They are not essential. Frame pointers should work well enough for unwinding

If you want to make this feature conflict with livepatch and ORC, silencing objtool warnings is not the way to do it.

I don't see why it would conflict with the unwinder anyways?

It doesn't change the long term stack state, so it should be invisible to the unwinder (unless you crash in the thunk, which is very unlikely)

I actually got some unwinder backtraces during development and they seemed to work.

Quoted 4 lines

Quoted 1 line

and afaik nobody can use livepatch in mainline anyways.

Why not? The patch creation tooling is still out-of-tree, but livepatch itself is fully supported in mainline.

Ok.

Still doesn't seem critical at this point if it's some out of tree thing.

Either way we'll need to figure out a way to get objtool support ASAP.

This is a fix for Variant 2 in
https://googleprojectzero.blogspot.com/2018/01/reading-privileged-memory-with-side.html

Any speculative indirect calls in the kernel can be tricked to execute any kernel code, which may allow side channel attacks that can leak arbitrary kernel data.

So we want to avoid speculative indirect calls in the kernel.

There's a special code sequence called a retpoline that can do indirect calls without speculation. We use a new compiler option -mindirect-branch=thunk-extern (gcc patch will be released separately) to recompile the kernel with this new sequence.

Hi!

Quoted 6 lines

This is a fix for Variant 2 in
https://googleprojectzero.blogspot.com/2018/01/reading-privileged-memory-with-side.html

Any speculative indirect calls in the kernel can be tricked to execute any kernel code, which may allow side channel attacks that can leak arbitrary kernel data.

Ok.

Quoted 6 lines

So we want to avoid speculative indirect calls in the kernel.

There's a special code sequence called a retpoline that can do indirect calls without speculation. We use a new compiler option -mindirect-branch=thunk-extern (gcc patch will be released separately) to recompile the kernel with this new sequence.

So... this "retpoline" code is quite tricky; I guess it does the right on recent Intel CPUs. Does it also do the right thing on all the AMD, Cyrix, ... variants?

Is it neccessary on all the CPUs? I guess 486 does not need this?

Quoted 18 lines · On Thu, 4 Jan 2018 12:49:17 +0100 Pavel Machek <[email protected]> wrote:

Hi!

Quoted 6 lines

This is a fix for Variant 2 in
https://googleprojectzero.blogspot.com/2018/01/reading-privileged-memory-with-side.html

Any speculative indirect calls in the kernel can be tricked to execute any kernel code, which may allow side channel attacks that can leak arbitrary kernel data.

Ok.

Quoted 6 lines

So we want to avoid speculative indirect calls in the kernel.

There's a special code sequence called a retpoline that can do indirect calls without speculation. We use a new compiler option -mindirect-branch=thunk-extern (gcc patch will be released separately) to recompile the kernel with this new sequence.

Quoted 5 lines

So... this "retpoline" code is quite tricky; I guess it does the right on recent Intel CPUs. Does it also do the right thing on all the AMD, Cyrix, ... variants?

Is it neccessary on all the CPUs? I guess 486 does not need this?

It's architecturally valid x86 code so it should work on any x86-32 processor.

You are correct older processors and some of the newer ones don't need it. AMD and VIA I don't know about but for Intel we can probably avoid it on older Atom, on Quark, and the really old CPUs nobody actually runs any more.

That's all an optimization once we have correctness.