[Xen-devel] [PATCH v2] x86/emul: Correct the decoding of mov to/from cr/dr

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

The mov to/from cr/dr behave as if they were encoded with Mod = 3.  When
encoded with Mod != 3, no displacement or SIB bytes are fetched.

Add a test with a deliberately malformed ModRM byte.  (Also add the
automatically-generated simd.h to .gitignore.)

Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
CC: Jan Beulich <JBeulich@xxxxxxxx>

v2:
 * Rebase over SSE series.
 * oct => hex
---
 .gitignore                                   |  1 +
 tools/tests/x86_emulator/test_x86_emulator.c | 21 +++++++++++++++++++++
 xen/arch/x86/x86_emulate/x86_emulate.c       | 20 +++++++++++++++++++-
 3 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/.gitignore b/.gitignore
index 443b12a..4567de7 100644
--- a/.gitignore
+++ b/.gitignore
@@ -217,6 +217,7 @@ tools/security/xensec_tool
 tools/tests/x86_emulator/asm
 tools/tests/x86_emulator/blowfish.bin
 tools/tests/x86_emulator/blowfish.h
+tools/tests/x86_emulator/simd.h
 tools/tests/x86_emulator/test_x86_emulator
 tools/tests/x86_emulator/x86_emulate
 tools/tests/xen-access/xen-access
diff --git a/tools/tests/x86_emulator/test_x86_emulator.c 
b/tools/tests/x86_emulator/test_x86_emulator.c
index c5467a0..1e416fc 100644
--- a/tools/tests/x86_emulator/test_x86_emulator.c
+++ b/tools/tests/x86_emulator/test_x86_emulator.c
@@ -1000,6 +1000,27 @@ int main(int argc, char **argv)
     }
     printf("okay\n");
 
+    printf("%-40s", "Testing mov %%cr4,%%esi (bad ModRM)...");
+    /*
+     * Mod = 1, Reg = 4, R/M = 6 would normally encode a memory reference of
+     * disp8(%esi), but mov to/from cr/dr are special and behave as if they
+     * were encoded with Mod == 3.
+     */
+    instr[0] = 0x0f; instr[1] = 0x20, instr[2] = 0x66;
+    instr[3] = 0; /* Supposed disp8. */
+    regs.esi = 0;
+    regs.eip = (unsigned long)&instr[0];
+    rc = x86_emulate(&ctxt, &emulops);
+    /*
+     * We don't care precicely what gets read from %cr4 into %esi, just so
+     * long as ModRM is treated as a register operand and 0(%esi) isn't
+     * followed as a memory reference.
+     */
+    if ( (rc != X86EMUL_OKAY) ||
+         (regs.eip != (unsigned long)&instr[3]) )
+        goto fail;
+    printf("okay\n");
+
 #define decl_insn(which) extern const unsigned char which[], \
                          which##_end[] asm ( ".L" #which "_end" )
 #define put_insn(which, insn) ".pushsection .test, \"ax\", @progbits\n" \
diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c 
b/xen/arch/x86/x86_emulate/x86_emulate.c
index 63e4d89..1b507f7 100644
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -2269,7 +2269,8 @@ x86_decode_twobyte(
         }
         /* fall through */
     case 0x21: case 0x23: /* mov to/from dr */
-        generate_exception_if(lock_prefix || ea.type != OP_REG, EXC_UD);
+        ASSERT(ea.type == OP_REG); /* Early operand adjustment ensures this. */
+        generate_exception_if(lock_prefix, EXC_UD);
         op_bytes = mode_64bit() ? 8 : 4;
         break;
 
@@ -2685,6 +2686,23 @@ x86_decode(
             }
             break;
 
+        case ext_0f:
+            switch ( b )
+            {
+            case 0x20: /* mov cr,reg */
+            case 0x21: /* mov dr,reg */
+            case 0x22: /* mov reg,cr */
+            case 0x23: /* mov reg,dr */
+                /*
+                 * Mov to/from cr/dr ignore the encoding of Mod, and behave as
+                 * if they were encoded as reg/reg instructions.  No futher
+                 * disp/SIB bytes are fetched.
+                 */
+                modrm_mod = 3;
+                break;
+            }
+            break;
+
         case vex_0f38:
             d = ext0f38_table[b].to_mem ? DstMem | SrcReg
                                         : DstReg | SrcMem;
-- 
2.1.4


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