Re: [Xen-devel] [PATCH v3 10/12] fuzz/x86_emulate: Add --rerun option to try to track down instability

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

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?

> @@ -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

/* */

> +    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.

> +            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() ?

> +            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

> +                        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?

~Andrew

> +
> +int LLVMFuzzerTestOneInput(const uint8_t *data_p, size_t size)
> +{
> +    struct fuzz_state state[2];
>  
>      if ( size < fuzz_minimal_input_size() )
>      {


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