Skip to content

Commit f7e5707

Browse files
sean-jcbonzini
authored andcommitted
KVM: x86: Forcibly leave nested virt when SMM state is toggled
Forcibly leave nested virtualization operation if userspace toggles SMM state via KVM_SET_VCPU_EVENTS or KVM_SYNC_X86_EVENTS. If userspace forces the vCPU out of SMM while it's post-VMXON and then injects an SMI, vmx_enter_smm() will overwrite vmx->nested.smm.vmxon and end up with both vmxon=false and smm.vmxon=false, but all other nVMX state allocated. Don't attempt to gracefully handle the transition as (a) most transitions are nonsencial, e.g. forcing SMM while L2 is running, (b) there isn't sufficient information to handle all transitions, e.g. SVM wants access to the SMRAM save state, and (c) KVM_SET_VCPU_EVENTS must precede KVM_SET_NESTED_STATE during state restore as the latter disallows putting the vCPU into L2 if SMM is active, and disallows tagging the vCPU as being post-VMXON in SMM if SMM is not active. Abuse of KVM_SET_VCPU_EVENTS manifests as a WARN and memory leak in nVMX due to failure to free vmcs01's shadow VMCS, but the bug goes far beyond just a memory leak, e.g. toggling SMM on while L2 is active puts the vCPU in an architecturally impossible state. WARNING: CPU: 0 PID: 3606 at free_loaded_vmcs arch/x86/kvm/vmx/vmx.c:2665 [inline] WARNING: CPU: 0 PID: 3606 at free_loaded_vmcs+0x158/0x1a0 arch/x86/kvm/vmx/vmx.c:2656 Modules linked in: CPU: 1 PID: 3606 Comm: syz-executor725 Not tainted 5.17.0-rc1-syzkaller #0 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 RIP: 0010:free_loaded_vmcs arch/x86/kvm/vmx/vmx.c:2665 [inline] RIP: 0010:free_loaded_vmcs+0x158/0x1a0 arch/x86/kvm/vmx/vmx.c:2656 Code: <0f> 0b eb b3 e8 8f 4d 9f 00 e9 f7 fe ff ff 48 89 df e8 92 4d 9f 00 Call Trace: <TASK> kvm_arch_vcpu_destroy+0x72/0x2f0 arch/x86/kvm/x86.c:11123 kvm_vcpu_destroy arch/x86/kvm/../../../virt/kvm/kvm_main.c:441 [inline] kvm_destroy_vcpus+0x11f/0x290 arch/x86/kvm/../../../virt/kvm/kvm_main.c:460 kvm_free_vcpus arch/x86/kvm/x86.c:11564 [inline] kvm_arch_destroy_vm+0x2e8/0x470 arch/x86/kvm/x86.c:11676 kvm_destroy_vm arch/x86/kvm/../../../virt/kvm/kvm_main.c:1217 [inline] kvm_put_kvm+0x4fa/0xb00 arch/x86/kvm/../../../virt/kvm/kvm_main.c:1250 kvm_vm_release+0x3f/0x50 arch/x86/kvm/../../../virt/kvm/kvm_main.c:1273 __fput+0x286/0x9f0 fs/file_table.c:311 task_work_run+0xdd/0x1a0 kernel/task_work.c:164 exit_task_work include/linux/task_work.h:32 [inline] do_exit+0xb29/0x2a30 kernel/exit.c:806 do_group_exit+0xd2/0x2f0 kernel/exit.c:935 get_signal+0x4b0/0x28c0 kernel/signal.c:2862 arch_do_signal_or_restart+0x2a9/0x1c40 arch/x86/kernel/signal.c:868 handle_signal_work kernel/entry/common.c:148 [inline] exit_to_user_mode_loop kernel/entry/common.c:172 [inline] exit_to_user_mode_prepare+0x17d/0x290 kernel/entry/common.c:207 __syscall_exit_to_user_mode_work kernel/entry/common.c:289 [inline] syscall_exit_to_user_mode+0x19/0x60 kernel/entry/common.c:300 do_syscall_64+0x42/0xb0 arch/x86/entry/common.c:86 entry_SYSCALL_64_after_hwframe+0x44/0xae </TASK> Cc: [email protected] Reported-by: [email protected] Signed-off-by: Sean Christopherson <[email protected]> Message-Id: <[email protected]> Signed-off-by: Paolo Bonzini <[email protected]>
1 parent aa3b39f commit f7e5707

File tree

6 files changed

+12
-7
lines changed

6 files changed

+12
-7
lines changed

arch/x86/include/asm/kvm_host.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1496,6 +1496,7 @@ struct kvm_x86_ops {
14961496
};
14971497

14981498
struct kvm_x86_nested_ops {
1499+
void (*leave_nested)(struct kvm_vcpu *vcpu);
14991500
int (*check_events)(struct kvm_vcpu *vcpu);
15001501
bool (*hv_timer_pending)(struct kvm_vcpu *vcpu);
15011502
void (*triple_fault)(struct kvm_vcpu *vcpu);

arch/x86/kvm/svm/nested.c

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -983,9 +983,9 @@ void svm_free_nested(struct vcpu_svm *svm)
983983
/*
984984
* Forcibly leave nested mode in order to be able to reset the VCPU later on.
985985
*/
986-
void svm_leave_nested(struct vcpu_svm *svm)
986+
void svm_leave_nested(struct kvm_vcpu *vcpu)
987987
{
988-
struct kvm_vcpu *vcpu = &svm->vcpu;
988+
struct vcpu_svm *svm = to_svm(vcpu);
989989

990990
if (is_guest_mode(vcpu)) {
991991
svm->nested.nested_run_pending = 0;
@@ -1411,7 +1411,7 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,
14111411
return -EINVAL;
14121412

14131413
if (!(kvm_state->flags & KVM_STATE_NESTED_GUEST_MODE)) {
1414-
svm_leave_nested(svm);
1414+
svm_leave_nested(vcpu);
14151415
svm_set_gif(svm, !!(kvm_state->flags & KVM_STATE_NESTED_GIF_SET));
14161416
return 0;
14171417
}
@@ -1478,7 +1478,7 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,
14781478
*/
14791479

14801480
if (is_guest_mode(vcpu))
1481-
svm_leave_nested(svm);
1481+
svm_leave_nested(vcpu);
14821482
else
14831483
svm->nested.vmcb02.ptr->save = svm->vmcb01.ptr->save;
14841484

@@ -1532,6 +1532,7 @@ static bool svm_get_nested_state_pages(struct kvm_vcpu *vcpu)
15321532
}
15331533

15341534
struct kvm_x86_nested_ops svm_nested_ops = {
1535+
.leave_nested = svm_leave_nested,
15351536
.check_events = svm_check_nested_events,
15361537
.triple_fault = nested_svm_triple_fault,
15371538
.get_nested_state_pages = svm_get_nested_state_pages,

arch/x86/kvm/svm/svm.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -290,7 +290,7 @@ int svm_set_efer(struct kvm_vcpu *vcpu, u64 efer)
290290

291291
if ((old_efer & EFER_SVME) != (efer & EFER_SVME)) {
292292
if (!(efer & EFER_SVME)) {
293-
svm_leave_nested(svm);
293+
svm_leave_nested(vcpu);
294294
svm_set_gif(svm, true);
295295
/* #GP intercept is still needed for vmware backdoor */
296296
if (!enable_vmware_backdoor)

arch/x86/kvm/svm/svm.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -520,7 +520,7 @@ static inline bool nested_exit_on_nmi(struct vcpu_svm *svm)
520520

521521
int enter_svm_guest_mode(struct kvm_vcpu *vcpu,
522522
u64 vmcb_gpa, struct vmcb *vmcb12, bool from_vmrun);
523-
void svm_leave_nested(struct vcpu_svm *svm);
523+
void svm_leave_nested(struct kvm_vcpu *vcpu);
524524
void svm_free_nested(struct vcpu_svm *svm);
525525
int svm_allocate_nested(struct vcpu_svm *svm);
526526
int nested_svm_vmrun(struct kvm_vcpu *vcpu);

arch/x86/kvm/vmx/nested.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6771,6 +6771,7 @@ __init int nested_vmx_hardware_setup(int (*exit_handlers[])(struct kvm_vcpu *))
67716771
}
67726772

67736773
struct kvm_x86_nested_ops vmx_nested_ops = {
6774+
.leave_nested = vmx_leave_nested,
67746775
.check_events = vmx_check_nested_events,
67756776
.hv_timer_pending = nested_vmx_preemption_timer_pending,
67766777
.triple_fault = nested_vmx_triple_fault,

arch/x86/kvm/x86.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4860,8 +4860,10 @@ static int kvm_vcpu_ioctl_x86_set_vcpu_events(struct kvm_vcpu *vcpu,
48604860
vcpu->arch.apic->sipi_vector = events->sipi_vector;
48614861

48624862
if (events->flags & KVM_VCPUEVENT_VALID_SMM) {
4863-
if (!!(vcpu->arch.hflags & HF_SMM_MASK) != events->smi.smm)
4863+
if (!!(vcpu->arch.hflags & HF_SMM_MASK) != events->smi.smm) {
4864+
kvm_x86_ops.nested_ops->leave_nested(vcpu);
48644865
kvm_smm_changed(vcpu, events->smi.smm);
4866+
}
48654867

48664868
vcpu->arch.smi_pending = events->smi.pending;
48674869

0 commit comments

Comments
 (0)