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: <aO1-IV-R6XX7RIlv@google.com>
Date: Mon, 13 Oct 2025 15:33:05 -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 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);
        }


> Don't set GIF when emulating a change to EFER that clears EFER.SVME.
> 
> Fixes: c513f484c558 ("KVM: nSVM: leave guest mode when clearing EFER.SVME")
> Reviewed-by: Yosry Ahmed <yosry.ahmed@...ux.dev>
> Signed-off-by: Jim Mattson <jmattson@...gle.com>
> ---
>  arch/x86/kvm/svm/svm.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index 153c12dbf3eb..96177758d778 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -215,7 +215,6 @@ int svm_set_efer(struct kvm_vcpu *vcpu, u64 efer)
>  	if ((old_efer & EFER_SVME) != (efer & EFER_SVME)) {
>  		if (!(efer & EFER_SVME)) {
>  			svm_leave_nested(vcpu);
> -			svm_set_gif(svm, true);
>  			/* #GP intercept is still needed for vmware backdoor */
>  			if (!enable_vmware_backdoor)
>  				clr_exception_intercept(svm, GP_VECTOR);
> -- 
> 2.51.0.740.g6adb054d12-goog
>