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

["Jan Beulich" <JBeulich@xxxxxxxx>]

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

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

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

> @@ -834,10 +826,10 @@ int LLVMFuzzerTestOneInput(const uint8_t *data_p, 
> size_t size)
>          return 1;
>      }
>  
> -    memcpy(&input, data_p, size);
> +    state.corpus = (void*)data_p;

If for any reason the suggested type change can't or shouldn't be
done (and hence the cast needs to stay), then please add a blank
before * and don't cast away const-ness.

Jan


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