Re: [Xen-devel] [PATCH] fuzz/x86_emulate: fix bounds for input size

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Fri, Feb 23, 2018 at 04:33:18PM +0000, George Dunlap wrote:
> On 02/23/2018 04:30 PM, Wei Liu wrote:
> > On Fri, Feb 23, 2018 at 12:57:26AM +0100, Paul Semel wrote:
> >> The minimum size for the input size was set to DATA_OFFSET + 1 which was 
> >> meaning
> >> that we were requesting at least one character of the data array to be 
> >> filled.
> >> This is not needed for the fuzzer to get working correctly.
> > 
> > Sorry, I don't follow -- what do you expect the emulator to do if there
> > is no instruction to emulate?
> > 
> >>
> >> The maximum size for the input size was set to INPUT_SIZE, which is 
> >> actually
> >> the size of the data array inside the fuzz_corpus structure and so was not
> >> abling user (or AFL) to fill in the whole structure. Changing to
> >> sizeof(struct fuzz_corpus) correct this problem.
> >>
> >> Signed-off-by: Paul Semel <semelpaul@xxxxxxxxx>
> >> ---
> >>  tools/fuzz/x86_instruction_emulator/fuzz-emul.c | 9 +++++----
> >>  1 file changed, 5 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/tools/fuzz/x86_instruction_emulator/fuzz-emul.c 
> >> b/tools/fuzz/x86_instruction_emulator/fuzz-emul.c
> >> index 964682aa1a..f3ce2e7e27 100644
> >> --- a/tools/fuzz/x86_instruction_emulator/fuzz-emul.c
> >> +++ b/tools/fuzz/x86_instruction_emulator/fuzz-emul.c
> >> @@ -33,6 +33,7 @@ struct fuzz_corpus
> >>      unsigned char data[INPUT_SIZE];
> >>  } input;
> >>  #define DATA_OFFSET offsetof(struct fuzz_corpus, data)
> >> +#define FUZZ_CORPUS_SIZE (sizeof(struct fuzz_corpus))
> >>  
> >>  /*
> >>   * Internal state of the fuzzing harness.  Calculated initially from the 
> >> input
> >> @@ -822,13 +823,13 @@ int LLVMFuzzerTestOneInput(const uint8_t *data_p, 
> >> size_t size)
> >>      /* Reset all global state variables */
> >>      memset(&input, 0, sizeof(input));
> >>  
> >> -    if ( size <= DATA_OFFSET )
> >> +    if ( size < DATA_OFFSET )
> >>      {
> >>          printf("Input too small\n");
> >>          return 1;
> >>      }
> >>  
> >> -    if ( size > INPUT_SIZE )
> >> +    if ( size > FUZZ_CORPUS_SIZE )
> >>      {
> >>          printf("Input too large\n");
> >>          return 1;
> >> @@ -859,9 +860,9 @@ int LLVMFuzzerTestOneInput(const uint8_t *data_p, 
> >> size_t size)
> >>  
> >>  unsigned int fuzz_minimal_input_size(void)
> >>  {
> >> -    BUILD_BUG_ON(DATA_OFFSET > INPUT_SIZE);
> >> +    BUILD_BUG_ON(DATA_OFFSET > FUZZ_CORPUS_SIZE);
> > 
> > Thinking more about it, this BUILD_BUG_ON is probably irrelevant
> > nowadays because we've opted to use struct fuzz_corpus instead of a
> > bunch of data structures (when the fuzzer was first implemented). I
> > don't think we will go back to the old model in the future so deleting
> > this BUILD_BUG_ON should be fine.
> 
> We statically allocate an array of data of size INPUT_SIZE in
> afl-harness.c; I think that's the purpose of the BUILD_BUG_ON(), to make
> sure that that buffer is always big enough for our minimum file size.

The arrangement at the moment (instruction stream input being a member
of the corpus) makes sure DATA_OFFSET will never be larger than
FUZZ_CORPUS_SIZE.

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel