Re: [Xen-devel] [PATCH v4 08/12] fuzz/x86_emulate: Move all state into fuzz_state

[George Dunlap <george.dunlap@xxxxxxxxxx>]

On 10/12/2017 04:16 PM, Jan Beulich wrote:
>>>> On 11.10.17 at 19:52, <george.dunlap@xxxxxxxxxx> wrote:
>> --- a/tools/fuzz/x86_instruction_emulator/fuzz-emul.c
>> +++ b/tools/fuzz/x86_instruction_emulator/fuzz-emul.c
>> @@ -22,34 +22,31 @@
>>  
>>  #define SEG_NUM x86_seg_none
>>  
>> -/* Layout of data expected as fuzzing input. */
>> -struct fuzz_corpus
>> +/*
>> + * State of the fuzzing harness and emulated cpu.  Calculated
>> + * initially from the input corpus, and later mutated by the emulation
>> + * callbacks (and the emulator itself, in the case of regs).
>> + */
>> +struct fuzz_state
>>  {
>> +    /* Emulated CPU state */
>> +    unsigned long options;
>>      unsigned long cr[5];
>>      uint64_t msr[MSR_INDEX_MAX];
>> -    struct cpu_user_regs regs;
>>      struct segment_register segments[SEG_NUM];
>> -    unsigned long options;
>> -    unsigned char data[INPUT_SIZE];
>> -} input;
>> -#define DATA_OFFSET offsetof(struct fuzz_corpus, data)
>> +    struct cpu_user_regs regs;
>>  
>> -/*
>> - * Internal state of the fuzzing harness.  Calculated initially from the 
>> input
>> - * corpus, and later mutates by the emulation callbacks.
>> - */
>> -struct fuzz_state
>> -{
>>      /* Fuzzer's input data. */
>> -    struct fuzz_corpus *corpus;
>> +#define DATA_OFFSET offsetof(struct fuzz_state, corpus)
>> +    const unsigned char * corpus;
> 
> Stray blank after *. Also any reason this can't be uint8_t,
> matching LLVMFuzzerTestOneInput()'s parameter and making
> it possible to avoid the cast you currently use on that
> assignment?

For some reason I thought this would make things uglier; but it actually
works pretty well.

>> @@ -646,11 +634,20 @@ static void set_sizes(struct x86_emulate_ctxt *ctxt)
>>          ctxt->addr_size = ctxt->sp_size = 64;
>>      else
>>      {
>> -        ctxt->addr_size = c->segments[x86_seg_cs].db ? 32 : 16;
>> -        ctxt->sp_size   = c->segments[x86_seg_ss].db ? 32 : 16;
>> +        ctxt->addr_size = s->segments[x86_seg_cs].db ? 32 : 16;
>> +        ctxt->sp_size   = s->segments[x86_seg_ss].db ? 32 : 16;
>>      }
>>  }
>>  
>> +static void setup_state(struct x86_emulate_ctxt *ctxt)
>> +{
>> +    struct fuzz_state *s = ctxt->data;
>> +
>> +    /* Fuzz all of the emulated state in one go */
>> +    if (!input_read(s, s, DATA_OFFSET))
> 
> Missing blanks.

Ack

> 
>> @@ -761,12 +757,11 @@ static void disable_hooks(struct x86_emulate_ctxt 
>> *ctxt)
>>  static void sanitize_input(struct x86_emulate_ctxt *ctxt)
>>  {
>>      struct fuzz_state *s = ctxt->data;
>> -    struct fuzz_corpus *c = s->corpus;
>> -    struct cpu_user_regs *regs = &c->regs;
>> -    unsigned long bitmap = c->options;
>> +    struct cpu_user_regs *regs = ctxt->regs;
>> +    unsigned long bitmap = s->options;
>>  
>>      /* Some hooks can't be disabled. */
>> -    c->options &= ~((1<<HOOK_read)|(1<<HOOK_insn_fetch));
>> +    s->options &= ~((1<<HOOK_read)|(1<<HOOK_insn_fetch));
> 
> Mind adding the missing blanks here while you touch this?

Like this?

    s->options &= ~((1<<HOOK_read) | (1<<HOOK_insn_fetch));

Thanks,
 -George

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