[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH v1] nSVM: Check injected event consistency



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>
---
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/2678629270
---
 xen/arch/x86/hvm/svm/vmcb.c | 36 ++++++++++++++++++++++++++++++++++++
 xen/arch/x86/hvm/svm/vmcb.h |  3 +++
 2 files changed, 39 insertions(+)

diff --git a/xen/arch/x86/hvm/svm/vmcb.c b/xen/arch/x86/hvm/svm/vmcb.c
index 975a1eaef8..1ca8474958 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,8 @@ 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 & 
SVM_EVENT_INJ_TYPE_MASK;
+    uint8_t vmcb_injected_vector = vmcb->event_inj.vector & 
SVM_EVENT_INJ_VEC_MASK;
 
 #define PRINTF(fmt, args...) do { \
     if ( !verbose ) return true; \
@@ -392,6 +419,15 @@ bool svm_vmcb_isvalid(
         PRINTF("eventinj: MBZ bits are set (%#"PRIx64")\n",
                vmcb->event_inj.raw);
 
+    if ( vmcb_injected_type > X86_ET_SW_INT )
+        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;
 }
diff --git a/xen/arch/x86/hvm/svm/vmcb.h b/xen/arch/x86/hvm/svm/vmcb.h
index 13ccfd3ff9..3c22095db1 100644
--- a/xen/arch/x86/hvm/svm/vmcb.h
+++ b/xen/arch/x86/hvm/svm/vmcb.h
@@ -306,6 +306,9 @@ enum
     TLB_CTRL_FLUSH_ASID_NONGLOBAL = 7,
 };
 
+#define SVM_EVENT_INJ_VEC_MASK     0xff
+#define SVM_EVENT_INJ_TYPE_MASK    0x7
+
 typedef union
 {
     struct
-- 
2.53.0




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.