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

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

[Jim Mattson]

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

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 ghc->hva, __copy_to_user() corrupts an unrelated task's memory.

Skip the flush when KVM synthesizes a VM-exit with vm_exit_reason == -1
(e.g. during vCPU teardown or SMM entry). In these paths, KVM forces the
vCPU out of guest mode internally--no architectural VM-exit is delivered
to L1.

Fixes: 61ada7488ffd ("KVM: nVMX: Cache shadow vmcs12 on VMEntry and flush to memory on VMExit")
Cc: stable@vger.kernel.org
Signed-off-by: Jim Mattson <jmattson@google.com>
---
 arch/x86/kvm/vmx/nested.c | 29 ++++++++++++++++++-----------
 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index 151873407abd..33dc8a07019f 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -5121,21 +5121,28 @@ void __nested_vmx_vmexit(struct kvm_vcpu *vcpu, u32 vm_exit_reason,
 	if (likely(!vmx->fail)) {
 		sync_vmcs02_to_vmcs12(vcpu, vmcs12);
 
-		if (vm_exit_reason != -1)
+		if (vm_exit_reason != -1) {
 			prepare_vmcs12(vcpu, vmcs12, vm_exit_reason,
 				       exit_intr_info, exit_qualification,
 				       exit_insn_len);
 
-		/*
-		 * Must happen outside of sync_vmcs02_to_vmcs12() as it will
-		 * also be used to capture vmcs12 cache as part of
-		 * capturing nVMX state for snapshot (migration).
-		 *
-		 * Otherwise, this flush will dirty guest memory at a
-		 * point it is already assumed by user-space to be
-		 * immutable.
-		 */
-		nested_flush_cached_shadow_vmcs12(vcpu, vmcs12);
+			/*
+			 * Must happen outside of sync_vmcs02_to_vmcs12() as it will
+			 * also be used to capture vmcs12 cache as part of
+			 * capturing nVMX state for snapshot (migration).
+			 *
+			 * Otherwise, this flush will dirty guest memory at a
+			 * point it is already assumed by user-space to be
+			 * immutable.
+			 *
+			 * Do not flush when vm_exit_reason == -1 (e.g. forced exit
+			 * on vCPU teardown or SMM entry). On teardown, KVM may run
+			 * with current->mm == NULL on a borrowed lazy TLB mm,
+			 * where writing guest memory would corrupt an unrelated
+			 * task. Nothing can observe the flush in that case anyway.
+			 */
+			nested_flush_cached_shadow_vmcs12(vcpu, vmcs12);
+		}
 	} else {
 		/*
 		 * The only expected VM-instruction error is "VM entry with

base-commit: 28924df2a08f440c73991b83028032c901de2ae4
-- 
2.55.0.979.g7e5102b832-goog

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Live migration during SMM with an active nested guest overwrites L2 state (e.g. RIP/RSP) with stale data from the hardware shadow VMCS.
- [High] Live migration during SMM with nested shadow VMCS enabled causes a Triple Fault on the destination.
- [High] During vCPU teardown, __nested_vmx_vmexit() unconditionally loads L1 host state, triggering a cross-process memory read via load_pdptrs() with a borrowed active_mm.
--

commit 15d3183acc0011b1c1063bab25d832a346f7a45c
Author: Jim Mattson <jmattson@google.com>

KVM: nVMX: Don't flush shadow VMCS12 to guest memory during vCPU teardown

When a vCPU is destroyed while L2 is active, KVM synthesizes a nested VM-Exit.
During this process exit, the vCPU destruction runs with a borrowed lazy TLB 
active_mm, so skipping the shadow VMCS12 flush prevents __copy_to_user() 
from corrupting an unrelated task's memory.

> diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
> index 151873407abd3..33dc8a07019f8 100644
> --- a/arch/x86/kvm/vmx/nested.c
> +++ b/arch/x86/kvm/vmx/nested.c
> @@ -5121,21 +5121,28 @@ void __nested_vmx_vmexit(struct kvm_vcpu *vcpu, u32 vm_exit_reason,
>  	if (likely(!vmx->fail)) {
>  		sync_vmcs02_to_vmcs12(vcpu, vmcs12);
>  
> -		if (vm_exit_reason != -1)
> +		if (vm_exit_reason != -1) {
>  			prepare_vmcs12(vcpu, vmcs12, vm_exit_reason,
>  				       exit_intr_info, exit_qualification,
>  				       exit_insn_len);
>  
> -		/*
> -		 * Must happen outside of sync_vmcs02_to_vmcs12() as it will
> -		 * also be used to capture vmcs12 cache as part of
> -		 * capturing nVMX state for snapshot (migration).
> -		 *
> -		 * Otherwise, this flush will dirty guest memory at a
> -		 * point it is already assumed by user-space to be
> -		 * immutable.
> -		 */
> -		nested_flush_cached_shadow_vmcs12(vcpu, vmcs12);
> +			/*
> +			 * Must happen outside of sync_vmcs02_to_vmcs12() as it will
> +			 * also be used to capture vmcs12 cache as part of
> +			 * capturing nVMX state for snapshot (migration).
> +			 *
> +			 * Otherwise, this flush will dirty guest memory at a
> +			 * point it is already assumed by user-space to be
> +			 * immutable.
> +			 *
> +			 * Do not flush when vm_exit_reason == -1 (e.g. forced exit
> +			 * on vCPU teardown or SMM entry). On teardown, KVM may run
> +			 * with current->mm == NULL on a borrowed lazy TLB mm,
> +			 * where writing guest memory would corrupt an unrelated
> +			 * task. Nothing can observe the flush in that case anyway.
> +			 */
> +			nested_flush_cached_shadow_vmcs12(vcpu, vmcs12);
> +		}
>  	} else {
>  		/*
>  		 * The only expected VM-instruction error is "VM entry with

[Severity: High]
This is a pre-existing issue, but since this patch handles vCPU teardown with
a borrowed active_mm, does the later call to load_vmcs12_host_state() in
__nested_vmx_vmexit() have a similar issue?

During vCPU teardown, __nested_vmx_vmexit() unconditionally loads L1 host
state:

arch/x86/kvm/vmx/nested.c:__nested_vmx_vmexit() {
    ...
    load_vmcs12_host_state(vcpu, vmcs12);
    ...
}

If the L1 host state uses PAE paging, nested_vmx_load_cr3() triggers the
PDPTR load:

arch/x86/kvm/vmx/nested.c:nested_vmx_load_cr3() {
    ...
    if (reload_pdptrs && !nested_ept && is_pae_paging(vcpu) &&
        CC(!load_pdptrs(vcpu, cr3))) {
    ...
}

This executes a memory read using copy_from_user() in the context of the
borrowed active_mm:

arch/x86/kvm/regs.c:load_pdptrs() {
    ...
    ret = kvm_vcpu_read_guest_page(vcpu, gpa_to_gfn(real_gpa), pdpte,
                                   cr3 & GENMASK(11, 5), sizeof(pdpte));
    ...
}

Could this cross-process memory read cause destructive side-effects if it hits
a memory-mapped I/O (MMIO) region in the unrelated task?


[Severity: Critical]
This is a pre-existing issue, but does skipping the shadow VMCS12 flush on
SMM entry (where vm_exit_reason == -1) cause data corruption during live
migration?

In __nested_vmx_vmexit(), need_vmcs12_to_shadow_sync is prevented from being
set on SMM entry because of the vm_exit_reason check:

arch/x86/kvm/vmx/nested.c:__nested_vmx_vmexit() {
    ...
    if ((vm_exit_reason != -1) &&
        (enable_shadow_vmcs || nested_vmx_is_evmptr12_valid(vmx)))
        vmx->nested.need_vmcs12_to_shadow_sync = true;
    ...
}

If live migration is initiated while in SMM, vmx_get_nested_state() treats
the hardware shadow VMCS as authoritative:

arch/x86/kvm/vmx/nested.c:vmx_get_nested_state() {
    ...
    else if (enable_shadow_vmcs)
        copy_shadow_to_vmcs12(vmx);
    ...
}

Since need_vmcs12_to_shadow_sync was not set, doesn't this overwrite the
correctly synced vmcs12 with stale data from the hardware shadow VMCS,
resulting in guest state rollback (e.g., RIP/RSP)?


[Severity: High]
This isn't a bug introduced by this patch, but is there a Triple Fault risk
during live migration in SMM with nested shadow VMCS enabled?

Because is_guest_mode is false in SMM, the shadow_vmcs12 is omitted from the
migration stream:

arch/x86/kvm/vmx/nested.c:vmx_get_nested_state() {
    ...
    if (is_guest_mode(vcpu) &&
        nested_cpu_has_shadow_vmcs(vmcs12) &&
        vmcs12->vmcs_link_pointer != INVALID_GPA)
        kvm_state.size += sizeof(user_vmx_nested_state->shadow_vmcs12);
    ...
}

On the destination, vmx_set_nested_state() skips restoring shadow_vmcs12 and
returns early:

arch/x86/kvm/vmx/nested.c:vmx_set_nested_state() {
    ...
    if (!(kvm_state->flags & KVM_STATE_NESTED_GUEST_MODE))
        return 0;
    ...
}

When the guest leaves SMM, vmx_leave_smm() checks the revision ID of the
zeroed shadow_vmcs12:

arch/x86/kvm/vmx/vmx.c:vmx_leave_smm() {
    ...
    /* Triple fault if the state is invalid.  */
    if (nested_vmx_check_restored_vmcs12(vcpu) < 0)
        return 1;
    ...
}

Could this zeroed buffer fail the revision ID check and induce a Triple Fault?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908132838.2116068-1-jmattson@google.com?part=1

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

[James Houghton]

On Tue, Sep 8, 2026 at 6:54 AM Jim Mattson <jmattson@google.com> wrote:
>
> 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, ...)
>
> 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 ghc->hva, __copy_to_user() corrupts an unrelated task's memory.
>
> Skip the flush when KVM synthesizes a VM-exit with vm_exit_reason == -1
> (e.g. during vCPU teardown or SMM entry). In these paths, KVM forces the
> vCPU out of guest mode internally--no architectural VM-exit is delivered
> to L1.
>
> Fixes: 61ada7488ffd ("KVM: nVMX: Cache shadow vmcs12 on VMEntry and flush to memory on VMExit")
> Cc: stable@vger.kernel.org
> Signed-off-by: Jim Mattson <jmattson@google.com>

Hi Jim,

This patch looks good for a stable backport, but I feel this kind of
bug could easily happen again with the current API.

One simple thing that would have prevented this would be to:
1. Change kvm_write_guest_cached() (and related) to take the vcpu
instead of the kvm, and
2. Check that vcpu->mm == current->mm before doing the uaccess.

This wouldn't be as suitable for a backport, but I think it's better
for preventing similar classes of bugs. I'm sure you (and
Paolo/Sean/others) will have better ideas. What do you think?

Thank you very much for debugging this. :)

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

[Jim Mattson]

On Tue, Sep 8, 2026 at 10:35 AM James Houghton <jthoughton@google.com> wrote:
>
> On Tue, Sep 8, 2026 at 6:54 AM Jim Mattson <jmattson@google.com> wrote:
> >
> > 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, ...)
> >
> > 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 ghc->hva, __copy_to_user() corrupts an unrelated task's memory.
> >
> > Skip the flush when KVM synthesizes a VM-exit with vm_exit_reason == -1
> > (e.g. during vCPU teardown or SMM entry). In these paths, KVM forces the
> > vCPU out of guest mode internally--no architectural VM-exit is delivered
> > to L1.
> >
> > Fixes: 61ada7488ffd ("KVM: nVMX: Cache shadow vmcs12 on VMEntry and flush to memory on VMExit")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Jim Mattson <jmattson@google.com>
>
> Hi Jim,
>
> This patch looks good for a stable backport, but I feel this kind of
> bug could easily happen again with the current API.
>
> One simple thing that would have prevented this would be to:
> 1. Change kvm_write_guest_cached() (and related) to take the vcpu
> instead of the kvm, and
> 2. Check that vcpu->mm == current->mm before doing the uaccess.
>
> This wouldn't be as suitable for a backport, but I think it's better
> for preventing similar classes of bugs. I'm sure you (and
> Paolo/Sean/others) will have better ideas. What do you think?

There seems to be some awareness of this issue within KVM, so that may
not be necessary. Also, as Sashiko points out, we have issues with
reads as well as writes. I don't know how many choke points we'd have
to test.

AFAICT (with AI assistance), all of the current problems are rooted in
the following call to nested_vmx_vmexit():

void vmx_leave_nested(struct kvm_vcpu *vcpu)
{
        if (is_guest_mode(vcpu)) {
                vcpu->arch.nested_run_pending = 0;
                nested_vmx_vmexit(vcpu, -1, 0, 0);
        }
        free_nested(vcpu);
}

On the SVM side, there is no comparable faux VM-exit. Instead of
calling nested_svm_vmexit(), the teardown of SVM nested state is an
open-coded sequence. Maybe something like that would work better for
VMX?

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

[James Houghton]

On Tue, Sep 8, 2026 at 12:13 PM Jim Mattson <jmattson@google.com> wrote:
>
> On Tue, Sep 8, 2026 at 10:35 AM James Houghton <jthoughton@google.com> wrote:
> > Hi Jim,
> >
> > This patch looks good for a stable backport, but I feel this kind of
> > bug could easily happen again with the current API.
> >
> > One simple thing that would have prevented this would be to:
> > 1. Change kvm_write_guest_cached() (and related) to take the vcpu
> > instead of the kvm, and
> > 2. Check that vcpu->mm == current->mm before doing the uaccess.
> >
> > This wouldn't be as suitable for a backport, but I think it's better
> > for preventing similar classes of bugs. I'm sure you (and
> > Paolo/Sean/others) will have better ideas. What do you think?
>
> There seems to be some awareness of this issue within KVM, so that may
> not be necessary. Also, as Sashiko points out, we have issues with
> reads as well as writes. I don't know how many choke points we'd have
> to test.
>
> AFAICT (with AI assistance), all of the current problems are rooted in
> the following call to nested_vmx_vmexit():
>
> void vmx_leave_nested(struct kvm_vcpu *vcpu)
> {
>         if (is_guest_mode(vcpu)) {
>                 vcpu->arch.nested_run_pending = 0;
>                 nested_vmx_vmexit(vcpu, -1, 0, 0);
>         }
>         free_nested(vcpu);
> }
>
> On the SVM side, there is no comparable faux VM-exit. Instead of
> calling nested_svm_vmexit(), the teardown of SVM nested state is an
> open-coded sequence. Maybe something like that would work better for
> VMX?

Maybe, but I don't think I'd feel any better about it (because the
change is still KVM/x86-specific).

Thinking harder about this, I think it is always invalid to call
`copy_to_user()` if `current->mm` is NULL. The riscv implementation[1]
seems to require `current->mm` to be non-NULL. So I'd be much happier
with a check in `__access_ok()` that `current->mm != NULL`, but I'm
not 100% sure that is okay.

I'm also trying to think a little bit harder about the exact mechanism
that causes the memory corruption. I think for `active_mm` (and
therefore the loaded page tables/CR3) to be different than the mm we
destroyed in `exit_mm()`, there must have been a context switch to the
victim user process and then back to the dying process after
`exit_mm()`. Right?

[1] See arch/riscv/include/asm/uaccess.h

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

[Sean Christopherson]

On Tue, Sep 08, 2026, Jim Mattson wrote:
> On Tue, Sep 8, 2026 at 10:35 AM James Houghton <jthoughton@google.com> wrote:
> >
> > On Tue, Sep 8, 2026 at 6:54 AM Jim Mattson <jmattson@google.com> wrote:
> > >
> > > 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, ...)
> > >
> > > 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 ghc->hva, __copy_to_user() corrupts an unrelated task's memory.
> > >
> > > Skip the flush when KVM synthesizes a VM-exit with vm_exit_reason == -1
> > > (e.g. during vCPU teardown or SMM entry). In these paths, KVM forces the
> > > vCPU out of guest mode internally--no architectural VM-exit is delivered
> > > to L1.
> > >
> > > Fixes: 61ada7488ffd ("KVM: nVMX: Cache shadow vmcs12 on VMEntry and flush to memory on VMExit")
> > > Cc: stable@vger.kernel.org
> > > Signed-off-by: Jim Mattson <jmattson@google.com>
> >
> > Hi Jim,
> >
> > This patch looks good for a stable backport, but I feel this kind of
> > bug could easily happen again with the current API.
> >
> > One simple thing that would have prevented this would be to:
> > 1. Change kvm_write_guest_cached() (and related) to take the vcpu
> > instead of the kvm, and
> > 2. Check that vcpu->mm == current->mm before doing the uaccess.

The mm pointer is store in "kvm", not in "kvm_vcpu", i.e. this sort of hardening
shouldn't require modifying the callsites.

> > This wouldn't be as suitable for a backport, but I think it's better
> > for preventing similar classes of bugs. I'm sure you (and
> > Paolo/Sean/others) will have better ideas. What do you think?
> 
> There seems to be some awareness of this issue within KVM, so that may
> not be necessary. Also, as Sashiko points out, we have issues with
> reads as well as writes. I don't know how many choke points we'd have
> to test.
> 
> AFAICT (with AI assistance), all of the current problems are rooted in
> the following call to nested_vmx_vmexit():
> 
> void vmx_leave_nested(struct kvm_vcpu *vcpu)
> {
>         if (is_guest_mode(vcpu)) {
>                 vcpu->arch.nested_run_pending = 0;
>                 nested_vmx_vmexit(vcpu, -1, 0, 0);
>         }
>         free_nested(vcpu);
> }
> 
> On the SVM side, there is no comparable faux VM-exit. Instead of
> calling nested_svm_vmexit(), the teardown of SVM nested state is an
> open-coded sequence. Maybe something like that would work better for
> VMX?

Yes, vmx_leave_nested()'s use of nested_vmx_vmexit() is an endless source of pain
and needs to be rewritten.

However, the bigger flaw is that the memslots are still valid when the VM is
being destroyed.  The hardening James suggested above really should be hardening,
not the primary mechanism for ensuring correctness.

Manually freeing each memslot one-by-one isn't a great option, because the latency
introduced by each synchronize_srcu_expedited() call would be rather absurd.  But
once KVM unregisters its mmu_notifier, i.e. once kvm_mmu_notifier_release() and
thus kvm_flush_shadow_all() runs, all indirect references to memslots need to be
gone.  And by "indirect references" I mean code in KVM that relies on a memslot
existing and being reachable, e.g. x86's rmaps and shadow page accounting.

KVM is infuriatingly close to enforcing that already, as only kvm_arch_destroy_vm()
and kvm_destroy_devices() run between unregistering the mmu_notifier and freeing
all memslot metadata.

	mmu_notifier_unregister(&kvm->mmu_notifier, kvm->mm);
	/*
	 * At this point, pending calls to invalidate_range_start()
	 * have completed but no more MMU notifiers will run, so
	 * mn_active_invalidate_count may remain unbalanced.
	 * No threads can be waiting in kvm_swap_active_memslots() as the
	 * last reference on KVM has been dropped, but freeing
	 * memslots would deadlock without this manual intervention.
	 *
	 * If the count isn't unbalanced, i.e. KVM did NOT unregister its MMU
	 * notifier between a start() and end(), then there shouldn't be any
	 * in-progress invalidations.
	 */
	WARN_ON(rcuwait_active(&kvm->mn_memslots_update_rcuwait));
	if (kvm->mn_active_invalidate_count)
		kvm->mn_active_invalidate_count = 0;
	else
		WARN_ON(kvm->mmu_invalidate_in_progress);
	kvm_arch_destroy_vm(kvm);
	kvm_destroy_devices(kvm);
	for (i = 0; i < kvm_arch_nr_memslot_as_ids(kvm); i++) {
		kvm_free_memslots(kvm, &kvm->__memslots[i][0]);
		kvm_free_memslots(kvm, &kvm->__memslots[i][1]);  <=== KVM will be very sad after this

	}

Destroying memslots before kvm_destroy_devices() is a-ok, all implementations do
nothing more than kvm_io_bus_unregister_dev() (a nop at this point in the VM's
lifecycle, as buses are already destroyed), and free of memory.

Unfortunately, kvm_arch_destroy_vm() is practically infeasible to audit.  But, we
don't need to audit that code, we just need to audit kvm_arch_flush_shadow_memslot(),
because if there are memslot references that are dropped by flush_shadow_memslot()
but not flush_shadow_all(), then KVM is already buggy and vulnerable.

*sigh*

And of course PPC is a disaster and does literally nothing on flush_shadow_all().
Doubly hilarious, the worst offender seems to be kvmhv_release_all_nested().  As
a quick and dirty fix, I think we can simply iterate over memslots and manually
flush each one?

Which, amazingly, appears to be safe even though kvmppc_uvmem_drop_pages() takes
mmap_lock, as exit_mmap()'s call to mmu_notifier_release() is (thanfully) super
obviously done without hold mmap_lock.

	/* mm's last user has gone, and its about to be pulled down */
	mmu_notifier_release(mm);

	mmap_read_lock(mm);

diff --git arch/powerpc/include/asm/kvm_host.h arch/powerpc/include/asm/kvm_host.h
index 2d139c807577..1c8d9d7360e9 100644
--- arch/powerpc/include/asm/kvm_host.h
+++ arch/powerpc/include/asm/kvm_host.h
@@ -903,7 +903,6 @@ struct kvm_vcpu_arch {
 #define __KVM_HAVE_CREATE_DEVICE
 
 static inline void kvm_arch_memslots_updated(struct kvm *kvm, u64 gen) {}
-static inline void kvm_arch_flush_shadow_all(struct kvm *kvm) {}
 static inline void kvm_arch_vcpu_blocking(struct kvm_vcpu *vcpu) {}
 static inline void kvm_arch_vcpu_unblocking(struct kvm_vcpu *vcpu) {}
 
diff --git arch/powerpc/kvm/powerpc.c arch/powerpc/kvm/powerpc.c
index 9194cf492d1c..2b88fb9e9bc4 100644
--- arch/powerpc/kvm/powerpc.c
+++ arch/powerpc/kvm/powerpc.c
@@ -764,6 +764,17 @@ void kvm_arch_commit_memory_region(struct kvm *kvm,
 	kvmppc_core_commit_memory_region(kvm, old, new, change);
 }
 
+void kvm_arch_flush_shadow_all(struct kvm *kvm)
+{
+	struct kvm_memory_slot *memslot;
+	struct kvm_memslots *slots;
+	int bkt;
+
+	slots = kvm_memslots(kvm);
+	kvm_for_each_memslot(memslot, bkt, slots)
+		kvmppc_core_flush_memslot(kvm, slot);
+}
+
 void kvm_arch_flush_shadow_memslot(struct kvm *kvm,
 				   struct kvm_memory_slot *slot)
 {

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.

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

The diff for #3 and #4 (lightly tested on x86):

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 4b3681796c75..f05ba2d1364d 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -9925,7 +9925,7 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
  * @size > 0 to install a new slot, while @size == 0 to uninstall a
  * slot.  The return code can be one of the following:
  *
- *   HVA:           on success (uninstall will return a bogus HVA)
+ *   HVA:           on success (uninstall will return a NULL HVA)
  *   -errno:        on error
  *
  * The caller should always use IS_ERR() to check the return value
@@ -9938,10 +9938,10 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
 void __user * __x86_set_memory_region(struct kvm *kvm, int id, gpa_t gpa,
 				      u32 size)
 {
-	int i, r;
-	unsigned long hva, old_npages;
 	struct kvm_memslots *slots = kvm_memslots(kvm);
 	struct kvm_memory_slot *slot;
+	unsigned long hva;
+	int i, r;
 
 	lockdep_assert_held(&kvm->slots_lock);
 
@@ -9965,8 +9965,7 @@ void __user * __x86_set_memory_region(struct kvm *kvm, int id, gpa_t gpa,
 		if (!slot || !slot->npages)
 			return NULL;
 
-		old_npages = slot->npages;
-		hva = slot->userspace_addr;
+		hva = 0;
 	}
 
 	for (i = 0; i < kvm_arch_nr_memslot_as_ids(kvm); i++) {
@@ -9982,9 +9981,6 @@ void __user * __x86_set_memory_region(struct kvm *kvm, int id, gpa_t gpa,
 			return ERR_PTR_USR(r);
 	}
 
-	if (!size)
-		vm_munmap(hva, old_npages * PAGE_SIZE);
-
 	return (void __user *)hva;
 }
 EXPORT_SYMBOL_FOR_KVM_INTERNAL(__x86_set_memory_region);
@@ -10013,20 +10009,6 @@ void kvm_arch_pre_destroy_vm(struct kvm *kvm)
 
 void kvm_arch_destroy_vm(struct kvm *kvm)
 {
-	if (current->mm == kvm->mm) {
-		/*
-		 * Free memory regions allocated on behalf of userspace,
-		 * unless the memory map has changed due to process exit
-		 * or fd copying.
-		 */
-		mutex_lock(&kvm->slots_lock);
-		__x86_set_memory_region(kvm, APIC_ACCESS_PAGE_PRIVATE_MEMSLOT,
-					0, 0);
-		__x86_set_memory_region(kvm, IDENTITY_PAGETABLE_PRIVATE_MEMSLOT,
-					0, 0);
-		__x86_set_memory_region(kvm, TSS_PRIVATE_MEMSLOT, 0, 0);
-		mutex_unlock(&kvm->slots_lock);
-	}
 	if (kvm->arch.created_mediated_pmu)
 		perf_release_mediated_pmu();
 	kvm_destroy_vcpus(kvm);
@@ -10066,6 +10048,16 @@ void kvm_arch_free_memslot(struct kvm *kvm, struct kvm_memory_slot *slot)
 	}
 
 	kvm_page_track_free_memslot(slot);
+
+	/*
+	 * Free memory regions allocated on behalf of userspace, unless the
+	 * memory map has changed due to process exit or fd copying.  Leak the
+	 * mapping on failure, e.g. if the task is killed, worst case scenario,
+	 * the page(s) will be reclaimed when the process exits.
+	 */
+	if (current->mm == kvm->mm && slot->id >= KVM_USER_MEM_SLOTS &&
+	    !WARN_ON_ONCE(!slot->npages))
+		vm_munmap(slot->userspace_addr, slot->npages * PAGE_SIZE);
 }
 
 int memslot_rmap_alloc(struct kvm_memory_slot *slot, unsigned long npages)
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 03bfc92864b6..bed48f70b2d8 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -1693,6 +1693,7 @@ int kvm_arch_vcpu_should_kick(struct kvm_vcpu *vcpu);
 bool kvm_arch_dy_runnable(struct kvm_vcpu *vcpu);
 bool kvm_arch_dy_has_pending_interrupt(struct kvm_vcpu *vcpu);
 bool kvm_arch_vcpu_preempted_in_kernel(struct kvm_vcpu *vcpu);
+void kvm_arch_destroy_memslots(struct kvm *kvm);
 void kvm_arch_pre_destroy_vm(struct kvm *kvm);
 void kvm_arch_create_vm_debugfs(struct kvm *kvm);
 
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 65eb26a0520d..899bf5970434 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -945,23 +945,36 @@ static void kvm_free_memslot(struct kvm *kvm, struct kvm_memory_slot *slot)
 	kfree(slot);
 }
 
-static void kvm_free_memslots(struct kvm *kvm, struct kvm_memslots *slots)
+static const struct kvm_memslots kvm_empty_memslots = {
+	.generation = -1ull,
+	.hva_tree = RB_ROOT_CACHED,
+	.gfn_tree = RB_ROOT,
+	.id_hash[0 ... (ARRAY_SIZE(kvm_empty_memslots.id_hash) - 1)] = HLIST_HEAD_INIT,
+	.node_idx = 0,
+};
+
+static void kvm_destroy_memslots(struct kvm *kvm)
 {
 	struct hlist_node *idnode;
 	struct kvm_memory_slot *memslot;
-	int bkt;
+	int bkt, i;
+
+	mutex_lock(&kvm->slots_lock);
+	for (i = 0; i < kvm_arch_nr_memslot_as_ids(kvm); i++)
+		rcu_assign_pointer(kvm->memslots[i], &kvm_empty_memslots);
+
+	synchronize_srcu_expedited(&kvm->srcu);
+	mutex_unlock(&kvm->slots_lock);
 
 	/*
 	 * The same memslot objects live in both active and inactive sets,
-	 * arbitrarily free using index '1' so the second invocation of this
-	 * function isn't operating over a structure with dangling pointers
-	 * (even though this function isn't actually touching them).
+	 * arbitrarily free using index '1'.
 	 */
-	if (!slots->node_idx)
-		return;
-
-	hash_for_each_safe(slots->id_hash, bkt, idnode, memslot, id_node[1])
-		kvm_free_memslot(kvm, memslot);
+	for (i = 0; i < kvm_arch_nr_memslot_as_ids(kvm); i++) {
+		hash_for_each_safe(kvm->__memslots[i][1].id_hash, bkt, idnode,
+				   memslot, id_node[1])
+			kvm_free_memslot(kvm, memslot);
+	}
 }
 
 static umode_t kvm_stats_debugfs_mode(const struct kvm_stats_desc *desc)
@@ -1292,12 +1305,10 @@ static void kvm_destroy_vm(struct kvm *kvm)
 		kvm->mn_active_invalidate_count = 0;
 	else
 		WARN_ON(kvm->mmu_invalidate_in_progress);
+	kvm_destroy_memslots(kvm);
+
 	kvm_arch_destroy_vm(kvm);
 	kvm_destroy_devices(kvm);
-	for (i = 0; i < kvm_arch_nr_memslot_as_ids(kvm); i++) {
-		kvm_free_memslots(kvm, &kvm->__memslots[i][0]);
-		kvm_free_memslots(kvm, &kvm->__memslots[i][1]);
-	}
 	cleanup_srcu_struct(&kvm->irq_srcu);
 	srcu_barrier(&kvm->srcu);
 	cleanup_srcu_struct(&kvm->srcu);
@@ -2005,6 +2016,9 @@ static int kvm_set_memory_region(struct kvm *kvm,
 
 	lockdep_assert_held(&kvm->slots_lock);
 
+	if (WARN_ON_ONCE(!refcount_read(&kvm->users_count)))
+		return -EIO;
+
 	r = check_memory_region_flags(kvm, mem);
 	if (r)
 		return r;

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

[James Houghton]

On Wed, Sep 9, 2026 at 12:00 PM Sean Christopherson <seanjc@google.com> wrote:
>
> On Tue, Sep 08, 2026, Jim Mattson wrote:
> > On Tue, Sep 8, 2026 at 10:35 AM James Houghton <jthoughton@google.com> wrote:
> > >
> > > On Tue, Sep 8, 2026 at 6:54 AM Jim Mattson <jmattson@google.com> wrote:
> > > >
> > > > 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, ...)
> > > >
> > > > 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 ghc->hva, __copy_to_user() corrupts an unrelated task's memory.
> > > >
> > > > Skip the flush when KVM synthesizes a VM-exit with vm_exit_reason == -1
> > > > (e.g. during vCPU teardown or SMM entry). In these paths, KVM forces the
> > > > vCPU out of guest mode internally--no architectural VM-exit is delivered
> > > > to L1.
> > > >
> > > > Fixes: 61ada7488ffd ("KVM: nVMX: Cache shadow vmcs12 on VMEntry and flush to memory on VMExit")
> > > > Cc: stable@vger.kernel.org
> > > > Signed-off-by: Jim Mattson <jmattson@google.com>
> > >
> > > Hi Jim,
> > >
> > > This patch looks good for a stable backport, but I feel this kind of
> > > bug could easily happen again with the current API.
> > >
> > > One simple thing that would have prevented this would be to:
> > > 1. Change kvm_write_guest_cached() (and related) to take the vcpu
> > > instead of the kvm, and
> > > 2. Check that vcpu->mm == current->mm before doing the uaccess.
>
> The mm pointer is store in "kvm", not in "kvm_vcpu", i.e. this sort of hardening
> shouldn't require modifying the callsites.

Yeahhh, I noticed this a few minutes after I sent that email, heh. :(

> > > This wouldn't be as suitable for a backport, but I think it's better
> > > for preventing similar classes of bugs. I'm sure you (and
> > > Paolo/Sean/others) will have better ideas. What do you think?
> >
> > There seems to be some awareness of this issue within KVM, so that may
> > not be necessary. Also, as Sashiko points out, we have issues with
> > reads as well as writes. I don't know how many choke points we'd have
> > to test.
> >
> > AFAICT (with AI assistance), all of the current problems are rooted in
> > the following call to nested_vmx_vmexit():
> >
> > void vmx_leave_nested(struct kvm_vcpu *vcpu)
> > {
> >         if (is_guest_mode(vcpu)) {
> >                 vcpu->arch.nested_run_pending = 0;
> >                 nested_vmx_vmexit(vcpu, -1, 0, 0);
> >         }
> >         free_nested(vcpu);
> > }
> >
> > On the SVM side, there is no comparable faux VM-exit. Instead of
> > calling nested_svm_vmexit(), the teardown of SVM nested state is an
> > open-coded sequence. Maybe something like that would work better for
> > VMX?
>
> Yes, vmx_leave_nested()'s use of nested_vmx_vmexit() is an endless source of pain
> and needs to be rewritten.
>
> However, the bigger flaw is that the memslots are still valid when the VM is
> being destroyed.  The hardening James suggested above really should be hardening,
> not the primary mechanism for ensuring correctness.
>
> Manually freeing each memslot one-by-one isn't a great option, because the latency
> introduced by each synchronize_srcu_expedited() call would be rather absurd.  But
> once KVM unregisters its mmu_notifier, i.e. once kvm_mmu_notifier_release() and
> thus kvm_flush_shadow_all() runs, all indirect references to memslots need to be
> gone.  And by "indirect references" I mean code in KVM that relies on a memslot
> existing and being reachable, e.g. x86's rmaps and shadow page accounting.
>
> KVM is infuriatingly close to enforcing that already, as only kvm_arch_destroy_vm()
> and kvm_destroy_devices() run between unregistering the mmu_notifier and freeing
> all memslot metadata.
>
>         mmu_notifier_unregister(&kvm->mmu_notifier, kvm->mm);
>         /*
>          * At this point, pending calls to invalidate_range_start()
>          * have completed but no more MMU notifiers will run, so
>          * mn_active_invalidate_count may remain unbalanced.
>          * No threads can be waiting in kvm_swap_active_memslots() as the
>          * last reference on KVM has been dropped, but freeing
>          * memslots would deadlock without this manual intervention.
>          *
>          * If the count isn't unbalanced, i.e. KVM did NOT unregister its MMU
>          * notifier between a start() and end(), then there shouldn't be any
>          * in-progress invalidations.
>          */
>         WARN_ON(rcuwait_active(&kvm->mn_memslots_update_rcuwait));
>         if (kvm->mn_active_invalidate_count)
>                 kvm->mn_active_invalidate_count = 0;
>         else
>                 WARN_ON(kvm->mmu_invalidate_in_progress);
>         kvm_arch_destroy_vm(kvm);
>         kvm_destroy_devices(kvm);
>         for (i = 0; i < kvm_arch_nr_memslot_as_ids(kvm); i++) {
>                 kvm_free_memslots(kvm, &kvm->__memslots[i][0]);
>                 kvm_free_memslots(kvm, &kvm->__memslots[i][1]);  <=== KVM will be very sad after this
>
>         }
>
> Destroying memslots before kvm_destroy_devices() is a-ok, all implementations do
> nothing more than kvm_io_bus_unregister_dev() (a nop at this point in the VM's
> lifecycle, as buses are already destroyed), and free of memory.
>
> Unfortunately, kvm_arch_destroy_vm() is practically infeasible to audit.  But, we
> don't need to audit that code, we just need to audit kvm_arch_flush_shadow_memslot(),
> because if there are memslot references that are dropped by flush_shadow_memslot()
> but not flush_shadow_all(), then KVM is already buggy and vulnerable.
>
> *sigh*
>
> And of course PPC is a disaster and does literally nothing on flush_shadow_all().
> Doubly hilarious, the worst offender seems to be kvmhv_release_all_nested().  As
> a quick and dirty fix, I think we can simply iterate over memslots and manually
> flush each one?
>
> Which, amazingly, appears to be safe even though kvmppc_uvmem_drop_pages() takes
> mmap_lock, as exit_mmap()'s call to mmu_notifier_release() is (thanfully) super
> obviously done without hold mmap_lock.
>
>         /* mm's last user has gone, and its about to be pulled down */
>         mmu_notifier_release(mm);
>
>         mmap_read_lock(mm);
>
> diff --git arch/powerpc/include/asm/kvm_host.h arch/powerpc/include/asm/kvm_host.h
> index 2d139c807577..1c8d9d7360e9 100644
> --- arch/powerpc/include/asm/kvm_host.h
> +++ arch/powerpc/include/asm/kvm_host.h
> @@ -903,7 +903,6 @@ struct kvm_vcpu_arch {
>  #define __KVM_HAVE_CREATE_DEVICE
>
>  static inline void kvm_arch_memslots_updated(struct kvm *kvm, u64 gen) {}
> -static inline void kvm_arch_flush_shadow_all(struct kvm *kvm) {}
>  static inline void kvm_arch_vcpu_blocking(struct kvm_vcpu *vcpu) {}
>  static inline void kvm_arch_vcpu_unblocking(struct kvm_vcpu *vcpu) {}
>
> diff --git arch/powerpc/kvm/powerpc.c arch/powerpc/kvm/powerpc.c
> index 9194cf492d1c..2b88fb9e9bc4 100644
> --- arch/powerpc/kvm/powerpc.c
> +++ arch/powerpc/kvm/powerpc.c
> @@ -764,6 +764,17 @@ void kvm_arch_commit_memory_region(struct kvm *kvm,
>         kvmppc_core_commit_memory_region(kvm, old, new, change);
>  }
>
> +void kvm_arch_flush_shadow_all(struct kvm *kvm)
> +{
> +       struct kvm_memory_slot *memslot;
> +       struct kvm_memslots *slots;
> +       int bkt;
> +
> +       slots = kvm_memslots(kvm);
> +       kvm_for_each_memslot(memslot, bkt, slots)
> +               kvmppc_core_flush_memslot(kvm, slot);
> +}
> +
>  void kvm_arch_flush_shadow_memslot(struct kvm *kvm,
>                                    struct kvm_memory_slot *slot)
>  {
>
> 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.

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

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

[Sean Christopherson]

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.

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

[Sean Christopherson]

On Thu, Sep 10, 2026, Sean Christopherson wrote:
> On Thu, Sep 10, 2026, James Houghton wrote:
> > On Wed, Sep 9, 2026 at 12:00 PM Sean Christopherson <seanjc@google.com> wrote:
> > 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.

Oh, and I'm not saying we shouldn't try to harden the uaccess APIs to guard against
a NULL current->mm.  That absolutely would be worthwhile.  I'm just saying that for
KVM, it sadly isn't sufficient.

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

[Sean Christopherson]

On Thu, Sep 10, 2026, Sean Christopherson wrote:
> 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).

To clarify: my goal isn't to harden literally every uaccess in KVM, just those
that have a dependency on memslots, i.e. are accessing guest memory.  Those also
happen to be the ones that are often buried deep in KVM, in widely-used APIs.