linux-kernel - Re: [PATCH V2 2/5] KVM: SVM: Inject #UD for STGI if EFER.SVME=0 and SVM Lock and DEV are not available

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: <23l6bxew3cbnsgl6zagqz6qqtkb7mwfeqcl4opp2ulk3cu5fmq@hqfnuptmaiti>
Date: Mon, 12 Jan 2026 20:50:03 +0000
From: Yosry Ahmed <yosry.ahmed@...ux.dev>
To: Kevin Cheng <chengkev@...gle.com>
Cc: seanjc@...gle.com, pbonzini@...hat.com, kvm@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH V2 2/5] KVM: SVM: Inject #UD for STGI if EFER.SVME=0 and
 SVM Lock and DEV are not available

On Mon, Jan 12, 2026 at 05:45:32PM +0000, Kevin Cheng wrote:
> The AMD APM states that STGI causes a #UD if SVM is not enabled and
> neither SVM Lock nor the device exclusion vector (DEV) are supported.

Might be useful to also mention the following part "Support for DEV is
part of the SKINIT architecture".

> Fix the STGI exit handler by injecting #UD when these conditions are
> met.
> 
> Signed-off-by: Kevin Cheng <chengkev@...gle.com>
> ---
>  arch/x86/kvm/svm/svm.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index 6373a25d85479..557c84a060fc6 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -2271,8 +2271,18 @@ static int stgi_interception(struct kvm_vcpu *vcpu)
>  {
>  	int ret;
>  
> -	if (nested_svm_check_permissions(vcpu))
> +	if ((!(vcpu->arch.efer & EFER_SVME) &&
> +	     !guest_cpu_cap_has(vcpu, X86_FEATURE_SVML) &&
> +	     !guest_cpu_cap_has(vcpu, X86_FEATURE_SKINIT)) ||
> +	    !is_paging(vcpu)) {
> +		kvm_queue_exception(vcpu, UD_VECTOR);
> +		return 1;
> +	}
> +
> +	if (to_svm(vcpu)->vmcb->save.cpl) {
> +		kvm_inject_gp(vcpu, 0);
>  		return 1;
> +	}

Not a big fan of open-coding nested_svm_check_permissions() here. The
checks could get out of sync.

How about refactoring nested_svm_check_permissions() like so:

diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index f295a41ec659..7f53c54b9d39 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -1520,9 +1520,10 @@ int nested_svm_exit_handled(struct vcpu_svm *svm)
        return vmexit;
 }

-int nested_svm_check_permissions(struct kvm_vcpu *vcpu)
+static int __nested_svm_check_permissions(struct kvm_vcpu *vcpu,
+                                         bool insn_allowed)
 {
-       if (!(vcpu->arch.efer & EFER_SVME) || !is_paging(vcpu)) {
+       if (!insn_allowed || !is_paging(vcpu)) {
                kvm_queue_exception(vcpu, UD_VECTOR);
                return 1;
        }
@@ -1535,6 +1536,11 @@ int nested_svm_check_permissions(struct kvm_vcpu *vcpu)
        return 0;
 }

+int nested_svm_check_permissions(struct kvm_vcpu *vcpu)
+{
+       return __nested_svm_check_permissions(vcpu, vcpu->arch.efer & EFER_SVME);
+}
+
 static bool nested_svm_is_exception_vmexit(struct kvm_vcpu *vcpu, u8 vector,
                                           u32 error_code)
 {
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 7041498a8091..6340c4ce323c 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -2333,9 +2333,19 @@ void svm_set_gif(struct vcpu_svm *svm, bool value)

 static int stgi_interception(struct kvm_vcpu *vcpu)
 {
+       bool insn_allowed;
        int ret;

-       if (nested_svm_check_permissions(vcpu))
+       /*
+        * According to the APM, STGI is allowed even with SVM disabled if SVM
+        * Lock or device exclusion vector (DEV) are supported. DEV is part of
+        * the SKINIT architecture.
+        */
+       insn_allowed = (vcpu->arch.efer & EFER_SVME) ||
+               guest_cpu_cap_has(vcpu, X86_FEATURE_SVML) ||
+               guest_cpu_cap_has(vcpu, X86_FEATURE_SKINIT);
+
+       if (__nested_svm_check_permissions(vcpu, insn_allowed))
                return 1;

        ret = kvm_skip_emulated_instruction(vcpu);

---

We may also want to rename nested_svm_check_permissions() to
nested_svm_insn_check_permissions() or something. Sean, WDYT?

>  
>  	ret = kvm_skip_emulated_instruction(vcpu);
>  	svm_set_gif(to_svm(vcpu), true);
> -- 
> 2.52.0.457.g6b5491de43-goog
>