Re: [PATCH] KVM: nVMX: Don't flush shadow VMCS12 to guest memory during vCPU teardown

[Sean Christopherson <seanjc@google.com>]

On Thu, Sep 10, 2026, James Houghton wrote:
> On Wed, Sep 9, 2026 at 12:00 PM Sean Christopherson <seanjc@google.com> wrote:
> > If the above works for PPC, then KVM can nuke memslots before calling into
> > kvm_arch_destroy_vm().  x86's asinine memslot deletion in kvm_arch_destroy_vm()
> > needs to be addressed, but that code exists purely to do vm_munmap(), and can
> > and should be moved to kvm_arch_free_memslot().
> >
> > All that said, I'm not sure this aggressive fix is the right thing to send to
> > stable@.  For that, James' suggestion of hardening KVM's usage of
> > __copy_{to,from}_user() seems like the best blend of being comprehensive without
> > being overly invasive/risky.
> 
> This seems kind of nightmareish to backport; there are a lot of
> copy_*_user() callsites that will need updating. Maybe I have a
> different idea of the diff you're suggesting.

Nah, it's not many, because it's only the __copy_{to,from}_user{,inatomic}() usage
that needs handling.  Everything else is strictly scoped to an ioctl, where (a)
current->mm can't be NULL and (b) KVM doesn't make any assumption about the address
space.

At a glance, it's 11 total: 5 in virt/kvm, 4 in vmx.c, and 2 in PPC's book3s_64_mmu_radix.c.
Well, plus 4 more to also harden {,__}kvm_{get,put}_guest().

And even if that number were doubled or tripled, the backports would still be
relatively easy.  The overwhelming majority won't conflict, and the few that do
should be trivial to resolve (more than likely, simply drop the change).

> I wish we could just change the uaccess primitives, like
> {,__}access_ok(), to check that `current->mm` is not NULL (and WARN
> and return -EFAULT if it is NULL).

That wouldn't help at all in this case, because the access_ok() check is done
when memslots are modified.  Which is the crux of KVM's problems: KVM decouples
the initial checks from the accesses, relying on kvm->mm to

And even if we hardened all of the uaccess helpers, we'd _still_ have problems,
because it's not just a NULL current->mm that's problematic.  The last reference
to a VM file, i.e. to struct kvm, can be put by a different _process_.  I.e. KVM
still needs to guard against reading/writing guest memory using a valid, non-NULL
current->mm that isn't kvm->mm.  That can't be genericized in the uaccess APIs,
because the rule that only a specific address space can be used is very much unique
to KVM.

> That diff is also not trivial to backport; many arch implementations would
> need updating. I have half a mind to send an RFC patch to linux-mm@ to see
> what they think. :)
> 
> > So, as an immediate set of changes, what if we do this over ~5 patches, with patches
> > 1 and 2 tagged for stable@?
> >
> >   1. Add kvm_copy_{to,from}_user{,_inatomic)() and return -EFAULT if current->mm
> >      is not kvm->mm.
> 
> SGTM. This is not mutually exclusive with the generic uaccess changes
> I'm suggesting above. If this is actually reasonably backportable,
> sure let's backport it.
> 
> >   2. Hack-a-fix PPC's kvm_arch_flush_shadow_all().
> >   3. Do x86's vm_munmap() in kvm_arch_free_memslot().
> >   4. Nuke memslots before calling kvm_arch_destroy_vm().
> >   5. Change the current->mm checks in kvm_copy_{to,from}_user{,_inatomic)() to
> >      WARN_ON_ONCE() on failure.
> >
> > And then in the near-ish future, take things a step further and do:
> >
> >   6. Fix the vmx_leave_nested() trainwreck.
> >   7. Harden the common kvm_{read,write}_guest family of APIs even further by
> >      adding an early WARN_ON_ONCE() on current->mm != kvm->mm, i.e. to detect
> >      bad KVM behavior as additional defense-in-depth.
> 
> This all SGTM, thanks Sean.