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

[Xen-devel] [PATCH v4 11/12] fuzz/x86_emulate: Set and fuzz more CPU state



x86_emulate() operates not only on state passed to it in
cpu_user_regs, but also on state currently found on the cpu: namely,
the FPU and XMM registers.  At the moment, we re-zero (and/or
re-initialize) cpu_user_regs on every invocation, but leave the
cpu-stored state alone.  In "persistent mode", this causes test cases
to behave differently -- sometimes significantly so -- depending on
which test cases have been run beforehand.

Zero out the state before each test run, and then fuzz it based on the
corpus input.

The Intel manual claims that, "If [certain CPUID bits] are set, the
processor deprecates FCS and FDS, and the field is saved as 0000h";
but experimentally it would be more accurate to say, "the field is
occasionally saved as 0000h".  This causes the --rerun checking to
trip non-deterministically.  Sanitize them to zero.

Signed-off-by: George Dunlap <george.dunlap@xxxxxxxxxx>
---
v4:
- Remove ineffective fxrstor
- Sanitize fcs and fds elements
v3:
- Make type 512 bytes rather than 464
- Style changes
- Change argument from 'store' to 'write'
- Add a comment explaining why we always 'save' even for a write
- Sanitize mxcsr with mxcrs_mask when writing instead of zeroing it in 
sanitize_state
- Get rid of redundant mxcsr_mask setting
- Add comments explaining why we're arbitrarily writing 32 bits
v2: Rebase on top of previous changes

CC: Ian Jackson <ian.jackson@xxxxxxxxxx>
CC: Wei Liu <wei.liu2@xxxxxxxxxx>
CC: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
CC: Jan Beulich <jbeulich@xxxxxxxx>
---
 tools/fuzz/x86_instruction_emulator/fuzz-emul.c | 102 +++++++++++++++++++++++-
 1 file changed, 101 insertions(+), 1 deletion(-)

diff --git a/tools/fuzz/x86_instruction_emulator/fuzz-emul.c 
b/tools/fuzz/x86_instruction_emulator/fuzz-emul.c
index f1621f98da..881c4d03c1 100644
--- a/tools/fuzz/x86_instruction_emulator/fuzz-emul.c
+++ b/tools/fuzz/x86_instruction_emulator/fuzz-emul.c
@@ -36,6 +36,7 @@ struct fuzz_state
     uint64_t msr[MSR_INDEX_MAX];
     struct segment_register segments[SEG_NUM];
     struct cpu_user_regs regs;
+    char fxsave[512] __attribute__((aligned(16)));
 
     /* Fuzzer's input data. */
 #define DATA_SIZE_FULL offsetof(struct fuzz_state, corpus)
@@ -594,6 +595,75 @@ static const struct x86_emulate_ops all_fuzzer_ops = {
 };
 #undef SET
 
+/*
+ * This funciton will read or write fxsave to the fpu.  When writing,
+ * it 'sanitizes' the state: It will mask off the appropriate bits in
+ * the mxcsr, 'restore' the state to the fpu, then 'save' it again so
+ * that the data in fxsave reflects what's actually in the FPU.
+ *
+ * TODO: Extend state beyond just FPU (ymm registers, &c)
+ */
+static void _set_fpu_state(char *fxsave, bool write)
+{
+    if ( cpu_has_fxsr )
+    {
+        static union __attribute__((__aligned__(16))) {
+            char x[512];
+            struct {
+                uint16_t cw, sw;
+                uint8_t  tw, _rsvd1;
+                uint16_t op;
+                uint32_t ip;
+                uint16_t cs, _rsvd2;
+                uint32_t dp;
+                uint16_t ds, _rsvd3;
+                uint32_t mxcsr;
+                uint32_t mxcsr_mask;
+                /* ... */
+            };
+        } *fxs;
+
+        fxs = (typeof(fxs))fxsave;
+
+        if ( write )
+        {
+            /* 
+             * Clear reserved bits to make sure we don't get any
+             * exceptions
+             */
+            fxs->mxcsr &= mxcsr_mask;
+
+            /*
+             * The Intel manual says that on newer models CS/DS are
+             * deprecated and that these fields "are saved as 0000h".
+             * Experimentally, however, at least on my test box,
+             * whether this saved as 0000h or as the previously
+             * written value is random; meaning that when run with
+             * --rerun, we occasionally detect a "state mismatch" in these
+             * bytes.  Instead, simply sanitize them to zero.
+             *
+             * TODO Check CPUID as specified in the manual before
+             * clearing
+             */
+            fxs->cs = fxs->ds = 0;
+
+            asm volatile( "fxrstor %0" :: "m" (*fxs) );
+        }
+
+        asm volatile( "fxsave %0" : "=m" (*fxs) );
+    }
+}
+
+static void set_fpu_state(char *fxsave)
+{
+    _set_fpu_state(fxsave, true);
+}
+
+static void save_fpu_state(char *fxsave)
+{
+    _set_fpu_state(fxsave, false);
+}
+
 static void setup_fpu_exception_handler(void)
 {
     /* FIXME - just disable exceptions for now */
@@ -669,7 +739,11 @@ static void setup_state(struct x86_emulate_ctxt *ctxt)
         return;
     }
 
-    /* Modify only select bits of state */
+    /*
+     * Modify only select bits of state.  In general, try not to fuzz less
+     * than 32 bits at a time; otherwise we're reading 2 bytes in order to 
fuzz only
+     * one byte. 
+     */
 
     /* Always read 'options' */
     if ( !input_read(s, s, DATA_SIZE_COMPACT) )
@@ -732,6 +806,18 @@ static void setup_state(struct x86_emulate_ctxt *ctxt)
             printf("Setting cpu_user_regs offset %x\n", offset);
             continue;
         }
+        offset -= sizeof(struct cpu_user_regs);
+
+        /* Fuzz fxsave state */
+        if ( offset < sizeof(s->fxsave) / 4 )
+        {
+            /* 32-bit size is arbitrary; see comment above */
+            if ( !input_read(s, s->fxsave + (offset * 4), 4) )
+                return;
+            printf("Setting fxsave offset %x\n", offset * 4);
+            continue;
+        }
+        offset -= 128;
 
         /* None of the above -- take that as "start emulating" */
         
@@ -914,6 +1000,8 @@ static int runtest(struct fuzz_state *state) {
 
     disable_hooks(state);
 
+    set_fpu_state(state->fxsave);
+
     do {
         /* FIXME: Until we actually implement SIGFPE handling properly */
         setup_fpu_exception_handler();
@@ -925,6 +1013,8 @@ static int runtest(struct fuzz_state *state) {
         printf("Emulation result: %d\n", rc);
     } while ( rc == X86EMUL_OKAY );
 
+    save_fpu_state(state->fxsave);
+    
     return 0;
 }
 
@@ -1008,6 +1098,16 @@ static void compare_states(struct fuzz_state state[2])
         if ( memcmp(&state[0].ops, &state[1].ops, sizeof(state[0].ops)) )
             printf("ops differ!\n");
 
+        if ( memcmp(&state[0].fxsave, &state[1].fxsave, 
sizeof(state[0].fxsave)) )
+        {
+            printf("fxsave differs!\n");
+            for ( i = 0;  i < sizeof(state[0].fxsave)/sizeof(unsigned); i++ )
+            {
+                printf("[%04lu] %08x %08x\n",
+                        i * sizeof(unsigned), ((unsigned 
*)&state[0].fxsave)[i], ((unsigned *)&state[1].fxsave)[i]);
+            }
+        }
+
         if ( memcmp(&state[0].ctxt, &state[1].ctxt, sizeof(state[0].ctxt)) )
         {
             printf("ctxt differs!\n");
-- 
2.14.2


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

 


Rackspace

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