Skip to content

Commit 6fe13d8

Browse files
sean-jcbonzini
authored andcommitted
KVM: VMX: Reject KVM_RUN if emulation is required with pending exception
Reject KVM_RUN if emulation is required (because VMX is running without unrestricted guest) and an exception is pending, as KVM doesn't support emulating exceptions except when emulating real mode via vm86. The vCPU is hosed either way, but letting KVM_RUN proceed triggers a WARN due to the impossible condition. Alternatively, the WARN could be removed, but then userspace and/or KVM bugs would result in the vCPU silently running in a bad state, which isn't very friendly to users. Originally, the bug was hit by syzkaller with a nested guest as that doesn't require kvm_intel.unrestricted_guest=0. That particular flavor is likely fixed by commit cd0e615 ("KVM: nVMX: Synthesize TRIPLE_FAULT for L2 if emulation is required"), but it's trivial to trigger the WARN with a non-nested guest, and userspace can likely force bad state via ioctls() for a nested guest as well. Checking for the impossible condition needs to be deferred until KVM_RUN because KVM can't force specific ordering between ioctls. E.g. clearing exception.pending in KVM_SET_SREGS doesn't prevent userspace from setting it in KVM_SET_VCPU_EVENTS, and disallowing KVM_SET_VCPU_EVENTS with emulation_required would prevent userspace from queuing an exception and then stuffing sregs. Note, if KVM were to try and detect/prevent the condition prior to KVM_RUN, handle_invalid_guest_state() and/or handle_emulation_failure() would need to be modified to clear the pending exception prior to exiting to userspace. ------------[ cut here ]------------ WARNING: CPU: 6 PID: 137812 at arch/x86/kvm/vmx/vmx.c:1623 vmx_queue_exception+0x14f/0x160 [kvm_intel] CPU: 6 PID: 137812 Comm: vmx_invalid_nes Not tainted 5.15.2-7cc36c3e14ae-pop torvalds#279 Hardware name: ASUS Q87M-E/Q87M-E, BIOS 1102 03/03/2014 RIP: 0010:vmx_queue_exception+0x14f/0x160 [kvm_intel] Code: <0f> 0b e9 fd fe ff ff 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 RSP: 0018:ffffa45c83577d38 EFLAGS: 00010202 RAX: 0000000000000003 RBX: 0000000080000006 RCX: 0000000000000006 RDX: 0000000000000000 RSI: 0000000000010002 RDI: ffff9916af734000 RBP: ffff9916af734000 R08: 0000000000000000 R09: 0000000000000000 R10: 0000000000000000 R11: 0000000000000001 R12: 0000000000000006 R13: 0000000000000000 R14: ffff9916af734038 R15: 0000000000000000 FS: 00007f1e1a47c740(0000) GS:ffff99188fb80000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 00007f1e1a6a8008 CR3: 000000026f83b005 CR4: 00000000001726e0 Call Trace: kvm_arch_vcpu_ioctl_run+0x13a2/0x1f20 [kvm] kvm_vcpu_ioctl+0x279/0x690 [kvm] __x64_sys_ioctl+0x83/0xb0 do_syscall_64+0x3b/0xc0 entry_SYSCALL_64_after_hwframe+0x44/0xae Reported-by: [email protected] Signed-off-by: Sean Christopherson <[email protected]> Message-Id: <[email protected]> Signed-off-by: Paolo Bonzini <[email protected]>
1 parent 579f15d commit 6fe13d8

File tree

5 files changed

+37
-5
lines changed

5 files changed

+37
-5
lines changed

arch/x86/include/asm/kvm-x86-ops.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ KVM_X86_OP_NULL(tlb_remote_flush)
5555
KVM_X86_OP_NULL(tlb_remote_flush_with_range)
5656
KVM_X86_OP(tlb_flush_gva)
5757
KVM_X86_OP(tlb_flush_guest)
58+
KVM_X86_OP(vcpu_pre_run)
5859
KVM_X86_OP(run)
5960
KVM_X86_OP_NULL(handle_exit)
6061
KVM_X86_OP_NULL(skip_emulated_instruction)

arch/x86/include/asm/kvm_host.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1380,6 +1380,7 @@ struct kvm_x86_ops {
13801380
*/
13811381
void (*tlb_flush_guest)(struct kvm_vcpu *vcpu);
13821382

1383+
int (*vcpu_pre_run)(struct kvm_vcpu *vcpu);
13831384
enum exit_fastpath_completion (*run)(struct kvm_vcpu *vcpu);
13841385
int (*handle_exit)(struct kvm_vcpu *vcpu,
13851386
enum exit_fastpath_completion exit_fastpath);

arch/x86/kvm/svm/svm.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3829,6 +3829,11 @@ static void svm_cancel_injection(struct kvm_vcpu *vcpu)
38293829
svm_complete_interrupts(vcpu);
38303830
}
38313831

3832+
static int svm_vcpu_pre_run(struct kvm_vcpu *vcpu)
3833+
{
3834+
return 1;
3835+
}
3836+
38323837
static fastpath_t svm_exit_handlers_fastpath(struct kvm_vcpu *vcpu)
38333838
{
38343839
if (to_svm(vcpu)->vmcb->control.exit_code == SVM_EXIT_MSR &&
@@ -4658,6 +4663,7 @@ static struct kvm_x86_ops svm_x86_ops __initdata = {
46584663
.tlb_flush_gva = svm_flush_tlb_gva,
46594664
.tlb_flush_guest = svm_flush_tlb,
46604665

4666+
.vcpu_pre_run = svm_vcpu_pre_run,
46614667
.run = svm_vcpu_run,
46624668
.handle_exit = handle_exit,
46634669
.skip_emulated_instruction = skip_emulated_instruction,

arch/x86/kvm/vmx/vmx.c

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5426,6 +5426,14 @@ static int handle_nmi_window(struct kvm_vcpu *vcpu)
54265426
return 1;
54275427
}
54285428

5429+
static bool vmx_emulation_required_with_pending_exception(struct kvm_vcpu *vcpu)
5430+
{
5431+
struct vcpu_vmx *vmx = to_vmx(vcpu);
5432+
5433+
return vmx->emulation_required && !vmx->rmode.vm86_active &&
5434+
vcpu->arch.exception.pending;
5435+
}
5436+
54295437
static int handle_invalid_guest_state(struct kvm_vcpu *vcpu)
54305438
{
54315439
struct vcpu_vmx *vmx = to_vmx(vcpu);
@@ -5445,8 +5453,7 @@ static int handle_invalid_guest_state(struct kvm_vcpu *vcpu)
54455453
if (!kvm_emulate_instruction(vcpu, 0))
54465454
return 0;
54475455

5448-
if (vmx->emulation_required && !vmx->rmode.vm86_active &&
5449-
vcpu->arch.exception.pending) {
5456+
if (vmx_emulation_required_with_pending_exception(vcpu)) {
54505457
kvm_prepare_emulation_failure_exit(vcpu);
54515458
return 0;
54525459
}
@@ -5468,6 +5475,16 @@ static int handle_invalid_guest_state(struct kvm_vcpu *vcpu)
54685475
return 1;
54695476
}
54705477

5478+
static int vmx_vcpu_pre_run(struct kvm_vcpu *vcpu)
5479+
{
5480+
if (vmx_emulation_required_with_pending_exception(vcpu)) {
5481+
kvm_prepare_emulation_failure_exit(vcpu);
5482+
return 0;
5483+
}
5484+
5485+
return 1;
5486+
}
5487+
54715488
static void grow_ple_window(struct kvm_vcpu *vcpu)
54725489
{
54735490
struct vcpu_vmx *vmx = to_vmx(vcpu);
@@ -7708,6 +7725,7 @@ static struct kvm_x86_ops vmx_x86_ops __initdata = {
77087725
.tlb_flush_gva = vmx_flush_tlb_gva,
77097726
.tlb_flush_guest = vmx_flush_tlb_guest,
77107727

7728+
.vcpu_pre_run = vmx_vcpu_pre_run,
77117729
.run = vmx_vcpu_run,
77127730
.handle_exit = vmx_handle_exit,
77137731
.skip_emulated_instruction = vmx_skip_emulated_instruction,

arch/x86/kvm/x86.c

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10393,10 +10393,16 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
1039310393
} else
1039410394
WARN_ON(vcpu->arch.pio.count || vcpu->mmio_needed);
1039510395

10396-
if (kvm_run->immediate_exit)
10396+
if (kvm_run->immediate_exit) {
1039710397
r = -EINTR;
10398-
else
10399-
r = vcpu_run(vcpu);
10398+
goto out;
10399+
}
10400+
10401+
r = static_call(kvm_x86_vcpu_pre_run)(vcpu);
10402+
if (r <= 0)
10403+
goto out;
10404+
10405+
r = vcpu_run(vcpu);
1040010406

1040110407
out:
1040210408
kvm_put_guest_fpu(vcpu);

0 commit comments

Comments
 (0)