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

[George Dunlap <george.dunlap@xxxxxxxxxx>]

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.

Signed-off-by: George Dunlap <george.dunlap@xxxxxxxxxx>
---
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 | 82 ++++++++++++++++++++++++-
 1 file changed, 81 insertions(+), 1 deletion(-)

diff --git a/tools/fuzz/x86_instruction_emulator/fuzz-emul.c 
b/tools/fuzz/x86_instruction_emulator/fuzz-emul.c
index 7685e976b8..79dd36ec30 100644
--- a/tools/fuzz/x86_instruction_emulator/fuzz-emul.c
+++ b/tools/fuzz/x86_instruction_emulator/fuzz-emul.c
@@ -40,6 +40,8 @@ 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)
@@ -596,6 +598,54 @@ 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 {
+                uint32_t other[6];
+                uint32_t mxcsr;
+                uint32_t mxcsr_mask;
+                /* ... */
+            };
+        } *fxs;
+
+        fxs = (typeof(fxs)) fxsave;
+
+        if ( write )
+        {
+            char null[512] __attribute__((aligned(16))) = { };
+            
+            fxs->mxcsr &= mxcsr_mask;
+
+            asm volatile( "fxrstor %0" :: "m" (*null) );
+            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 */
@@ -674,7 +724,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) )
@@ -737,6 +791,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 < 128 )
+        {
+            /* 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" */
         
@@ -919,6 +985,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();
@@ -930,6 +998,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;
 }
 
@@ -1013,6 +1083,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