Skip to content

Commit e7177c7

Browse files
sean-jcgregkh
authored andcommitted
KVM: x86: Don't (re)check L1 intercepts when completing userspace I/O
commit e750f85 upstream. When completing emulation of instruction that generated a userspace exit for I/O, don't recheck L1 intercepts as KVM has already finished that phase of instruction execution, i.e. has already committed to allowing L2 to perform I/O. If L1 (or host userspace) modifies the I/O permission bitmaps during the exit to userspace, KVM will treat the access as being intercepted despite already having emulated the I/O access. Pivot on EMULTYPE_NO_DECODE to detect that KVM is completing emulation. Of the three users of EMULTYPE_NO_DECODE, only complete_emulated_io() (the intended "recipient") can reach the code in question. gp_interception()'s use is mutually exclusive with is_guest_mode(), and complete_emulated_insn_gp() unconditionally pairs EMULTYPE_NO_DECODE with EMULTYPE_SKIP. The bad behavior was detected by a syzkaller program that toggles port I/O interception during the userspace I/O exit, ultimately resulting in a WARN on vcpu->arch.pio.count being non-zero due to KVM no completing emulation of the I/O instruction. WARNING: CPU: 23 PID: 1083 at arch/x86/kvm/x86.c:8039 emulator_pio_in_out+0x154/0x170 [kvm] Modules linked in: kvm_intel kvm irqbypass CPU: 23 UID: 1000 PID: 1083 Comm: repro Not tainted 6.16.0-rc5-c1610d2d66b1-next-vm #74 NONE Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 0.0.0 02/06/2015 RIP: 0010:emulator_pio_in_out+0x154/0x170 [kvm] PKRU: 55555554 Call Trace: <TASK> kvm_fast_pio+0xd6/0x1d0 [kvm] vmx_handle_exit+0x149/0x610 [kvm_intel] kvm_arch_vcpu_ioctl_run+0xda8/0x1ac0 [kvm] kvm_vcpu_ioctl+0x244/0x8c0 [kvm] __x64_sys_ioctl+0x8a/0xd0 do_syscall_64+0x5d/0xc60 entry_SYSCALL_64_after_hwframe+0x4b/0x53 </TASK> Reported-by: [email protected] Closes: https://lore.kernel.org/all/[email protected] Fixes: 8a76d7f ("KVM: x86: Add x86 callback for intercept check") Cc: [email protected] Cc: Jim Mattson <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Sean Christopherson <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
1 parent 716dceb commit e7177c7

File tree

3 files changed

+13
-14
lines changed

3 files changed

+13
-14
lines changed

arch/x86/kvm/emulate.c

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5104,12 +5104,11 @@ void init_decode_cache(struct x86_emulate_ctxt *ctxt)
51045104
ctxt->mem_read.end = 0;
51055105
}
51065106

5107-
int x86_emulate_insn(struct x86_emulate_ctxt *ctxt)
5107+
int x86_emulate_insn(struct x86_emulate_ctxt *ctxt, bool check_intercepts)
51085108
{
51095109
const struct x86_emulate_ops *ops = ctxt->ops;
51105110
int rc = X86EMUL_CONTINUE;
51115111
int saved_dst_type = ctxt->dst.type;
5112-
bool is_guest_mode = ctxt->ops->is_guest_mode(ctxt);
51135112

51145113
ctxt->mem_read.pos = 0;
51155114

@@ -5157,7 +5156,7 @@ int x86_emulate_insn(struct x86_emulate_ctxt *ctxt)
51575156
fetch_possible_mmx_operand(&ctxt->dst);
51585157
}
51595158

5160-
if (unlikely(is_guest_mode) && ctxt->intercept) {
5159+
if (unlikely(check_intercepts) && ctxt->intercept) {
51615160
rc = emulator_check_intercept(ctxt, ctxt->intercept,
51625161
X86_ICPT_PRE_EXCEPT);
51635162
if (rc != X86EMUL_CONTINUE)
@@ -5186,7 +5185,7 @@ int x86_emulate_insn(struct x86_emulate_ctxt *ctxt)
51865185
goto done;
51875186
}
51885187

5189-
if (unlikely(is_guest_mode) && (ctxt->d & Intercept)) {
5188+
if (unlikely(check_intercepts) && (ctxt->d & Intercept)) {
51905189
rc = emulator_check_intercept(ctxt, ctxt->intercept,
51915190
X86_ICPT_POST_EXCEPT);
51925191
if (rc != X86EMUL_CONTINUE)
@@ -5240,7 +5239,7 @@ int x86_emulate_insn(struct x86_emulate_ctxt *ctxt)
52405239

52415240
special_insn:
52425241

5243-
if (unlikely(is_guest_mode) && (ctxt->d & Intercept)) {
5242+
if (unlikely(check_intercepts) && (ctxt->d & Intercept)) {
52445243
rc = emulator_check_intercept(ctxt, ctxt->intercept,
52455244
X86_ICPT_POST_MEMACCESS);
52465245
if (rc != X86EMUL_CONTINUE)

arch/x86/kvm/kvm_emulate.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -230,7 +230,6 @@ struct x86_emulate_ops {
230230
void (*set_nmi_mask)(struct x86_emulate_ctxt *ctxt, bool masked);
231231

232232
bool (*is_smm)(struct x86_emulate_ctxt *ctxt);
233-
bool (*is_guest_mode)(struct x86_emulate_ctxt *ctxt);
234233
int (*leave_smm)(struct x86_emulate_ctxt *ctxt);
235234
void (*triple_fault)(struct x86_emulate_ctxt *ctxt);
236235
int (*set_xcr)(struct x86_emulate_ctxt *ctxt, u32 index, u64 xcr);
@@ -514,7 +513,7 @@ bool x86_page_table_writing_insn(struct x86_emulate_ctxt *ctxt);
514513
#define EMULATION_RESTART 1
515514
#define EMULATION_INTERCEPTED 2
516515
void init_decode_cache(struct x86_emulate_ctxt *ctxt);
517-
int x86_emulate_insn(struct x86_emulate_ctxt *ctxt);
516+
int x86_emulate_insn(struct x86_emulate_ctxt *ctxt, bool check_intercepts);
518517
int emulator_task_switch(struct x86_emulate_ctxt *ctxt,
519518
u16 tss_selector, int idt_index, int reason,
520519
bool has_error_code, u32 error_code);

arch/x86/kvm/x86.c

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8567,11 +8567,6 @@ static bool emulator_is_smm(struct x86_emulate_ctxt *ctxt)
85678567
return is_smm(emul_to_vcpu(ctxt));
85688568
}
85698569

8570-
static bool emulator_is_guest_mode(struct x86_emulate_ctxt *ctxt)
8571-
{
8572-
return is_guest_mode(emul_to_vcpu(ctxt));
8573-
}
8574-
85758570
#ifndef CONFIG_KVM_SMM
85768571
static int emulator_leave_smm(struct x86_emulate_ctxt *ctxt)
85778572
{
@@ -8655,7 +8650,6 @@ static const struct x86_emulate_ops emulate_ops = {
86558650
.guest_cpuid_is_intel_compatible = emulator_guest_cpuid_is_intel_compatible,
86568651
.set_nmi_mask = emulator_set_nmi_mask,
86578652
.is_smm = emulator_is_smm,
8658-
.is_guest_mode = emulator_is_guest_mode,
86598653
.leave_smm = emulator_leave_smm,
86608654
.triple_fault = emulator_triple_fault,
86618655
.set_xcr = emulator_set_xcr,
@@ -9209,7 +9203,14 @@ int x86_emulate_instruction(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa,
92099203
ctxt->exception.address = 0;
92109204
}
92119205

9212-
r = x86_emulate_insn(ctxt);
9206+
/*
9207+
* Check L1's instruction intercepts when emulating instructions for
9208+
* L2, unless KVM is re-emulating a previously decoded instruction,
9209+
* e.g. to complete userspace I/O, in which case KVM has already
9210+
* checked the intercepts.
9211+
*/
9212+
r = x86_emulate_insn(ctxt, is_guest_mode(vcpu) &&
9213+
!(emulation_type & EMULTYPE_NO_DECODE));
92139214

92149215
if (r == EMULATION_INTERCEPTED)
92159216
return 1;

0 commit comments

Comments
 (0)