Skip to content

Commit 2117d53

Browse files
committed
KVM: x86: drop error recovery in em_jmp_far and em_ret_far
em_jmp_far and em_ret_far assumed that setting IP can only fail in 64 bit mode, but syzkaller proved otherwise (and SDM agrees). Code segment was restored upon failure, but it was left uninitialized outside of long mode, which could lead to a leak of host kernel stack. We could have fixed that by always saving and restoring the CS, but we take a simpler approach and just break any guest that manages to fail as the error recovery is error-prone and modern CPUs don't need emulator for this. Found by syzkaller: WARNING: CPU: 2 PID: 3668 at arch/x86/kvm/emulate.c:2217 em_ret_far+0x428/0x480 Kernel panic - not syncing: panic_on_warn set ... CPU: 2 PID: 3668 Comm: syz-executor Not tainted 4.9.0-rc4+ #49 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011 [...] Call Trace: [...] __dump_stack lib/dump_stack.c:15 [...] dump_stack+0xb3/0x118 lib/dump_stack.c:51 [...] panic+0x1b7/0x3a3 kernel/panic.c:179 [...] __warn+0x1c4/0x1e0 kernel/panic.c:542 [...] warn_slowpath_null+0x2c/0x40 kernel/panic.c:585 [...] em_ret_far+0x428/0x480 arch/x86/kvm/emulate.c:2217 [...] em_ret_far_imm+0x17/0x70 arch/x86/kvm/emulate.c:2227 [...] x86_emulate_insn+0x87a/0x3730 arch/x86/kvm/emulate.c:5294 [...] x86_emulate_instruction+0x520/0x1ba0 arch/x86/kvm/x86.c:5545 [...] emulate_instruction arch/x86/include/asm/kvm_host.h:1116 [...] complete_emulated_io arch/x86/kvm/x86.c:6870 [...] complete_emulated_mmio+0x4e9/0x710 arch/x86/kvm/x86.c:6934 [...] kvm_arch_vcpu_ioctl_run+0x3b7a/0x5a90 arch/x86/kvm/x86.c:6978 [...] kvm_vcpu_ioctl+0x61e/0xdd0 arch/x86/kvm/../../../virt/kvm/kvm_main.c:2557 [...] vfs_ioctl fs/ioctl.c:43 [...] do_vfs_ioctl+0x18c/0x1040 fs/ioctl.c:679 [...] SYSC_ioctl fs/ioctl.c:694 [...] SyS_ioctl+0x8f/0xc0 fs/ioctl.c:685 [...] entry_SYSCALL_64_fastpath+0x1f/0xc2 Reported-by: Dmitry Vyukov <[email protected]> Cc: [email protected] Fixes: d1442d8 ("KVM: x86: Handle errors when RIP is set during far jumps") Signed-off-by: Radim Krčmář <[email protected]>
1 parent 444fdad commit 2117d53

File tree

1 file changed

+11
-25
lines changed

1 file changed

+11
-25
lines changed

arch/x86/kvm/emulate.c

Lines changed: 11 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -2105,16 +2105,10 @@ static int em_iret(struct x86_emulate_ctxt *ctxt)
21052105
static int em_jmp_far(struct x86_emulate_ctxt *ctxt)
21062106
{
21072107
int rc;
2108-
unsigned short sel, old_sel;
2109-
struct desc_struct old_desc, new_desc;
2110-
const struct x86_emulate_ops *ops = ctxt->ops;
2108+
unsigned short sel;
2109+
struct desc_struct new_desc;
21112110
u8 cpl = ctxt->ops->cpl(ctxt);
21122111

2113-
/* Assignment of RIP may only fail in 64-bit mode */
2114-
if (ctxt->mode == X86EMUL_MODE_PROT64)
2115-
ops->get_segment(ctxt, &old_sel, &old_desc, NULL,
2116-
VCPU_SREG_CS);
2117-
21182112
memcpy(&sel, ctxt->src.valptr + ctxt->op_bytes, 2);
21192113

21202114
rc = __load_segment_descriptor(ctxt, sel, VCPU_SREG_CS, cpl,
@@ -2124,12 +2118,10 @@ static int em_jmp_far(struct x86_emulate_ctxt *ctxt)
21242118
return rc;
21252119

21262120
rc = assign_eip_far(ctxt, ctxt->src.val, &new_desc);
2127-
if (rc != X86EMUL_CONTINUE) {
2128-
WARN_ON(ctxt->mode != X86EMUL_MODE_PROT64);
2129-
/* assigning eip failed; restore the old cs */
2130-
ops->set_segment(ctxt, old_sel, &old_desc, 0, VCPU_SREG_CS);
2131-
return rc;
2132-
}
2121+
/* Error handling is not implemented. */
2122+
if (rc != X86EMUL_CONTINUE)
2123+
return X86EMUL_UNHANDLEABLE;
2124+
21332125
return rc;
21342126
}
21352127

@@ -2189,14 +2181,8 @@ static int em_ret_far(struct x86_emulate_ctxt *ctxt)
21892181
{
21902182
int rc;
21912183
unsigned long eip, cs;
2192-
u16 old_cs;
21932184
int cpl = ctxt->ops->cpl(ctxt);
2194-
struct desc_struct old_desc, new_desc;
2195-
const struct x86_emulate_ops *ops = ctxt->ops;
2196-
2197-
if (ctxt->mode == X86EMUL_MODE_PROT64)
2198-
ops->get_segment(ctxt, &old_cs, &old_desc, NULL,
2199-
VCPU_SREG_CS);
2185+
struct desc_struct new_desc;
22002186

22012187
rc = emulate_pop(ctxt, &eip, ctxt->op_bytes);
22022188
if (rc != X86EMUL_CONTINUE)
@@ -2213,10 +2199,10 @@ static int em_ret_far(struct x86_emulate_ctxt *ctxt)
22132199
if (rc != X86EMUL_CONTINUE)
22142200
return rc;
22152201
rc = assign_eip_far(ctxt, eip, &new_desc);
2216-
if (rc != X86EMUL_CONTINUE) {
2217-
WARN_ON(ctxt->mode != X86EMUL_MODE_PROT64);
2218-
ops->set_segment(ctxt, old_cs, &old_desc, 0, VCPU_SREG_CS);
2219-
}
2202+
/* Error handling is not implemented. */
2203+
if (rc != X86EMUL_CONTINUE)
2204+
return X86EMUL_UNHANDLEABLE;
2205+
22202206
return rc;
22212207
}
22222208

0 commit comments

Comments
 (0)