[Xen-devel] [PATCH 1/4] x86emul: avoid speculative out of bounds accesses

["Jan Beulich" <JBeulich@xxxxxxxx>]

There are a few array accesses here the indexes of which are (at least
indirectly) driven by the guest. Use array_access_nospec() to bound
such accesses. In the {,_}decode_gpr() cases replace existing guarding
constructs.

To deal with an otherwise occurring #include cycle, drop the inclusion
of asm/x86_emulate.h from asm/processor.h. This include had been
introduced for obtaining the struct cpuid_leaf declaration, which has
since moved into the x86 helper library.

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

--- a/xen/arch/x86/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate.c
@@ -102,7 +102,7 @@ int x86emul_read_dr(unsigned int reg, un
     switch ( reg )
     {
     case 0 ... 3:
-        *val = curr->arch.dr[reg];
+        *val = array_access_nospec(curr->arch.dr, reg);
         break;
 
     case 4:
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -2207,10 +2207,7 @@ static void *_decode_gpr(
 
     ASSERT(modrm_reg < ARRAY_SIZE(byte_reg_offsets));
 
-    /* For safety in release builds.  Debug builds will hit the ASSERT() */
-    modrm_reg &= ARRAY_SIZE(byte_reg_offsets) - 1;
-
-    return (void *)regs + byte_reg_offsets[modrm_reg];
+    return (void *)regs + array_access_nospec(byte_reg_offsets, modrm_reg);
 }
 
 static unsigned long *decode_vex_gpr(
@@ -2935,7 +2932,7 @@ x86_decode(
                     b = insn_fetch_type(uint8_t);
                     opcode |= MASK_INSR(0x8f08 + ext - ext_8f08,
                                         X86EMUL_OPC_EXT_MASK);
-                    d = xop_table[ext - ext_8f08];
+                    d = array_access_nospec(xop_table, ext - ext_8f08);
                 }
                 else
                 {
--- a/xen/arch/x86/x86_emulate/x86_emulate.h
+++ b/xen/arch/x86/x86_emulate/x86_emulate.h
@@ -642,6 +642,12 @@ int x86_emulate_wrapper(
 #define x86_emulate x86_emulate_wrapper
 #endif
 
+#ifdef __XEN__
+# include <xen/nospec.h>
+#else
+# define array_access_nospec(arr, idx) arr[idx]
+#endif
+
 /* Map GPRs by ModRM encoding to their offset within struct cpu_user_regs. */
 extern const uint8_t cpu_user_regs_gpr_offsets[X86_NR_GPRS];
 
@@ -658,10 +664,7 @@ static inline unsigned long *decode_gpr(
 
     ASSERT(modrm < ARRAY_SIZE(cpu_user_regs_gpr_offsets));
 
-    /* For safety in release builds.  Debug builds will hit the ASSERT() */
-    modrm &= ARRAY_SIZE(cpu_user_regs_gpr_offsets) - 1;
-
-    return (void *)regs + cpu_user_regs_gpr_offsets[modrm];
+    return (void *)regs + array_access_nospec(cpu_user_regs_gpr_offsets, 
modrm);
 }
 
 /* Unhandleable read, write or instruction fetch */
--- a/xen/include/asm-x86/processor.h
+++ b/xen/include/asm-x86/processor.h
@@ -13,7 +13,6 @@
 #include <asm/types.h>
 #include <asm/cpufeature.h>
 #include <asm/desc.h>
-#include <asm/x86_emulate.h>
 #endif
 
 #include <asm/x86-defns.h>





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