Re: [PATCH 2/4] KVM: nSVM: Synthesize #GP instead of SHUTDOWN if L2 attempts to clear EFER.SVME

[Sean Christopherson <seanjc@google.com>]

On Thu, Oct 22, 2026, sashiko-bot@kernel.org wrote:
> > diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> > index 487897ef11dd6..a6ff9635677a1 100644
> > --- a/arch/x86/kvm/svm/svm.c
> > +++ b/arch/x86/kvm/svm/svm.c
> > @@ -227,17 +227,12 @@ int svm_set_efer(struct kvm_vcpu *vcpu, u64 efer)
> >  	if ((old_efer & EFER_SVME) != (efer & EFER_SVME)) {
> >  		if (!(efer & EFER_SVME)) {
> >  			/*
> > -			 * Architecturally, clearing EFER.SVME while a guest is
> > -			 * running yields undefined behavior, i.e. KVM can do
> > -			 * literally anything.  Force the vCPU back into L1 as
> > -			 * that is the safest option for KVM, but synthesize a
> > -			 * triple fault (for L1!) so that KVM at least doesn't
> > -			 * run random L2 code in the context of L1.  Do so if
> > -			 * and only if the vCPU is actively running, e.g. to
> > -			 * avoid positives if userspace is stuffing state.
> > +			 * KVM is supposed to synthesize #GP if L2 attempts to
> > +			 * clear SVME.
> >  			 */
> > -			if (is_guest_mode(vcpu) && vcpu->wants_to_run)
> > -				kvm_make_request(KVM_REQ_TRIPLE_FAULT, vcpu);
> > +			if (KVM_BUG_ON(is_guest_mode(vcpu) &&
> > +				       vcpu->wants_to_run, vcpu->kvm))
> > +				return -EIO;
> >  
> >  			svm_leave_nested(vcpu);
> >  			/* #GP intercept is still needed for vmware backdoor */
> 
> [Severity: High]
> Can userspace still reach this KVM_BUG_ON() during KVM_RUN via
> KVM_SYNC_X86_SREGS when a vCPU is in guest mode?
> 
> In virt/kvm/kvm_main.c:kvm_vcpu_ioctl(), vcpu->wants_to_run is set before
> calling kvm_arch_vcpu_ioctl_run():
> 
> 	vcpu->wants_to_run = !READ_ONCE(vcpu->run->immediate_exit__unsafe);
> 	r = kvm_arch_vcpu_ioctl_run(vcpu);
> 	vcpu->wants_to_run = false;
> 
> Then arch/x86/kvm/x86.c:kvm_arch_vcpu_ioctl_run() syncs dirty registers
> from userspace while vcpu->wants_to_run is already true:
> 
> 	if (kvm_run->kvm_dirty_regs) {
> 		r = sync_regs(vcpu);
> 		if (r != 0)
> 			goto out;
> 	}
> 
> When KVM_SYNC_X86_SREGS is set in kvm_dirty_regs, the call chain
> sync_regs() -> kvm_run_sync_regs_from_user() -> __set_sregs() ->
> __set_sregs_common() in arch/x86/kvm/regs.c bypasses set_efer() in
> arch/x86/kvm/msrs.c and calls svm_set_efer() directly:
> 
> 	*mmu_reset_needed |= vcpu->arch.efer != sregs->efer;
> 	kvm_x86_call(set_efer)(vcpu, sregs->efer);
> 
> If userspace clears EFER_SVME in vcpu->run->s.regs.sregs.efer while
> is_guest_mode(vcpu) is true, wouldn't svm_set_efer() trigger the
> KVM_BUG_ON() and return -EIO (which __set_sregs_common() ignores) after
> already updating vcpu->arch.efer = efer and without calling
> svm_leave_nested(vcpu)?

Oof.  I think the right answer is to straight up disallow that?

diff --git a/arch/x86/kvm/regs.c b/arch/x86/kvm/regs.c
index 3621f1a47d37..e53496fdeb82 100644
--- a/arch/x86/kvm/regs.c
+++ b/arch/x86/kvm/regs.c
@@ -738,6 +738,10 @@ int kvm_run_sync_regs_from_user(struct kvm_vcpu *vcpu)
 	if (vcpu->run->kvm_dirty_regs & KVM_SYNC_X86_SREGS) {
 		struct kvm_sregs sregs = vcpu->run->s.regs.sregs;
 
+		if (is_guest_mode(vcpu) &&
+		    (vcpu->arch.efer & EFER_SVME) && !(sregs.efer & EFER_SVME))
+			return -EINVAL;
+
 		if (__set_sregs(vcpu, &sregs))
 			return -EINVAL;

Ideally, kvm_is_valid_sregs() would disallow that, but the risk of breaking
userspace, e.g. on save/restore, is too high.  Whereas I really, really hope no
one is using KVM_SYNC_X86_SREGS to yank the vCPU out of L2 during save/restore.