[PATCH] KVM: nVMX: Don't load L1's host state when freeing a vCPU

[Hyunwoo Kim <imv4bel@gmail.com>]

Don't load L1's host state when kicking a vCPU out of nested guest mode as
part of freeing the vCPU, as loading host state processes vmcs12's VM-Exit
MSR load list, i.e. reads an (index, value) pair out of guest memory and
feeds it to kvm_emulate_msr_write() with host_initiated=false. Letting the
guest emulate WRMSR against VM-scope state that KVM is actively tearing
down goes sideways in at least two ways.

Writing HV_X64_MSR_ICR sends an IPI, which for a non-shorthand,
non-broadcast destination walks kvm->arch.apic_map to dereference the
target's local APIC. kvm_free_lapic() neither rebuilds nor dirties the map,
and the map is freed only after all vCPUs are destroyed, i.e. the map still
points at the already-freed local APIC of a previously destroyed vCPU. This
requires userspace to expose Hyper-V's CPUID to the guest. Writing
MSR_KVM_SYSTEM_TIME_NEW activates the kvmclock gfn=>pfn cache, which leaves
the cache's list entry, resident in the about-to-be-freed vCPU, linked into
kvm->gpc_list; the next vCPU to manipulate the list writes through that
entry.

The vCPU will never run again, so nothing can observe the loaded host
state. Simply restore KVM's MMU pointers so that they aren't left pointing
at the nested MMU, and bail. nSVM does the same, i.e. doesn't emulate a
VM-Exit when forcibly leaving nested mode, and performs only the equivalent
MMU cleanup.

Bail just before the branch that splits the success and VM-Fail paths, as
leaving guest mode, canceling the VMX-preemption timer, and switching back
to vmcs01 are all needed by the free path. Canceling the timer is in fact
the only reason the free path goes through an emulated VM-Exit, see commit
b4b65b5642d6 ("KVM: x86: cleanup freeing of nested state").

Fixes: b4b65b5642d6 ("KVM: x86: cleanup freeing of nested state")
Cc: stable@vger.kernel.org
Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
---
 arch/x86/kvm/vmx/nested.c | 12 ++++++++++++
 arch/x86/kvm/vmx/vmx.h    |  3 +++
 2 files changed, 15 insertions(+)

diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index ddf6df7bee93b2..8d58547acb1887 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -384,6 +384,8 @@ static void free_nested(struct kvm_vcpu *vcpu)
  */
 void nested_vmx_free_vcpu(struct kvm_vcpu *vcpu)
 {
+	to_vmx(vcpu)->nested.vcpu_is_dying = true;
+
 	vcpu_load(vcpu);
 	vmx_leave_nested(vcpu);
 	vcpu_put(vcpu);
@@ -5173,6 +5175,16 @@ void __nested_vmx_vmexit(struct kvm_vcpu *vcpu, u32 vm_exit_reason,
 	/* in case we halted in L2 */
 	kvm_set_mp_state(vcpu, KVM_MP_STATE_RUNNABLE);
 
+	/*
+	 * Don't emulate guest-controlled state, e.g. vmcs12's VM-Exit MSR load
+	 * list, when freeing the vCPU.  Bail only after leaving guest mode,
+	 * canceling the preemption timer, and switching back to vmcs01.
+	 */
+	if (vmx->nested.vcpu_is_dying) {
+		nested_ept_uninit_mmu_context(vcpu);
+		return;
+	}
+
 	if (likely(!vmx->fail)) {
 		if (vm_exit_reason != -1)
 			trace_kvm_nested_vmexit_inject(vmcs12->vm_exit_reason,
diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h
index dc8517f15bc463..2bacd3fe4c7ded 100644
--- a/arch/x86/kvm/vmx/vmx.h
+++ b/arch/x86/kvm/vmx/vmx.h
@@ -76,6 +76,9 @@ struct nested_vmx {
 	gpa_t vmxon_ptr;
 	bool pml_full;
 
+	/* Set when freeing the vCPU, to suppress emulation of guest state. */
+	bool vcpu_is_dying;
+
 	/* The guest-physical address of the current VMCS L1 keeps for L2 */
 	gpa_t current_vmptr;
 	/*
-- 
2.43.0