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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 10/10/17 17:20, George Dunlap wrote:
> This is in preparation for adding the option for a more "compact"
> interpretation of the fuzzing data, in which we only change select
> bits of the state.
>
> Signed-off-by: George Dunlap <george.dunlap@xxxxxxxxxx>
> Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
> ---
> v3:
>  - Move DATA_OFFSET inside the structure
>  - Remove a stray blank line
> v2: Port over 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 | 89 
> +++++++++++++------------
>  1 file changed, 45 insertions(+), 44 deletions(-)
>
> diff --git a/tools/fuzz/x86_instruction_emulator/fuzz-emul.c 
> b/tools/fuzz/x86_instruction_emulator/fuzz-emul.c
> index 8998f21fe1..20d52b33f8 100644
> --- a/tools/fuzz/x86_instruction_emulator/fuzz-emul.c
> +++ b/tools/fuzz/x86_instruction_emulator/fuzz-emul.c
> @@ -24,14 +24,8 @@
>  /* Layout of data expected as fuzzing input. */
>  struct fuzz_corpus
>  {
> -    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[4096];
>  } input;
> -#define DATA_OFFSET offsetof(struct fuzz_corpus, data)
>  
>  /*
>   * Internal state of the fuzzing harness.  Calculated initially from the 
> input
> @@ -39,7 +33,14 @@ struct fuzz_corpus
>   */

You've invalidated a number of the comments describing behaviours,
including the description of the difference between fuzz_state and
fuzz_corpus.

Why do you need to move any of this in the first place?  If you insist
on keeping the compact mode, what's wrong with conditionally reading the
AFL input into either fuzz_copus entirely, or into fuzz_corpus.data[]
and then conditionally deriving this state?

That way, you don't block work to fix the root cause, which needs to end
up with architectural values in fuzz_state, derived from a bitfields in
fuzz_corpus.

~Andrew

>  struct fuzz_state
>  {
> +    unsigned long options;
> +    unsigned long cr[5];
> +    uint64_t msr[MSR_INDEX_MAX];
> +    struct segment_register segments[SEG_NUM];
> +    struct cpu_user_regs regs;
> +
>      /* Fuzzer's input data. */
> +#define DATA_OFFSET offsetof(struct fuzz_state, corpus)
>      struct fuzz_corpus *corpus;
>  
>      /* Real amount of data backing corpus->data[]. */
>


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