Kernel KVM virtualization development
 help / color / mirror / Atom feed
* [PATCH] KVM: nSVM: Leave nested if loading CR3 fails at the end of KVM_SET_NESTED_STATE
@ 2026-11-13 17:04 Sean Christopherson
  2026-11-13 17:44 ` Jim Mattson
  2026-11-13 17:44 ` sashiko-bot
  0 siblings, 2 replies; 3+ messages in thread
From: Sean Christopherson @ 2026-11-13 17:04 UTC (permalink / raw)
  To: Sean Christopherson, Paolo Bonzini
  Cc: kvm, linux-kernel, Stefan Teodorescu, Sashiko Bot

Force the vCPU out of L2 if loading CR3 load at the end of setting nested
state fails, otherwise KVM will leave the vCPU in a half-baked state, e.g.
with L1's page walker, and if nested NPT is disabled, L1's PGD as well.

Fixes: e1779c2714c3 ("KVM: x86: nSVM: fix potential NULL derefernce on nested migration")
Cc: stable@vger.kernel.org
Reported-by: Stefan Teodorescu <fane@google.com>
Reported-by: Sashiko Bot <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/all/sashiko-outbox-210535@kernel.org
Closes: https://lore.kernel.org/all/sashiko-outbox-205939@kernel.org
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/svm/nested.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index 07554089ec05..4b1cdc7798bb 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -2114,8 +2114,10 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,
 	 */
 	ret = nested_svm_load_cr3(&svm->vcpu, vcpu->arch.cr3,
 				  nested_npt_enabled(svm), false);
-	if (ret)
+	if (ret) {
+		svm_leave_nested(vcpu);
 		goto out_free;
+	}
 
 	svm->nested.force_msr_bitmap_recalc = true;
 

base-commit: d21eef2084a9af87ac2a783bc7c2b175ba114e4e
-- 
2.56.1.703.gf848ed807c-goog


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] KVM: nSVM: Leave nested if loading CR3 fails at the end of KVM_SET_NESTED_STATE
  2026-11-13 17:04 [PATCH] KVM: nSVM: Leave nested if loading CR3 fails at the end of KVM_SET_NESTED_STATE Sean Christopherson
@ 2026-11-13 17:44 ` Jim Mattson
  2026-11-13 17:44 ` sashiko-bot
  1 sibling, 0 replies; 3+ messages in thread
From: Jim Mattson @ 2026-11-13 17:44 UTC (permalink / raw)
  To: Sean Christopherson
  Cc: Paolo Bonzini, kvm, linux-kernel, Stefan Teodorescu, Sashiko Bot

On Fri, Nov 13, 2026 at 9:34 AM Sean Christopherson <seanjc@google.com> wrote:
>
> Force the vCPU out of L2 if loading CR3 load at the end of setting nested
> state fails, otherwise KVM will leave the vCPU in a half-baked state, e.g.
> with L1's page walker, and if nested NPT is disabled, L1's PGD as well.
>
> Fixes: e1779c2714c3 ("KVM: x86: nSVM: fix potential NULL derefernce on nested migration")
> Cc: stable@vger.kernel.org
> Reported-by: Stefan Teodorescu <fane@google.com>
> Reported-by: Sashiko Bot <sashiko-bot@kernel.org>
> Closes: https://lore.kernel.org/all/sashiko-outbox-210535@kernel.org
> Closes: https://lore.kernel.org/all/sashiko-outbox-205939@kernel.org
> Signed-off-by: Sean Christopherson <seanjc@google.com>
Reviewed-by: Jim Mattson <jmattson@google.com>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] KVM: nSVM: Leave nested if loading CR3 fails at the end of KVM_SET_NESTED_STATE
  2026-11-13 17:04 [PATCH] KVM: nSVM: Leave nested if loading CR3 fails at the end of KVM_SET_NESTED_STATE Sean Christopherson
  2026-11-13 17:44 ` Jim Mattson
@ 2026-11-13 17:44 ` sashiko-bot
  1 sibling, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-11-13 17:44 UTC (permalink / raw)
  To: Sean Christopherson; +Cc: kvm

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Medium] KVM: nSVM: unrestored vmcb01 save state in
  svm_set_nested_state() on error
- [Medium] KVM: nSVM: svm_leave_nested() fails to reinitialize MMU
  context on L2 exit
--

commit 47588b5eb2f39bbca607aca29d4d677254fa60d9
Author: Sean Christopherson <seanjc@google.com>

KVM: nSVM: Leave nested if loading CR3 fails at the end of KVM_SET_NESTED_STATE

This commit calls svm_leave_nested() when nested_svm_load_cr3() fails at
the end of svm_set_nested_state().

> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> index 07554089ec05b..4b1cdc7798bb7 100644
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c
> @@ -2114,8 +2114,10 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,
>  	 */
>  	ret = nested_svm_load_cr3(&svm->vcpu, vcpu->arch.cr3,
>  				  nested_npt_enabled(svm), false);
> -	if (ret)
> +	if (ret) {
> +		svm_leave_nested(vcpu);
>  		goto out_free;
> +	}

[Severity: Medium]
Does calling svm_leave_nested() here leave vmcb01.ptr->save and vCPU
controls corrupted when nested_svm_load_cr3() fails?

By this point in svm_set_nested_state(), svm_copy_vmrun_state() has
already overwritten vmcb01.ptr->save with the raw user-supplied save area
without updating vcpu->arch, and nested_vmcb02_prepare_control() has
already applied L2 TSC scaling, TSC offset, and virtual NMI state.
svm_leave_nested() switches svm->vmcb back to vmcb01 without restoring
vmcb01.ptr->save from vmcb02.ptr->save or undoing the control changes,
leaving vmcb01's CR0/CR3/CR4/EFER out of sync with vcpu->arch and lacking
required shadow-paging bits when NPT is disabled on the host.

Since nested_svm_load_cr3() is called with reload_pdptrs=false, its only
failure condition is !kvm_vcpu_is_legal_cr3(vcpu, vcpu->arch.cr3), which
could be checked during the validation phase before mutating vCPU state.

[Severity: Medium]
Can svm_leave_nested() leave the vCPU's MMU uninitialized when forcing a
vCPU out of L2?

arch/x86/kvm/svm/nested.c:svm_leave_nested() {
    ...
		leave_guest_mode(vcpu);
    ...
		svm_switch_vmcb(svm, &svm->vmcb01);

		nested_svm_uninit_mmu_context(vcpu);
}

nested_svm_uninit_mmu_context() only sets vcpu->arch.mmu back to
&vcpu->arch.root_mmu without calling kvm_init_mmu().  If the vCPU was
already in L2 with nested NPT enabled and userspace updates CPUID (such
as reducing MAXPHYADDR, which also causes nested_svm_load_cr3() to fail),
kvm_mmu_after_set_cpuid() sets root_mmu.root_role.invalid = 1, while
kvm_init_mmu() skips reinitializing root_mmu because mmu_is_nested(vcpu)
is true.

After svm_leave_nested() exits guest mode without calling kvm_init_mmu(),
the vCPU is left in L1 with L2's gva_walk and root_mmu.root_role.invalid
still set.  On the next KVM_RUN with TDP MMU enabled,
kvm_tdp_mmu_alloc_root() allocates a root with role.invalid = 1,
triggering KVM_MMU_WARN_ON(!root || root->role.invalid) in
kvm_tdp_mmu_map() and leaking the invalid root on VM teardown.

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-11-13 17:44 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-13 17:04 [PATCH] KVM: nSVM: Leave nested if loading CR3 fails at the end of KVM_SET_NESTED_STATE Sean Christopherson
2026-11-13 17:44 ` Jim Mattson
2026-11-13 17:44 ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox