[PATCH v2] nSVM: Check injected event consistency

[Abdelkareem Abdelsaamad <abdelkareem.abdelsaamad@xxxxxxxxxx>]

On the AMD platforms, allowing a VMRUN instruction with a malformed VMCB has
debugging complications, security and performance implications. The APM volume
2 15.20 [1] states two possibilities that result in a VMRUN exit with
VMEXIT_INVALID due to injected events. These are either
• Reserved values of TYPE have been specified.
• 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).
Extend the VMCB checks to check for such inconsistency.

The collection of the invalid exception vectors are picked from the upstream KVM
commit ("7e79f71bca5c" KVM: nSVM: Add missing consistency check for EVENTINJ).

[1] https://docs.amd.com/v/u/en-US/24593_3.44_APM_Vol2

Signed-off-by: Abdelkareem Abdelsaamad <abdelkareem.abdelsaamad@xxxxxxxxxx>
---
Changes in v2:
- Remove the redundant SVM_EVENT_INJ_TYPE_MASK and SVM_EVENT_INJ_VEC_MASK
  constants.
- Correct the Injected Event Type consistency check to disallow the injection
  of reserved type 1 events.
---
Testing:
 - Using a locally developed XTF nested virt setup, I manually tested VMRUN
   instruction handling with a malformed VMCB:
   1) Inject event with the type (7).
      The hypervisor logs show the message
      (XEN) [  645.155609] d2v0[nsvm_vmcb_prepare4vmrun]: eventinj: Invalid 
Injected
            Event Type: (0x7)
   2) Inject event with the exception value (3) and the vector value (2) for 
NMI.
      The hypervisor logs show the message
      (XEN) [  645.157277] d2v0[nsvm_vmcb_prepare4vmrun]: eventinj: Invalid 
Injected Event.
             Exception type: (0x3), with a vector: (0x2) does not belong to an 
exception

 - CI tests:
https://gitlab.com/xen-project/people/aabdelsa/xen/-/pipelines/2682300446
---
 xen/arch/x86/hvm/svm/vmcb.c | 40 +++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/xen/arch/x86/hvm/svm/vmcb.c b/xen/arch/x86/hvm/svm/vmcb.c
index 975a1eaef8..c31d2a6f58 100644
--- a/xen/arch/x86/hvm/svm/vmcb.c
+++ b/xen/arch/x86/hvm/svm/vmcb.c
@@ -320,6 +320,31 @@ void svm_vmcb_dump(const char *from, const struct 
vmcb_struct *vmcb)
     svm_dump_sel("  TR", &vmcb->tr);
 }
 
+static bool is_valid_svm_vmcb_injected_exception_vector(
+    const struct vmcb_struct *vmcb, uint8_t vmcb_injected_vector)
+{
+    return ( (vmcb_injected_vector == X86_EXC_DE) ||
+             (vmcb_injected_vector == X86_EXC_DB) ||
+             (vmcb_injected_vector == X86_EXC_BP) ||
+             (vmcb_injected_vector == X86_EXC_OF) ||
+             (vmcb_injected_vector == X86_EXC_BR) ||
+             (vmcb_injected_vector == X86_EXC_UD) ||
+             (vmcb_injected_vector == X86_EXC_NM) ||
+             (vmcb_injected_vector == X86_EXC_DF) ||
+             (vmcb_injected_vector == X86_EXC_TS) ||
+             (vmcb_injected_vector == X86_EXC_NP) ||
+             (vmcb_injected_vector == X86_EXC_SS) ||
+             (vmcb_injected_vector == X86_EXC_GP) ||
+             (vmcb_injected_vector == X86_EXC_PF) ||
+             (vmcb_injected_vector == X86_EXC_MF) ||
+             (vmcb_injected_vector == X86_EXC_AC) ||
+             (vmcb_injected_vector == X86_EXC_MC) ||
+             (vmcb_injected_vector == X86_EXC_XM) ||
+             (vmcb_injected_vector == X86_EXC_HV) ||
+             (vmcb_injected_vector == X86_EXC_SX) ||
+             (vmcb_get_sev_es(vmcb) && vmcb_injected_vector == X86_EXC_VC) );
+}
+
 bool svm_vmcb_isvalid(
     const char *from, const struct vmcb_struct *vmcb, const struct vcpu *v,
     bool verbose)
@@ -330,6 +355,12 @@ bool svm_vmcb_isvalid(
     unsigned long cr4 = vmcb_get_cr4(vmcb);
     unsigned long valid;
     uint64_t efer = vmcb_get_efer(vmcb);
+    uint8_t vmcb_injected_type = vmcb->event_inj.type;
+    uint8_t vmcb_injected_vector = vmcb->event_inj.vector;
+    uint8_t vmcb_valid_event_inj_types_mask = (1 << X86_ET_EXT_INTR) |
+                                              (1 << X86_ET_NMI) |
+                                              (1 << X86_ET_HW_EXC) |
+                                              (1 << X86_ET_SW_INT);
 
 #define PRINTF(fmt, args...) do { \
     if ( !verbose ) return true; \
@@ -392,6 +423,15 @@ bool svm_vmcb_isvalid(
         PRINTF("eventinj: MBZ bits are set (%#"PRIx64")\n",
                vmcb->event_inj.raw);
 
+    if ( !((1 << vmcb_injected_type) & vmcb_valid_event_inj_types_mask) )
+        PRINTF("eventinj: Invalid Injected Event Type: (%#"PRIx8")\n",
+               vmcb_injected_type);
+
+    if ( (vmcb_injected_type == X86_ET_HW_EXC) &&
+         !is_valid_svm_vmcb_injected_exception_vector(vmcb, 
vmcb_injected_vector) )
+        PRINTF("eventinj: Invalid Injected Event. Exception type: (%#"PRIx8"),"
+               " with a vector: (%#"PRIx8") does not belong to an exception\n",
+               vmcb_injected_type, vmcb_injected_vector);
 #undef PRINTF
     return ret;
 }
-- 
2.53.0