On Fri, Oct 30, 2026 at 2:21 PM Sean Christopherson <[email protected]> wrote: > > For all intents and purposes, revert commit b4b65b5642d6 ("KVM: x86: cleanup > freeing of nested state") and go back to manually tearing down nested state > when freeing a vCPU. > > When a vCPU is destroyed while L2 is active, KVM synthesizes a nested > VM-Exit, which flushes the cached shadow VMCS12 back to guest memory: > > vmx_vcpu_free() > |-> nested_vmx_free_vcpu() > |-> vmx_leave_nested() > |-> nested_vmx_vmexit(vcpu, -1, 0, 0) > |-> nested_flush_cached_shadow_vmcs12() > |-> kvm_write_guest_cached() > |-> __copy_to_user(ghc->hva, ...) > > Accessing user memory via a memslot during VM destruction is broken, as > there are no guarantees that current->mm == kvm->mm when the VM is dying, > because the last reference to the VM can be put from a different process > than the original creating processes. > > And even if the original process does put the final reference, during > process exit, do_exit() calls exit_mm() before closing file descriptors, so > vCPU destruction runs with current->mm == NULL on a borrowed lazy TLB > active_mm. If the borrowed address space has a writable mapping at the > to-be-written userspace address, KVM will corrupt an unrelated task's > memory since uaccess APIs, including __copy_to_user(), don't sanity check > current->mm (and *can't* sanity perform KVM's current->mm == kvm->mm check
s/sanity// > since that is firmly a KVM-only concept). > > Furthermore, the shadow VMCS flaw is only the tip of the iceberg, as the > MSR load/store lists mean that emulating a nested VM-Exit can read or write > an arbitrary set of MSRs, e.g. if MSR_KVM_SYSTEM_TIME_NEW is in L1's MSR > load list: > > kvm_arch_vcpu_destroy() > kvmclock_reset() - takes pv_time off kvm->gpc_list > vmx_vcpu_free() > nested_vmx_free_vcpu() > vmx_leave_nested() > nested_vmx_vmexit() > __nested_vmx_vmexit() > load_vmcs12_host_state() > nested_vmx_load_msr() - writes the MSRs that L1 listed > kvm_cpu_has_injectable_intr() - reads Xen's vcpu_info > kvm_xen_destroy_vcpu() - deactivates the vcpu_info cache > > processing the MSR write puts the "pv_time" cache back on kvm->gpc_list, > and the next list_del() writes into the freed vCPU. And as effectively > pointed out by syzbot[*], attempting to fix each case individually is a > losing game of whack-a-mole. > > Deliberately leave "nested_run_pending" as-is, because even though doing so > creates potentitally inconsistent state, KVM should *never* consume said s/potentitally/potentially/ > state while freeing a vCPU. I.e. if leaving nested_run_pending set trips a > sanity check or bug, then that just means there's more fixing to be done. > > Open code only the "free vCPU" path, i.e. keep emulating a nested VM-Exit > when forcing a vCPU out of L2 via vmx_leave_nested(), as there are a number > of updates buried in the nested VM-Exit flow that are needed if the vCPU is > going to run in L1, e.g. clearing pending exceptions and MTF traps, setting > the TSC offset+scale information back to L1, and so on and so forth. Note, > this means svm_leave_nested() is all but guaranteed to be buggy, but that's > basically an open secret at this point and at least any bugs "only" break > L1, i.e. don't corrupt host state as is the case for abusing nested VM-Exit > when freeing a vCPU. > > Link: > https://lore.kernel.org/all/[email protected] [*] > Fixes: b4b65b5642d6 ("KVM: x86: cleanup freeing of nested state") > Cc: [email protected] > Co-developed-by: Chris Mason <[email protected]> > Signed-off-by: Chris Mason <[email protected]> > Signed-off-by: Sean Christopherson <[email protected]> Reviewed-by: Jim Mattson <[email protected]>
