[Xen-devel] [PATCH 4/4] x86/vLAPIC: avoid speculative out of bounds accesses

["Jan Beulich" <JBeulich@xxxxxxxx>]

Array indexes used in the MMIO and MSR read/write emulation functions
are derived from guest controlled values. Restrict their ranges to limit
the side effects of speculative execution.

Remove the unused vlapic_lvt_{vector,dm}() instead of adjusting them.

Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

--- a/xen/arch/x86/hvm/vlapic.c
+++ b/xen/arch/x86/hvm/vlapic.c
@@ -23,6 +23,7 @@
 #include <xen/domain.h>
 #include <xen/domain_page.h>
 #include <xen/event.h>
+#include <xen/nospec.h>
 #include <xen/trace.h>
 #include <xen/lib.h>
 #include <xen/sched.h>
@@ -44,6 +45,8 @@
 #define VLAPIC_VERSION                  0x00050014
 #define VLAPIC_LVT_NUM                  6
 
+#define VLAPIC_OFFSET_MASK(vlapic) ((ARRAY_SIZE((vlapic)->regs->data) - 1) & 
~0xf)
+
 #define LVT_MASK \
     (APIC_LVT_MASKED | APIC_SEND_PENDING | APIC_VECTOR_MASK)
 
@@ -65,12 +68,6 @@ static const unsigned int vlapic_lvt_mas
      LVT_MASK
 };
 
-#define vlapic_lvt_vector(vlapic, lvt_type)                     \
-    (vlapic_get_reg(vlapic, lvt_type) & APIC_VECTOR_MASK)
-
-#define vlapic_lvt_dm(vlapic, lvt_type)                         \
-    (vlapic_get_reg(vlapic, lvt_type) & APIC_MODE_MASK)
-
 #define vlapic_lvtt_period(vlapic)                              \
     ((vlapic_get_reg(vlapic, APIC_LVTT) & APIC_TIMER_MODE_MASK) \
      == APIC_TIMER_MODE_PERIODIC)
@@ -632,7 +629,8 @@ static int vlapic_mmio_read(struct vcpu
      */
     if ( (alignment + len) <= 4 && offset <= (APIC_TDCR + 3) )
     {
-        uint32_t reg = vlapic_read_aligned(vlapic, offset & ~0xf);
+        uint32_t reg = vlapic_read_aligned(vlapic,
+                                           offset & 
VLAPIC_OFFSET_MASK(vlapic));
 
         switch ( len )
         {
@@ -665,7 +663,7 @@ int guest_rdmsr_x2apic(const struct vcpu
     };
     const struct vlapic *vlapic = vcpu_vlapic(v);
     uint64_t high = 0;
-    uint32_t reg = msr - MSR_X2APIC_FIRST, offset = reg << 4;
+    uint32_t reg = msr - MSR_X2APIC_FIRST, offset;
 
     /*
      * The read side looks as if it might be safe to use outside of current
@@ -675,9 +673,14 @@ int guest_rdmsr_x2apic(const struct vcpu
     ASSERT(v == current);
 
     if ( !vlapic_x2apic_mode(vlapic) ||
-         (reg >= sizeof(readable) * 8) || !test_bit(reg, readable) )
+         (reg >= sizeof(readable) * 8) )
         return X86EMUL_EXCEPTION;
 
+    reg = array_index_nospec(reg, sizeof(readable) * 8);
+    if ( !test_bit(reg, readable) )
+        return X86EMUL_EXCEPTION;
+
+    offset = reg << 4;
     if ( offset == APIC_ICR )
         high = (uint64_t)vlapic_read_aligned(vlapic, APIC_ICR2) << 32;
 
@@ -856,8 +859,8 @@ void vlapic_reg_write(struct vcpu *v, un
     case APIC_LVTERR:       /* LVT Error Reg */
         if ( vlapic_sw_disabled(vlapic) )
             val |= APIC_LVT_MASKED;
-        val &= vlapic_lvt_mask[(reg - APIC_LVTT) >> 4];
-        vlapic_set_reg(vlapic, reg, val);
+        val &= array_access_nospec(vlapic_lvt_mask, (reg - APIC_LVTT) >> 4);
+        vlapic_set_reg(vlapic, reg & VLAPIC_OFFSET_MASK(vlapic), val);
         if ( reg == APIC_LVT0 )
         {
             vlapic_adjust_i8259_target(v->domain);
@@ -901,8 +904,6 @@ static int vlapic_mmio_write(struct vcpu
     unsigned int offset = address - vlapic_base_address(vlapic);
     unsigned int alignment = offset & 0xf;
 
-    offset &= ~0xf;
-
     if ( offset != APIC_EOI )
         HVM_DBG_LOG(DBG_LEVEL_VLAPIC,
                     "offset %#x with length %#x, and value is %#lx",
@@ -915,8 +916,10 @@ static int vlapic_mmio_write(struct vcpu
      * Some processors support smaller accesses, so we allow any access which
      * fully fits within the 32-bit register.
      */
-    if ( (alignment + len) <= 4 && offset <= APIC_TDCR )
+    if ( (alignment + len) <= 4 && offset <= APIC_TDCR + 3 )
     {
+        offset &= VLAPIC_OFFSET_MASK(vlapic);
+
         if ( unlikely(len < 4) )
         {
             uint32_t reg = vlapic_read_aligned(vlapic, offset);
@@ -946,7 +949,7 @@ static int vlapic_mmio_write(struct vcpu
 int vlapic_apicv_write(struct vcpu *v, unsigned int offset)
 {
     struct vlapic *vlapic = vcpu_vlapic(v);
-    uint32_t val = vlapic_get_reg(vlapic, offset);
+    uint32_t val = vlapic_get_reg(vlapic, offset & VLAPIC_OFFSET_MASK(vlapic));
 
     if ( vlapic_x2apic_mode(vlapic) )
     {




_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel