linux-kernel - Re: [PATCH v2 2/2] KVM: SVM: Don't set GIF when clearing EFER.SVME

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aO6-CbTRPp1ZNIWq@google.com>
Date: Tue, 14 Oct 2025 14:18:01 -0700
From: Sean Christopherson <seanjc@...gle.com>
To: Jim Mattson <jmattson@...gle.com>
Cc: Yosry Ahmed <yosry.ahmed@...ux.dev>, Paolo Bonzini <pbonzini@...hat.com>, 
	Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>, 
	Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org, 
	"H. Peter Anvin" <hpa@...or.com>, kvm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/2] KVM: SVM: Don't set GIF when clearing EFER.SVME

On Tue, Oct 14, 2025, Jim Mattson wrote:
> On Mon, Oct 13, 2025 at 3:33 PM Sean Christopherson <seanjc@...gle.com> wrote:
> >
> > On Thu, Oct 09, 2025, Jim Mattson wrote:
> > > Clearing EFER.SVME is not architected to set GIF.
> >
> > But it's also not architected to leave GIF set when the guest is running, which
> > was the basic gist of the Fixes commit.  I suspect that forcing GIF=1 was
> > intentional, e.g. so that the guest doesn't end up with GIF=0 after stuffing the
> > vCPU into SMM mode, which might actually be invalid.
> >
> > I think what we actually want is to to set GIF when force-leaving nested.  The
> > only path where it's not obvious that's "safe" is toggling SMM in
> > kvm_vcpu_ioctl_x86_set_vcpu_events().  In every other path, setting GIF is either
> > correct/desirable, or irrelevant because the caller immediately and unconditionally
> > sets/clears GIF.
> >
> > diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> > index a6443feab252..3392c7e22cae 100644
> > --- a/arch/x86/kvm/svm/nested.c
> > +++ b/arch/x86/kvm/svm/nested.c
> > @@ -1367,6 +1367,8 @@ void svm_leave_nested(struct kvm_vcpu *vcpu)
> >                 nested_svm_uninit_mmu_context(vcpu);
> >                 vmcb_mark_all_dirty(svm->vmcb);
> >
> > +               svm_set_gif(svm, true);
> > +
> >                 if (kvm_apicv_activated(vcpu->kvm))
> >                         kvm_make_request(KVM_REQ_APICV_UPDATE, vcpu);
> >         }
> >
> 
> This seems dangerously close to KVM making up "hardware" behavior, but
> I'm okay with that if you are.

Regardless of what KVM does, we're defining hardware behavior, i.e. keeping GIF
unchanged defines behavior just as much as setting GIF.  The only way to truly
avoid defining behavior would be to terminate the VM and completely prevent
userspace from accessing its state.