Re: [Qemu-devel] [PATCH v3 16/22] fuzz: add fuzzer skeleton

[Stefan Hajnoczi]

On Thu, Sep 19, 2019 at 01:49:09PM +0000, Oleinik, Alexander wrote:
> On Thu, 2019-09-19 at 13:48 +0100, Stefan Hajnoczi wrote:
> > > +static void usage(char *path)
> > > +{
> > > +    printf("Usage: %s --FUZZ_TARGET [LIBFUZZER ARGUMENTS]\n",
> > > path);
> > > +    printf("where --FUZZ_TARGET is one of:\n");
> > 
> > Is the "--" prefix a libfuzzer requirement?  I would have expected
> > either FUZZ_TARGET by itself or --fuzz-target=FUZZ_TARGET (a properly
> > formatted long option) so that collisions with other command-line
> > options are not possible.
> Yes libfuzzer will only pass arguments that start with "--". I can
> replace it with --fuzz-target=FUZZ_TARGET. Alternatively, I can try to
> build separate binaries for each target. It might waste disk space, but
> we wouldn't need arguments (--trace could be replace with TRACE=1 in
> ENV). With this design, I'm not sure what to do with code such as
> i440fx_fuzz.c which re-purposes some functions for multiple different
> fuzz targets.

Building a single fuzzing binary with all targets feels natural.  Please
support the --fuzz-target=TARGET syntax though.

> > A cleaner API:
> > 
> >   /* Each fuzz target implements the following interface: */
> >   typedef struct {
> >       const char *name;        /* command-line option for this target
> > */
> >       const char *description; /* human-readable help text */
> > 
> >       /* TODO documentation */
> >       void (*pre_main)(void);
> > 
> >       /* TODO documentation */
> >       void (*pre_fuzz)(QTestState *);
> > 
> >       /* TODO documentation */
> >       void (*fuzz)(QTestState *, const unsigned char *, size_t);
> >   } FuzzTarget;
> 
> Sounds good. Should there also be argc and argv here? 

If they are read-only and provided by the FuzzTarget, then yes.  The
reason I consider this "cleaner" is because the FuzzTarget struct is
stateless and just captures the information about the fuzz target
instead of mixing it with runtime state.  But like I said, I didn't
really understand the design of the struct so maybe I don't understand
the full problem :).

signature.asc
Description: PGP signature