Re: [PATCH] KVM: nVMX: Don't load L1's host state when freeing a vCPU
Sean Christopherson <[email protected]> Mon, 3 Aug 2026 17:44:22 -0700
| Newsgroups | org.kernel.vger.linux-kernel,org.kernel.vger.kvm |
|---|---|
| Message-ID | <[email protected]> |
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: [email protected]
> Signed-off-by: Hyunwoo Kim <[email protected]>
> ---
> 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
>