Skip to content

Commit 361da27

Browse files
committed
Merge branch 'kvm-nvmx-and-vm-teardown' into HEAD
The immediate issue being fixed here is a nVMX bug where KVM fails to detect that, after nested VM-Exit, L1 has a pending IRQ (or NMI). However, checking for a pending interrupt accesses the legacy PIC, and x86's kvm_arch_destroy_vm() currently frees the PIC before destroying vCPUs, i.e. checking for IRQs during the forced nested VM-Exit results in a NULL pointer deref; that's a prerequisite for the nVMX fix. The remaining patches attempt to bring a bit of sanity to x86's VM teardown code, which has accumulated a lot of cruft over the years. E.g. KVM currently unloads each vCPU's MMUs in a separate operation from destroying vCPUs, all because when guest SMP support was added, KVM had a kludgy MMU teardown flow that broke when a VM had more than one 1 vCPU. And that oddity lived on, for 18 years... Signed-off-by: Paolo Bonzini <[email protected]>
2 parents 0afd104 + b2aba52 commit 361da27

File tree

9 files changed

+22
-36
lines changed

9 files changed

+22
-36
lines changed

arch/arm64/include/asm/kvm_host.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1375,8 +1375,6 @@ static inline bool kvm_system_needs_idmapped_vectors(void)
13751375
return cpus_have_final_cap(ARM64_SPECTRE_V3A);
13761376
}
13771377

1378-
static inline void kvm_arch_sync_events(struct kvm *kvm) {}
1379-
13801378
void kvm_init_host_debug_data(void);
13811379
void kvm_vcpu_load_debug(struct kvm_vcpu *vcpu);
13821380
void kvm_vcpu_put_debug(struct kvm_vcpu *vcpu);

arch/loongarch/include/asm/kvm_host.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -326,7 +326,6 @@ static inline bool kvm_is_ifetch_fault(struct kvm_vcpu_arch *arch)
326326

327327
/* Misc */
328328
static inline void kvm_arch_hardware_unsetup(void) {}
329-
static inline void kvm_arch_sync_events(struct kvm *kvm) {}
330329
static inline void kvm_arch_memslots_updated(struct kvm *kvm, u64 gen) {}
331330
static inline void kvm_arch_vcpu_blocking(struct kvm_vcpu *vcpu) {}
332331
static inline void kvm_arch_vcpu_unblocking(struct kvm_vcpu *vcpu) {}

arch/mips/include/asm/kvm_host.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -886,7 +886,6 @@ extern unsigned long kvm_mips_get_ramsize(struct kvm *kvm);
886886
extern int kvm_vcpu_ioctl_interrupt(struct kvm_vcpu *vcpu,
887887
struct kvm_mips_interrupt *irq);
888888

889-
static inline void kvm_arch_sync_events(struct kvm *kvm) {}
890889
static inline void kvm_arch_free_memslot(struct kvm *kvm,
891890
struct kvm_memory_slot *slot) {}
892891
static inline void kvm_arch_memslots_updated(struct kvm *kvm, u64 gen) {}

arch/powerpc/include/asm/kvm_host.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -902,7 +902,6 @@ struct kvm_vcpu_arch {
902902
#define __KVM_HAVE_ARCH_WQP
903903
#define __KVM_HAVE_CREATE_DEVICE
904904

905-
static inline void kvm_arch_sync_events(struct kvm *kvm) {}
906905
static inline void kvm_arch_memslots_updated(struct kvm *kvm, u64 gen) {}
907906
static inline void kvm_arch_flush_shadow_all(struct kvm *kvm) {}
908907
static inline void kvm_arch_vcpu_blocking(struct kvm_vcpu *vcpu) {}

arch/riscv/include/asm/kvm_host.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -301,8 +301,6 @@ static inline bool kvm_arch_pmi_in_guest(struct kvm_vcpu *vcpu)
301301
return IS_ENABLED(CONFIG_GUEST_PERF_EVENTS) && !!vcpu;
302302
}
303303

304-
static inline void kvm_arch_sync_events(struct kvm *kvm) {}
305-
306304
#define KVM_RISCV_GSTAGE_TLB_MIN_ORDER 12
307305

308306
void kvm_riscv_local_hfence_gvma_vmid_gpa(unsigned long vmid,

arch/s390/include/asm/kvm_host.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1056,7 +1056,6 @@ bool kvm_s390_pv_cpu_is_protected(struct kvm_vcpu *vcpu);
10561056
extern int kvm_s390_gisc_register(struct kvm *kvm, u32 gisc);
10571057
extern int kvm_s390_gisc_unregister(struct kvm *kvm, u32 gisc);
10581058

1059-
static inline void kvm_arch_sync_events(struct kvm *kvm) {}
10601059
static inline void kvm_arch_free_memslot(struct kvm *kvm,
10611060
struct kvm_memory_slot *slot) {}
10621061
static inline void kvm_arch_memslots_updated(struct kvm *kvm, u64 gen) {}

arch/x86/kvm/x86.c

Lines changed: 14 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -12369,6 +12369,9 @@ void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu)
1236912369
{
1237012370
int idx;
1237112371

12372+
kvm_clear_async_pf_completion_queue(vcpu);
12373+
kvm_mmu_unload(vcpu);
12374+
1237212375
kvmclock_reset(vcpu);
1237312376

1237412377
kvm_x86_call(vcpu_free)(vcpu);
@@ -12762,31 +12765,6 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
1276212765
return ret;
1276312766
}
1276412767

12765-
static void kvm_unload_vcpu_mmu(struct kvm_vcpu *vcpu)
12766-
{
12767-
vcpu_load(vcpu);
12768-
kvm_mmu_unload(vcpu);
12769-
vcpu_put(vcpu);
12770-
}
12771-
12772-
static void kvm_unload_vcpu_mmus(struct kvm *kvm)
12773-
{
12774-
unsigned long i;
12775-
struct kvm_vcpu *vcpu;
12776-
12777-
kvm_for_each_vcpu(i, vcpu, kvm) {
12778-
kvm_clear_async_pf_completion_queue(vcpu);
12779-
kvm_unload_vcpu_mmu(vcpu);
12780-
}
12781-
}
12782-
12783-
void kvm_arch_sync_events(struct kvm *kvm)
12784-
{
12785-
cancel_delayed_work_sync(&kvm->arch.kvmclock_sync_work);
12786-
cancel_delayed_work_sync(&kvm->arch.kvmclock_update_work);
12787-
kvm_free_pit(kvm);
12788-
}
12789-
1279012768
/**
1279112769
* __x86_set_memory_region: Setup KVM internal memory slot
1279212770
*
@@ -12865,6 +12843,17 @@ EXPORT_SYMBOL_GPL(__x86_set_memory_region);
1286512843

1286612844
void kvm_arch_pre_destroy_vm(struct kvm *kvm)
1286712845
{
12846+
/*
12847+
* Stop all background workers and kthreads before destroying vCPUs, as
12848+
* iterating over vCPUs in a different task while vCPUs are being freed
12849+
* is unsafe, i.e. will lead to use-after-free. The PIT also needs to
12850+
* be stopped before IRQ routing is freed.
12851+
*/
12852+
cancel_delayed_work_sync(&kvm->arch.kvmclock_sync_work);
12853+
cancel_delayed_work_sync(&kvm->arch.kvmclock_update_work);
12854+
12855+
kvm_free_pit(kvm);
12856+
1286812857
kvm_mmu_pre_destroy_vm(kvm);
1286912858
}
1287012859

@@ -12884,7 +12873,6 @@ void kvm_arch_destroy_vm(struct kvm *kvm)
1288412873
__x86_set_memory_region(kvm, TSS_PRIVATE_MEMSLOT, 0, 0);
1288512874
mutex_unlock(&kvm->slots_lock);
1288612875
}
12887-
kvm_unload_vcpu_mmus(kvm);
1288812876
kvm_destroy_vcpus(kvm);
1288912877
kvm_x86_call(vm_destroy)(kvm);
1289012878
kvm_free_msr_filter(srcu_dereference_check(kvm->arch.msr_filter, &kvm->srcu, 1));

include/linux/kvm_host.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1747,7 +1747,6 @@ static inline void kvm_unregister_perf_callbacks(void) {}
17471747

17481748
int kvm_arch_init_vm(struct kvm *kvm, unsigned long type);
17491749
void kvm_arch_destroy_vm(struct kvm *kvm);
1750-
void kvm_arch_sync_events(struct kvm *kvm);
17511750

17521751
int kvm_cpu_has_pending_timer(struct kvm_vcpu *vcpu);
17531752

virt/kvm/kvm_main.c

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -489,6 +489,14 @@ void kvm_destroy_vcpus(struct kvm *kvm)
489489
kvm_for_each_vcpu(i, vcpu, kvm) {
490490
kvm_vcpu_destroy(vcpu);
491491
xa_erase(&kvm->vcpu_array, i);
492+
493+
/*
494+
* Assert that the vCPU isn't visible in any way, to ensure KVM
495+
* doesn't trigger a use-after-free if destroying vCPUs results
496+
* in VM-wide request, e.g. to flush remote TLBs when tearing
497+
* down MMUs, or to mark the VM dead if a KVM_BUG_ON() fires.
498+
*/
499+
WARN_ON_ONCE(xa_load(&kvm->vcpu_array, i) || kvm_get_vcpu(kvm, i));
492500
}
493501

494502
atomic_set(&kvm->online_vcpus, 0);
@@ -1263,7 +1271,6 @@ static void kvm_destroy_vm(struct kvm *kvm)
12631271
kvm_destroy_pm_notifier(kvm);
12641272
kvm_uevent_notify_change(KVM_EVENT_DESTROY_VM, kvm);
12651273
kvm_destroy_vm_debugfs(kvm);
1266-
kvm_arch_sync_events(kvm);
12671274
mutex_lock(&kvm_lock);
12681275
list_del(&kvm->vm_list);
12691276
mutex_unlock(&kvm_lock);

0 commit comments

Comments
 (0)