* [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