The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH v7 17/26] KVM: nSVM: Add missing consistency check for EVENTINJ
  2026-03-03  0:33 [PATCH v7 00/26] Nested SVM fixes, cleanups, and hardening Yosry Ahmed
@ 2026-03-03  0:34 ` Yosry Ahmed
  2026-08-03 22:54   ` Abdelkareem Abdelsaamad
  0 siblings, 1 reply; 4+ messages in thread
From: Yosry Ahmed @ 2026-03-03  0:34 UTC (permalink / raw)
  To: Sean Christopherson; +Cc: Paolo Bonzini, kvm, linux-kernel, Yosry Ahmed, stable

According to the APM Volume #2, 15.20 (24593—Rev. 3.42—March 2024):

  VMRUN exits with VMEXIT_INVALID error code if either:
  • Reserved values of TYPE have been specified, or
  • TYPE = 3 (exception) has been specified with a vector that does not
    correspond to an exception (this includes vector 2, which is an NMI,
    not an exception).

Add the missing consistency checks to KVM. For the second point, inject
VMEXIT_INVALID if the vector is anything but the vectors defined by the
APM for exceptions. Reserved vectors are also considered invalid, which
matches the HW behavior. Vector 9 (i.e. #CSO) is considered invalid
because it is reserved on modern CPUs, and according to LLMs no CPUs
exist supporting SVM and producing #CSOs.

Defined exceptions could be different between virtual CPUs as new CPUs
define new vectors. In a best effort to dynamically define the valid
vectors, make all currently defined vectors as valid except those
obviously tied to a CPU feature: SHSTK -> #CP and SEV-ES -> #VC. As new
vectors are defined, they can similarly be tied to corresponding CPU
features.

Invalid vectors on specific (e.g. old) CPUs that are missed by KVM
should be rejected by HW anyway.

Fixes: 3d6368ef580a ("KVM: SVM: Add VMRUN handler")
CC: stable@vger.kernel.org
Signed-off-by: Yosry Ahmed <yosry@kernel.org>
---
 arch/x86/kvm/svm/nested.c | 51 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index 93b3fab9b415d..15f483fac28a0 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -339,6 +339,54 @@ static bool nested_svm_check_bitmap_pa(struct kvm_vcpu *vcpu, u64 pa, u32 size)
 	    kvm_vcpu_is_legal_gpa(vcpu, addr + size - 1);
 }
 
+static bool nested_svm_event_inj_valid_exept(struct kvm_vcpu *vcpu, u8 vector)
+{
+	/*
+	 * Vectors that do not correspond to a defined exception are invalid
+	 * (including #NMI and reserved vectors). In a best effort to define
+	 * valid exceptions based on the virtual CPU, make all exceptions always
+	 * valid except those obviously tied to a CPU feature.
+	 */
+	switch (vector) {
+	case DE_VECTOR: case DB_VECTOR: case BP_VECTOR: case OF_VECTOR:
+	case BR_VECTOR: case UD_VECTOR: case NM_VECTOR: case DF_VECTOR:
+	case TS_VECTOR: case NP_VECTOR: case SS_VECTOR: case GP_VECTOR:
+	case PF_VECTOR: case MF_VECTOR: case AC_VECTOR: case MC_VECTOR:
+	case XM_VECTOR: case HV_VECTOR: case SX_VECTOR:
+		return true;
+	case CP_VECTOR:
+		return guest_cpu_cap_has(vcpu, X86_FEATURE_SHSTK);
+	case VC_VECTOR:
+		return guest_cpu_cap_has(vcpu, X86_FEATURE_SEV_ES);
+	}
+	return false;
+}
+
+/*
+ * According to the APM, VMRUN exits with SVM_EXIT_ERR if SVM_EVTINJ_VALID is
+ * set and:
+ * - The type of event_inj is not one of the defined values.
+ * - The type is SVM_EVTINJ_TYPE_EXEPT, but the vector is not a valid exception.
+ */
+static bool nested_svm_check_event_inj(struct kvm_vcpu *vcpu, u32 event_inj)
+{
+	u32 type = event_inj & SVM_EVTINJ_TYPE_MASK;
+	u8 vector = event_inj & SVM_EVTINJ_VEC_MASK;
+
+	if (!(event_inj & SVM_EVTINJ_VALID))
+		return true;
+
+	if (type != SVM_EVTINJ_TYPE_INTR && type != SVM_EVTINJ_TYPE_NMI &&
+	    type != SVM_EVTINJ_TYPE_EXEPT && type != SVM_EVTINJ_TYPE_SOFT)
+		return false;
+
+	if (type == SVM_EVTINJ_TYPE_EXEPT &&
+	    !nested_svm_event_inj_valid_exept(vcpu, vector))
+		return false;
+
+	return true;
+}
+
 static bool nested_vmcb_check_controls(struct kvm_vcpu *vcpu,
 				       struct vmcb_ctrl_area_cached *control)
 {
@@ -365,6 +413,9 @@ static bool nested_vmcb_check_controls(struct kvm_vcpu *vcpu,
 		return false;
 	}
 
+	if (CC(!nested_svm_check_event_inj(vcpu, control->event_inj)))
+		return false;
+
 	return true;
 }
 
-- 
2.53.0.473.g4a7958ca14-goog


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v7 17/26] KVM: nSVM: Add missing consistency check for EVENTINJ
       [not found] <kvm-20260303003421.2185681-18-yosry@kernel.org>
@ 2026-08-03 18:08 ` Abdelkareem Abdelsaamad
  2026-08-05 18:52   ` Sean Christopherson
  0 siblings, 1 reply; 4+ messages in thread
From: Abdelkareem Abdelsaamad @ 2026-08-03 18:08 UTC (permalink / raw)
  To: kvm, linux-kernel, Yosry Ahmed
  Cc: seanjc, pbonzini, teddy.astie, jbeulich, andrew.cooper3,
	roger.pau, jason.andryuk

Hey,
I am currently working on hardening the Xen hypervisor's nested SVM
implementation to add the VMRUN consistency checks for injected events,
see the Xen patch discussion thread in [1].

While reviewing KVM's logic in nested_svm_event_inj_valid_exept(), I
can see that BR_VECTOR (5) and OF_VECTOR (4) are treated as
unconditionally valid. The referenced AMD APM Vol 2, Section 15.20
explicitly state otherwise:
"If the VMM attempts to inject an event that is impossible for the
guest mode (e.g., a #BR exception when the guest is in 64-bit mode),
the event injection will fail... VMRUN will immediately exit with
VMEXIT_INVALID."
"Injecting an exception (TYPE = 3) with vectors 3 or 4 behaves like
a trap raised by INT3 and INTO instructions, respectively"

Also, the APM volume 3 chapter 3 (INTO instruction), states that the
#OF triggering instruction, INTO, is Invalid in 64-bit mode.

I attempted testing the injection with Xen-Testing-Framework (XTF)
bare-minimum testing setup. I injected an exception (TYPE=3) with the
named vectors (BR_VECTOR (5) and OF_VECTOR (4)) on Genoa host. They
both caused VMEXIT_INVALID.

I think the check in nested_svm_event_inj_valid_exept() needs to be
gated on a condition that only allows Type 3 exception injections for
OF_VECTOR (4) and BR_VECTOR (5) when the guest is not in 64-bit mode.

Please, could you have a look and share your insights on the
implemented logic?

[1] https://lists.xenproject.org/archives/html/xen-devel/2026-07/msg00808.html

--Abdelkareem

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v7 17/26] KVM: nSVM: Add missing consistency check for EVENTINJ
  2026-03-03  0:34 ` [PATCH v7 17/26] KVM: nSVM: Add missing consistency check for EVENTINJ Yosry Ahmed
@ 2026-08-03 22:54   ` Abdelkareem Abdelsaamad
  0 siblings, 0 replies; 4+ messages in thread
From: Abdelkareem Abdelsaamad @ 2026-08-03 22:54 UTC (permalink / raw)
  To: kvm, linux-kernel, Yosry Ahmed
  Cc: seanjc, pbonzini, teddy.astie, jbeulich, andrew.cooper3,
	roger.pau, jason.andryuk

Hey,
I am currently working on hardening the Xen hypervisor's nested SVM
implementation to add the VMRUN consistency checks for injected events,
see the Xen patch discussion thread in [1].

While reviewing KVM's logic in nested_svm_event_inj_valid_exept(), I
can see that BR_VECTOR (5) and OF_VECTOR (4) are treated as
unconditionally valid. The referenced AMD APM Vol 2, Section 15.20
explicitly state otherwise:
"If the VMM attempts to inject an event that is impossible for the
guest mode (e.g., a #BR exception when the guest is in 64-bit mode),
the event injection will fail... VMRUN will immediately exit with
VMEXIT_INVALID."
"Injecting an exception (TYPE = 3) with vectors 3 or 4 behaves like
a trap raised by INT3 and INTO instructions, respectively"

Also, the APM volume 3 chapter 3 (INTO instruction), states that the
#OF triggering instruction, INTO, is Invalid in 64-bit mode.

I attempted testing the injection with Xen-Testing-Framework (XTF)
bare-minimum testing setup. I injected an exception (TYPE=3) with the
named vectors (BR_VECTOR (5) and OF_VECTOR (4)) on Genoa host. They
both caused VMEXIT_INVALID.

I think the check in nested_svm_event_inj_valid_exept() needs to be
gated on a condition that only allows Type 3 exception injections for
OF_VECTOR (4) and BR_VECTOR (5) when the guest is not in 64-bit mode.

Please, could you have a look and share your insights on the
implemented logic?

[1] https://lists.xenproject.org/archives/html/xen-devel/2026-07/msg00808.html

--Abdelkareem

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v7 17/26] KVM: nSVM: Add missing consistency check for EVENTINJ
  2026-08-03 18:08 ` [PATCH v7 17/26] KVM: nSVM: Add missing consistency check for EVENTINJ Abdelkareem Abdelsaamad
@ 2026-08-05 18:52   ` Sean Christopherson
  0 siblings, 0 replies; 4+ messages in thread
From: Sean Christopherson @ 2026-08-05 18:52 UTC (permalink / raw)
  To: Abdelkareem Abdelsaamad
  Cc: kvm, linux-kernel, Yosry Ahmed, pbonzini, teddy.astie, jbeulich,
	andrew.cooper3, roger.pau, jason.andryuk

On Mon, Aug 03, 2026, Abdelkareem Abdelsaamad wrote:
> Hey,
> I am currently working on hardening the Xen hypervisor's nested SVM
> implementation to add the VMRUN consistency checks for injected events,
> see the Xen patch discussion thread in [1].
> 
> While reviewing KVM's logic in nested_svm_event_inj_valid_exept(), I
> can see that BR_VECTOR (5) and OF_VECTOR (4) are treated as
> unconditionally valid. The referenced AMD APM Vol 2, Section 15.20
> explicitly state otherwise:
> "If the VMM attempts to inject an event that is impossible for the
> guest mode (e.g., a #BR exception when the guest is in 64-bit mode),
> the event injection will fail... VMRUN will immediately exit with
> VMEXIT_INVALID."
> "Injecting an exception (TYPE = 3) with vectors 3 or 4 behaves like
> a trap raised by INT3 and INTO instructions, respectively"
> 
> Also, the APM volume 3 chapter 3 (INTO instruction), states that the
> #OF triggering instruction, INTO, is Invalid in 64-bit mode.

LOL, _that's_ what SVM decides is worthy of a consistency check?

> I attempted testing the injection with Xen-Testing-Framework (XTF)
> bare-minimum testing setup. I injected an exception (TYPE=3) with the
> named vectors (BR_VECTOR (5) and OF_VECTOR (4)) on Genoa host. They
> both caused VMEXIT_INVALID.
> 
> I think the check in nested_svm_event_inj_valid_exept() needs to be
> gated on a condition that only allows Type 3 exception injections for
> OF_VECTOR (4) and BR_VECTOR (5) when the guest is not in 64-bit mode.

It'd probably require a dedicated check in nested_svm_check_cached_vmcb12(),
because the consistency check involves both control state and save state.  Given
that event injection validaton on SVM is inherently flawed due to hardware behavior
being microarchitecture specific, addressing this is very low down on the priority
list.  If someone wants to tackle it, by all means, but realistically I doubt this
will get fixed anytime soon.

> Please, could you have a look and share your insights on the
> implemented logic?
> 
> [1] https://lists.xenproject.org/archives/html/xen-devel/2026-07/msg00808.html
> 
> --Abdelkareem

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-08-05 18:52 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <kvm-20260303003421.2185681-18-yosry@kernel.org>
2026-08-03 18:08 ` [PATCH v7 17/26] KVM: nSVM: Add missing consistency check for EVENTINJ Abdelkareem Abdelsaamad
2026-08-05 18:52   ` Sean Christopherson
2026-03-03  0:33 [PATCH v7 00/26] Nested SVM fixes, cleanups, and hardening Yosry Ahmed
2026-03-03  0:34 ` [PATCH v7 17/26] KVM: nSVM: Add missing consistency check for EVENTINJ Yosry Ahmed
2026-08-03 22:54   ` Abdelkareem Abdelsaamad

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox