From 60c50f2b0bf5d3f894ca428cf4b4374fbea2d082 Mon Sep 17 00:00:00 2001 From: Andrew Cooper Date: Thu, 8 Feb 2018 12:46:40 +0100 Subject: [PATCH] x86: Support indirect thunks from assembly code Introduce INDIRECT_CALL and INDIRECT_JMP which either degrade to a normal indirect branch, or dispatch to the __x86_indirect_thunk_* symbols. Update all the manual indirect branches in to use the new thunks. The indirect branches in the early boot and kexec path are left intact as we can't use the compiled-in thunks at those points. This is part of XSA-254. Signed-off-by: Andrew Cooper Reviewed-by: Jan Beulich master commit: 7c508612f7a5096b4819d4ef2ce566e01bd66c0c master date: 2018-01-16 17:45:50 +0000 --- xen/Rules.mk | 4 +-- xen/arch/x86/Rules.mk | 6 ++++ xen/arch/x86/boot/trampoline.S | 24 ++++++++++++-- xen/arch/x86/traps.c | 41 +++++++++++++++++------- xen/arch/x86/x86_64/entry.S | 6 ++-- xen/arch/x86/x86_emulate/x86_emulate.c | 16 ++++----- xen/common/wait.c | 8 +++-- xen/include/asm-x86/asm_defns.h | 8 +++++ xen/include/asm-x86/indirect_thunk_asm.h | 41 ++++++++++++++++++++++++ xen/include/xen/config.h | 2 +- 10 files changed, 126 insertions(+), 30 deletions(-) create mode 100644 xen/include/asm-x86/indirect_thunk_asm.h diff --git a/xen/Rules.mk b/xen/Rules.mk index 08cc776ebc..2653f745a8 100644 --- a/xen/Rules.mk +++ b/xen/Rules.mk @@ -72,8 +72,8 @@ endif AFLAGS-y += -D__ASSEMBLY__ -# Clang's built-in assembler can't handle .code16/.code32/.code64 yet -AFLAGS-$(clang) += -no-integrated-as +# Clang's built-in assembler can't handle embedded .include's +CFLAGS-$(clang) += -no-integrated-as ALL_OBJS := $(ALL_OBJS-y) diff --git a/xen/arch/x86/Rules.mk b/xen/arch/x86/Rules.mk index ad56e56ac6..2784e19f27 100644 --- a/xen/arch/x86/Rules.mk +++ b/xen/arch/x86/Rules.mk @@ -32,3 +32,9 @@ CFLAGS += -mindirect-branch=thunk-extern -mindirect-branch-register CFLAGS += -DCONFIG_INDIRECT_THUNK export CONFIG_INDIRECT_THUNK=y endif + +# Set up the assembler include path properly for older GCC toolchains. Clang +# objects to the agument being passed however. +ifneq ($(clang),y) +CFLAGS += -Wa,-I$(BASEDIR)/include +endif diff --git a/xen/arch/x86/boot/trampoline.S b/xen/arch/x86/boot/trampoline.S index b0136148bb..8a17e128d7 100644 --- a/xen/arch/x86/boot/trampoline.S +++ b/xen/arch/x86/boot/trampoline.S @@ -128,8 +128,28 @@ trampoline_protmode_entry: .code64 start64: /* Jump to high mappings. */ - movabs $__high_start,%rax - jmpq *%rax + movabs $__high_start, %rdi + +#ifdef CONFIG_INDIRECT_THUNK + /* + * If booting virtualised, or hot-onlining a CPU, sibling threads can + * attempt Branch Target Injection against this jmp. + * + * We've got no usable stack so can't use a RETPOLINE thunk, and are + * further than disp32 from the high mappings so couldn't use + * JUMP_THUNK even if it was a non-RETPOLINE thunk. Furthermore, an + * LFENCE isn't necessarily safe to use at this point. + * + * As this isn't a hotpath, use a fully serialising event to reduce + * the speculation window as much as possible. %ebx needs preserving + * for __high_start. + */ + mov %ebx, %esi + cpuid + mov %esi, %ebx +#endif + + jmpq *%rdi .code32 trampoline_boot_cpu_entry: diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c index f4bf8b58fa..5d6ccde9f9 100644 --- a/xen/arch/x86/traps.c +++ b/xen/arch/x86/traps.c @@ -2789,6 +2789,8 @@ int pv_emul_cpuid(unsigned int *eax, unsigned int *ebx, unsigned int *ecx, return X86EMUL_OKAY; } +void __x86_indirect_thunk_rcx(void); + /* Instruction fetch with error handling. */ #define insn_fetch(type, base, eip, limit) \ ({ unsigned long _rc, _ptr = (base) + (eip); \ @@ -2827,6 +2829,8 @@ static int emulate_privileged_op(struct cpu_user_regs *regs) unsigned long code_base, code_limit; char *io_emul_stub = NULL; void (*io_emul)(struct cpu_user_regs *); + struct stubs *this_stubs = &this_cpu(stubs); + unsigned long stub_va = this_stubs->addr + STUB_BUF_SIZE / 2; uint64_t val; if ( !read_descriptor(regs->cs, v, &code_base, &code_limit, &ar, 1) ) @@ -3010,31 +3014,44 @@ static int emulate_privileged_op(struct cpu_user_regs *regs) * context. This is needed for some systems which (ab)use IN/OUT * to communicate with BIOS code in system-management mode. */ - io_emul_stub = map_domain_page(_mfn(this_cpu(stubs.mfn))) + - (this_cpu(stubs.addr) & ~PAGE_MASK) + - STUB_BUF_SIZE / 2; + io_emul_stub = map_domain_page(_mfn(this_stubs->mfn)) + + (stub_va & ~PAGE_MASK); /* movq $host_to_guest_gpr_switch,%rcx */ io_emul_stub[0] = 0x48; io_emul_stub[1] = 0xb9; *(void **)&io_emul_stub[2] = (void *)host_to_guest_gpr_switch; + +#ifdef CONFIG_INDIRECT_THUNK + /* callq __x86_indirect_thunk_rcx */ + io_emul_stub[10] = 0xe8; + *(int32_t *)&io_emul_stub[11] = + (long)__x86_indirect_thunk_rcx - (stub_va + 11 + 4); +#else /* callq *%rcx */ io_emul_stub[10] = 0xff; io_emul_stub[11] = 0xd1; + /* TODO: untangle ideal_nops from init/livepatch Kconfig options. */ + memcpy(&io_emul_stub[12], "\x0f\x1f\x00", 3); /* P6_NOP3 */ +#endif + /* data16 or nop */ - io_emul_stub[12] = (op_bytes != 2) ? 0x90 : 0x66; + io_emul_stub[15] = (op_bytes != 2) ? 0x90 : 0x66; /* */ - io_emul_stub[13] = opcode; + io_emul_stub[16] = opcode; /* imm8 or nop */ - io_emul_stub[14] = 0x90; + io_emul_stub[17] = 0x90; /* ret (jumps to guest_to_host_gpr_switch) */ - io_emul_stub[15] = 0xc3; - BUILD_BUG_ON(STUB_BUF_SIZE / 2 < 16); + io_emul_stub[18] = 0xc3; + BUILD_BUG_ON(STUB_BUF_SIZE / 2 < 19); /* Handy function-typed pointer to the stub. */ - io_emul = (void *)(this_cpu(stubs.addr) + STUB_BUF_SIZE / 2); + io_emul = (void *)stub_va; if ( ioemul_handle_quirk ) - ioemul_handle_quirk(opcode, &io_emul_stub[12], regs); + { + BUILD_BUG_ON(STUB_BUF_SIZE / 2 < 15 + 10); + ioemul_handle_quirk(opcode, &io_emul_stub[15], regs); + } /* I/O Port and Interrupt Flag instructions. */ switch ( opcode ) @@ -3043,7 +3060,7 @@ static int emulate_privileged_op(struct cpu_user_regs *regs) op_bytes = 1; case 0xe5: /* IN imm8,%eax */ port = insn_fetch(u8, code_base, eip, code_limit); - io_emul_stub[14] = port; /* imm8 */ + io_emul_stub[17] = port; /* imm8 */ exec_in: if ( !guest_io_okay(port, op_bytes, v, regs) ) goto fail; @@ -3072,7 +3089,7 @@ static int emulate_privileged_op(struct cpu_user_regs *regs) op_bytes = 1; case 0xe7: /* OUT %eax,imm8 */ port = insn_fetch(u8, code_base, eip, code_limit); - io_emul_stub[14] = port; /* imm8 */ + io_emul_stub[17] = port; /* imm8 */ exec_out: if ( !guest_io_okay(port, op_bytes, v, regs) ) goto fail; diff --git a/xen/arch/x86/x86_64/entry.S b/xen/arch/x86/x86_64/entry.S index 505604fd6e..20536d44aa 100644 --- a/xen/arch/x86/x86_64/entry.S +++ b/xen/arch/x86/x86_64/entry.S @@ -578,7 +578,8 @@ handle_exception_saved: movzbl UREGS_entry_vector(%rsp),%eax leaq exception_table(%rip),%rdx PERFC_INCR(exceptions, %rax, %rbx) - callq *(%rdx,%rax,8) + mov (%rdx, %rax, 8), %rdx + INDIRECT_CALL %rdx mov %r15, STACK_CPUINFO_FIELD(xen_cr3)(%r14) testb $3,UREGS_cs(%rsp) jz restore_all_xen @@ -750,7 +751,8 @@ handle_ist_exception: 1: movq %rsp,%rdi movzbl UREGS_entry_vector(%rsp),%eax leaq exception_table(%rip),%rdx - callq *(%rdx,%rax,8) + mov (%rdx, %rax, 8), %rdx + INDIRECT_CALL %rdx mov %r15, STACK_CPUINFO_FIELD(xen_cr3)(%r14) cmpb $TRAP_nmi,UREGS_entry_vector(%rsp) jne ret_from_intr diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c b/xen/arch/x86/x86_emulate/x86_emulate.c index 9851416713..bfabec06ba 100644 --- a/xen/arch/x86/x86_emulate/x86_emulate.c +++ b/xen/arch/x86/x86_emulate/x86_emulate.c @@ -880,8 +880,8 @@ do { \ struct fpu_insn_ctxt fic_ = { .insn_bytes = nr_ }; \ memcpy(get_stub(stub), ((uint8_t[]){ bytes, 0xc3 }), nr_ + 1); \ get_fpu(X86EMUL_FPU_fpu, &fic_); \ - asm volatile ( "call *%[stub]" : "+m" (fic_) : \ - [stub] "rm" (stub.func) ); \ + asm volatile ( "INDIRECT_CALL %[stub]" : "+m" (fic_) : \ + [stub] "r" (stub.func) ); \ put_fpu(&fic_); \ put_stub(stub); \ } while (0) @@ -894,11 +894,11 @@ do { \ memcpy(get_stub(stub), ((uint8_t[]){ bytes, 0xc3 }), nr_ + 1); \ get_fpu(X86EMUL_FPU_fpu, &fic_); \ asm volatile ( _PRE_EFLAGS("[eflags]", "[mask]", "[tmp]") \ - "call *%[func];" \ + "INDIRECT_CALL %[func];" \ _POST_EFLAGS("[eflags]", "[mask]", "[tmp]") \ : [eflags] "+g" (_regs.eflags), \ [tmp] "=&r" (tmp_), "+m" (fic_) \ - : [func] "rm" (stub.func), \ + : [func] "r" (stub.func), \ [mask] "i" (EFLG_ZF|EFLG_PF|EFLG_CF) ); \ put_fpu(&fic_); \ put_stub(stub); \ @@ -4766,8 +4766,8 @@ x86_emulate( if ( !rc ) { copy_REX_VEX(buf, rex_prefix, vex); - asm volatile ( "call *%0" : : "r" (stub.func), "a" (mmvalp) - : "memory" ); + asm volatile ( "INDIRECT_CALL %0" : : "r" (stub.func), "a" (mmvalp) + : "memory" ); } put_fpu(&fic); put_stub(stub); @@ -5058,8 +5058,8 @@ x86_emulate( if ( !rc ) { copy_REX_VEX(buf, rex_prefix, vex); - asm volatile ( "call *%0" : : "r" (stub.func), "a" (ea.reg) - : "memory" ); + asm volatile ( "INDIRECT_CALL %0" : : "r" (stub.func), "a" (ea.reg) + : "memory" ); } put_fpu(&fic); put_stub(stub); diff --git a/xen/common/wait.c b/xen/common/wait.c index 877ef19638..6bb65c60eb 100644 --- a/xen/common/wait.c +++ b/xen/common/wait.c @@ -204,12 +204,14 @@ void check_wakeup_from_wait(void) /* * Hand-rolled longjmp(). Returns to the pointer on the top of - * wqv->stack, and lands on a `rep movs` instruction. + * wqv->stack, and lands on a `rep movs` instruction. All other GPRs are + * restored from the stack, so are available for use here. */ asm volatile ( - "mov %1,%%"__OP"sp; jmp *(%0)" + "mov %1,%%"__OP"sp; INDIRECT_JMP %[ip]" : : "S" (wqv->stack), "D" (wqv->esp), - "c" ((char *)get_cpu_info() - (char *)wqv->esp) + "c" ((char *)get_cpu_info() - (char *)wqv->esp), + [ip] "r" (*(unsigned long *)wqv->stack) : "memory" ); unreachable(); } diff --git a/xen/include/asm-x86/asm_defns.h b/xen/include/asm-x86/asm_defns.h index 8eb5c0c6de..358deffb5b 100644 --- a/xen/include/asm-x86/asm_defns.h +++ b/xen/include/asm-x86/asm_defns.h @@ -13,6 +13,14 @@ #include #include +#ifdef __ASSEMBLY__ +# include +#else +asm ( "\t.equ CONFIG_INDIRECT_THUNK, " + __stringify(IS_ENABLED(CONFIG_INDIRECT_THUNK)) ); +asm ( "\t.include \"asm/indirect_thunk_asm.h\"" ); +#endif + #ifndef __ASSEMBLY__ void ret_from_intr(void); #endif diff --git a/xen/include/asm-x86/indirect_thunk_asm.h b/xen/include/asm-x86/indirect_thunk_asm.h new file mode 100644 index 0000000000..96bcc25497 --- /dev/null +++ b/xen/include/asm-x86/indirect_thunk_asm.h @@ -0,0 +1,41 @@ +/* + * Warning! This file is included at an assembler level for .c files, causing + * usual #ifdef'ary to turn into comments. + */ + +.macro INDIRECT_BRANCH insn:req arg:req +/* + * Create an indirect branch. insn is one of call/jmp, arg is a single + * register. + * + * With no compiler support, this degrades into a plain indirect call/jmp. + * With compiler support, dispatch to the correct __x86_indirect_thunk_* + */ + .if CONFIG_INDIRECT_THUNK == 1 + + $done = 0 + .irp reg, ax, cx, dx, bx, bp, si, di, 8, 9, 10, 11, 12, 13, 14, 15 + .ifeqs "\arg", "%r\reg" + \insn __x86_indirect_thunk_r\reg + $done = 1 + .exitm + .endif + .endr + + .if $done != 1 + .error "Bad register arg \arg" + .endif + + .else + \insn *\arg + .endif +.endm + +/* Convenience wrappers. */ +.macro INDIRECT_CALL arg:req + INDIRECT_BRANCH call \arg +.endm + +.macro INDIRECT_JMP arg:req + INDIRECT_BRANCH jmp \arg +.endm diff --git a/xen/include/xen/config.h b/xen/include/xen/config.h index 473c5e8c3e..9f39687fab 100644 --- a/xen/include/xen/config.h +++ b/xen/include/xen/config.h @@ -7,7 +7,7 @@ #ifndef __XEN_CONFIG_H__ #define __XEN_CONFIG_H__ -#include +#include #ifndef __ASSEMBLY__ #include