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

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

[Hyunwoo Kim]

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

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

[Sean Christopherson]

On Sat, Aug 01, 2026, Hyunwoo Kim wrote:
> 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.

Wouldn't this also require an "unclean" shutdown of the VM, because the VM would
still need live memslots in order to process the MSR load/store lists.  I wonder
if that's an avenue to a short-term stopgap "fix" as well as long-term hardening.
E.g. if KVM were to nuke memslots as part of kvm_destroy_vm(), I think that would
plug this particular hole?

> 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.

I don't have the links off-hand, but nSVM's behavior of not emulating VM-Exit
has also led to problems (I think we've failed to account for things that are
handled by the VM-Exit path, on multiple occassions).  That said, emulating a
VM-Exit while a vCPU is being destroyed is beyond awful, e.g. it requires
loading+putting the vCPU, which is its own gigantic can of worms.

> 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").

IIRC, we've accumulated more horrors since then.  I completely agree this code
is buggy and needs to be fixed, but I don't want to take a quick-and-dirty fix,
at least not without an exit strategy, which would/should force us to assess
exactly what is/isn't needed from the __nested_vmx_vmexit() flow. 

> 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
>