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

[George Dunlap <george.dunlap@xxxxxxxxxx>]

On 10/11/2017 12:30 PM, George Dunlap wrote:
> On 10/10/2017 07:20 PM, Andrew Cooper wrote:
>> 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.
> 
> Well completely apart from the 'compact' format, I think this move makes
> sense.  The state moved is actually the state of the "emulated cpu" --
> the emulator actually modifies this state as instructions are executed.
> I think it makes sense to keep the "current state of the virtual
> processor" separate from "input we get from a file".

It's also necessary for when we add the `--rerun` parameter: We have to
make sure we leave the input data alone, and have two parallel states
that we set up and can compare.

 -George

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