[Xen-devel] [PATCH v3] x86_emulate: split the {reg, mem} union in struct operand.

[Tim Deegan <tim@xxxxxxx>]

In the hopes of making any future errors along the lines of XSA-123
into clean crashes instead of memory corruption bugs.

Signed-off-by: Tim Deegan <tim@xxxxxxx>
Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
CC: Keir Fraser <keir@xxxxxxx>
CC: Jan Beulich <jbeulich@xxxxxxxx>
---
v3: drop unhelpful unnamed struct (Jan Beulich / gcc)
v2: tweak poison values (Andrew Cooper)
---
 xen/arch/x86/x86_emulate/x86_emulate.c | 27 ++++++++++++++++-----------
 1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c 
b/xen/arch/x86/x86_emulate/x86_emulate.c
index c082c9e..ae32c82 100644
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -319,16 +319,20 @@ struct operand {
     /* Original operand value. */
     unsigned long orig_val;
 
-    union {
-        /* OP_REG: Pointer to register field. */
-        unsigned long *reg;
-        /* OP_MEM: Segment and offset. */
-        struct {
-            enum x86_segment seg;
-            unsigned long    off;
-        } mem;
-    };
+    /* OP_REG: Pointer to register field. */
+    unsigned long *reg;
+
+    /* OP_MEM: Segment and offset. */
+    struct {
+        enum x86_segment seg;
+        unsigned long    off;
+    } mem;
 };
+#ifdef __x86_64__
+#define REG_POISON ((unsigned long *) 0x8086000000008086UL) /* non-canonical */
+#else
+#define REG_POISON NULL /* 32-bit builds are for user-space, so NULL is OK. */
+#endif
 
 typedef union {
     uint64_t mmx;
@@ -1447,14 +1451,15 @@ x86_emulate(
     unsigned int op_bytes, def_op_bytes, ad_bytes, def_ad_bytes;
     bool_t lock_prefix = 0;
     int override_seg = -1, rc = X86EMUL_OKAY;
-    struct operand src, dst;
+    struct operand src = { .reg = REG_POISON };
+    struct operand dst = { .reg = REG_POISON };
     enum x86_swint_type swint_type;
     DECLARE_ALIGNED(mmval_t, mmval);
     /*
      * Data operand effective address (usually computed from ModRM).
      * Default is a memory operand relative to segment DS.
      */
-    struct operand ea = { .type = OP_MEM };
+    struct operand ea = { .type = OP_MEM, .reg = REG_POISON };
     ea.mem.seg = x86_seg_ds; /* gcc may reject anon union initializer */
 
     ctxt->retire.byte = 0;
-- 
2.1.0


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel