On Thu, Mar 17, 2022 at 11:52:33AM +0100, Borislav Petkov wrote: > From: Borislav Petkov <bp@xxxxxxx> > > The commit in Fixes started adding INT3 after RETs as a mitigation > against straight-line speculation. > > The fastop SETcc implementation in kvm's insn emulator uses macro magic > to generate all possible SETcc functions and to jump to them when > emulating the respective instruction. > > However, it hardcodes the size and alignment of those functions to 4: a > three-byte SETcc insn and a single-byte RET. BUT, with SLS, there's an > INT3 that gets slapped after the RET, which brings the whole scheme out > of alignment: > > 15: 0f 90 c0 seto %al > 18: c3 ret > 19: cc int3 > 1a: 0f 1f 00 nopl (%rax) > 1d: 0f 91 c0 setno %al > 20: c3 ret > 21: cc int3 > 22: 0f 1f 00 nopl (%rax) > 25: 0f 92 c0 setb %al > 28: c3 ret > 29: cc int3 > > and this explodes like this: > > int3: 0000 [#1] PREEMPT SMP PTI > CPU: 0 PID: 2435 Comm: qemu-system-x86 Not tainted 5.17.0-rc8-sls #1 > Hardware name: Dell Inc. Precision WorkStation T3400 /0TP412, BIOS A14 04/30/2012 > RIP: 0010:setc+0x5/0x8 [kvm] > Code: 00 00 0f 1f 00 0f b6 05 43 24 06 00 c3 cc 0f 1f 80 00 00 00 00 0f 90 c0 c3 cc 0f 1f 00 0f 91 c0 c3 cc 0f 1f 00 0f 92 c0 c3 cc <0f> 1f 00 0f 93 c0 c3 cc 0f 1f 00 0f 94 c0 c3 cc 0f 1f 00 0f 95 c0 > Call Trace: > <TASK> > ? x86_emulate_insn [kvm] > ? x86_emulate_instruction [kvm] > ? vmx_handle_exit [kvm_intel] > ? kvm_arch_vcpu_ioctl_run [kvm] > ? kvm_vcpu_ioctl [kvm] > ? __x64_sys_ioctl > ? do_syscall_64+0x40/0xa0 > ? entry_SYSCALL_64_after_hwframe+0x44/0xae > </TASK> > > Raise the alignment value when SLS is enabled and use a macro for that > instead of hard-coding naked numbers. > > Fixes: e463a09af2f0 ("x86: Add straight-line-speculation mitigation") > Reported-by: Jamie Heilman <jamie@xxxxxxxxxxxxxxxxxxxxx> > Signed-off-by: Borislav Petkov <bp@xxxxxxx> > Link: https://lore.kernel.org/r/YjGzJwjrvxg5YZ0Z@xxxxxxxxxxxxxxxxxxxxx Acked-by: Peter Zijlstra (Intel) <peterz@xxxxxxxxxxxxx> Depending on what Paolo wants, it might make sense to merge this into tip/x86/urgent such that we can then resolve the merge conflict vs tip/x86/core with something like the below: diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c index 113fd5c1b874..06dfbe9adcdb 100644 --- a/arch/x86/kvm/emulate.c +++ b/arch/x86/kvm/emulate.c @@ -24,6 +24,7 @@ #include <linux/stringify.h> #include <asm/debugreg.h> #include <asm/nospec-branch.h> +#include <asm/ibt.h> #include "x86.h" #include "tss.h" @@ -431,7 +432,19 @@ static int fastop(struct x86_emulate_ctxt *ctxt, fastop_t fop); /* Special case for SETcc - 1 instruction per cc */ -#define SETCC_ALIGN (4 * (1 + IS_ENABLED(CONFIG_SLS))) +/* + * Depending on .config the SETcc functions look like: + * + * setcc: + * +0 ENDBR [CONFIG_X86_KERNEL_IBT] + * +4 SETcc %al + * +7 RET + * +8 INT3 [CONFIG_SLS] + * + * Which gives possible sizes: 4, 5, 8, 9 which when rounded up to the + * next power-of-two alignment become: 4, 8, 16. + */ +#define SETCC_ALIGN (4 * (1 + IS_ENABLED(CONFIG_SLS)) * (1 + HAS_KERNEL_IBT)) #define FOP_SETCC(op) \ ".align " __stringify(SETCC_ALIGN) " \n\t" \