|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v3 10/12] fuzz/x86_emulate: Add --rerun option to try to track down instability
On 10/10/2017 07:44 PM, Andrew Cooper wrote:
> On 10/10/17 17:20, George Dunlap wrote:
>> @@ -659,7 +667,10 @@ static void setup_state(struct x86_emulate_ctxt *ctxt)
>> {
>> /* Fuzz all of the state in one go */
>> if ( !input_read(s, s, DATA_SIZE_FULL) )
>> + {
>> + printf("Input size too small\n");
>> exit(-1);
>> + }
>
> Is this hunk intended to be in the previous patch?
Looking more into it, I suppose it shouldn't be needed at all, since we
check the input size in the main fuzzing function.
>
>> @@ -886,21 +896,144 @@ int LLVMFuzzerInitialize(int *argc, char ***argv)
>> return 0;
>> }
>>
>> -int LLVMFuzzerTestOneInput(const uint8_t *data_p, size_t size)
>> +static void setup_fuzz_state(struct fuzz_state *state, const void *data_p,
>> size_t size)
>> {
>> - struct fuzz_state state = {
>> - .ops = all_fuzzer_ops,
>> - };
>> - struct x86_emulate_ctxt ctxt = {
>> - .data = &state,
>> - .regs = &state.regs,
>> - .addr_size = 8 * sizeof(void *),
>> - .sp_size = 8 * sizeof(void *),
>> - };
>> + memset(state, 0, sizeof(*state));
>> + state->corpus = data_p;
>> + state->data_num = size;
>> +}
>> +
>> +static int runtest(struct fuzz_state *state) {
>> int rc;
>>
>> - /* Reset all global state variables */
>> - memset(&input, 0, sizeof(input));
>> + struct x86_emulate_ctxt *ctxt = &state->ctxt;
>> +
>> + state->ops = all_fuzzer_ops;
>> +
>> + ctxt->data = state;
>> + ctxt->regs = &state->regs;
>> +
>> + setup_state(ctxt);
>> +
>> + sanitize_state(ctxt);
>> +
>> + disable_hooks(state);
>> +
>> + do {
>> + /* FIXME: Until we actually implement SIGFPE handling properly */
>> + setup_fpu_exception_handler();
>> +
>> + set_sizes(ctxt);
>> + dump_state(ctxt);
>> +
>> + rc = x86_emulate(ctxt, &state->ops);
>> + printf("Emulation result: %d\n", rc);
>> + } while ( rc == X86EMUL_OKAY );
>> +
>> + return 0;
>> +}
>> +
>> +static void compare_states(struct fuzz_state state[2])
>> +{
>> + // First zero any "internal" pointers
>
> /* */
Ack
>
>> + state[0].corpus = state[1].corpus = NULL;
>> + state[0].ctxt.data = state[1].ctxt.data = NULL;
>> + state[0].ctxt.regs = state[1].ctxt.regs = NULL;
>> +
>> + if ( memcmp(&state[0], &state[1], sizeof(struct fuzz_state)) )
>> + {
>> + unsigned int i;
>> +
>> + printf("State mismatch\n");
>> +
>> + for ( i=0; i<ARRAY_SIZE(state[0].cr); i++ )
>
> Various spaces.
Ack
>
>> + if ( state[0].cr[i] != state[1].cr[i] )
>> + printf("cr[%u]: %lx != %lx\n",
>> + i, state[0].cr[i], state[1].cr[i]);
>> +
>> + for ( i=0; i<ARRAY_SIZE(state[0].msr); i++ )
>> + if ( state[0].msr[i] != state[1].msr[i] )
>> + printf("msr[%u]: %lx != %lx\n",
>> + i, state[0].msr[i], state[1].msr[i]);
>> +
>> + for ( i=0; i<ARRAY_SIZE(state[0].segments); i++ )
>> + if ( memcmp(&state[0].segments[i], &state[1].segments[i],
>> + sizeof(state[0].segments[0])) )
>> + printf("segments[%u]: [%x:%x:%x:%lx] != [%x:%x:%x:%lx]!\n",
>> i,
>> + (unsigned)state[0].segments[i].sel,
>> + (unsigned)state[0].segments[i].attr,
>> + state[0].segments[i].limit,
>> + state[0].segments[i].base,
>> + (unsigned)state[1].segments[i].sel,
>> + (unsigned)state[1].segments[i].attr,
>> + state[1].segments[i].limit,
>> + state[1].segments[i].base);
>> +
>> + if ( state[0].data_num != state[1].data_num )
>> + printf("data_num: %lx != %lx\n", state[0].data_num,
>> + state[1].data_num);
>> + if ( state[0].data_index != state[1].data_index )
>> + printf("data_index: %lx != %lx\n", state[0].data_index,
>> + state[1].data_index);
>> +
>> + if ( memcmp(&state[0].regs, &state[1].regs, sizeof(state[0].regs)) )
>> + {
>> + printf("registers differ!\n");
>> + /* Print If Not Equal */
>> +#define PINE(elem)\
>
> PRINT_NE() ?
OK.
>
>> + if ( state[0].elem != state[1].elem ) \
>> + printf(#elem " differ: %lx != %lx\n", \
>> + (unsigned long)state[0].elem, \
>> + (unsigned long)state[1].elem)
>> + PINE(regs.r15);
>
> Hmm - this is going to cause problems for the 32bit build. I can't
> think of an easy suggestion to fix it.
>
>> + PINE(regs.r14);
>> + PINE(regs.r13);
>> + PINE(regs.r12);
>> + PINE(regs.rbp);
>> + PINE(regs.rbx);
>> + PINE(regs.r10);
>> + PINE(regs.r11);
>> + PINE(regs.r9);
>> + PINE(regs.r8);
>> + PINE(regs.rax);
>> + PINE(regs.rcx);
>> + PINE(regs.rdx);
>> + PINE(regs.rsi);
>> + PINE(regs.rdi);
>> +
>> + for ( i = offsetof(struct cpu_user_regs, error_code) /
>> sizeof(unsigned);
>> + i < sizeof(state[1].regs)/sizeof(unsigned); i++ )
>> + {
>> + printf("[%04lu] %08x %08x\n",
>> + i * sizeof(unsigned), ((unsigned
>> *)&state[0].regs)[i],
>> + ((unsigned *)&state[1].regs)[i]);
>
> It would be helpful to pull out at least rflags individually, because
> that has the highest individual chance of being unstable. The segment
> selectors might also be nice to pull out individually, whereas
> everything else should be zero and remain zero throughout (at which
> point, hex-dumping is fine).
>
>> + }
>> + }
>> +
>> + if ( memcmp(&state[0].ops, &state[1].ops, sizeof(state[0].ops)) )
>> + printf("ops differ!\n");
>> +
>> + if ( memcmp(&state[0].ctxt, &state[1].ctxt, sizeof(state[0].ctxt)) )
>> + {
>> + printf("ctxt differs!\n");
>> + for ( i = 0; i < sizeof(state[0].ctxt)/sizeof(unsigned); i++ )
>> + {
>> + printf("[%04lu] %08x %08x\n",
>
> %04zu for size_t
Ack
>
>> + i * sizeof(unsigned), ((unsigned
>> *)&state[0].ctxt)[i],
>> + ((unsigned *)&state[1].ctxt)[i]);
>> + }
>> +
>> + }
>> +
>> + abort();
>> + }
>> +}
>> +
>> +bool opt_rerun = false;
>
> Should this not be somewhere near the top of the file?
I've been introducing these just before they're used so it's easier to
see the default value. If you prefer them all to be at the top of the
file I can move them there.
-George
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |