On Fri, Feb 10, 2017 at 12:57 PM Jean-Marc Valin <jmvalin at jmvalin.ca> wrote:
> Hi Felicia,
>
> Overall the patch looks good to me and it's a pretty reasonable starting
> points. Some minor comments below.
>
> On 10/02/17 02:44 PM, Felicia Lim wrote:
> > - decodes a sequence of input packets rather than just one (I'm planning
> > on using the Opus test vectors as the seed corpus)
>
> I remember from experimenting with AFL that it didn't like the test
> vectors because they were very long and caused the fuzzer to run slowly.
> Depending on the fuzzer you're using, you might want to use shorter
> vectors. In general, it appears unlikely that any bug would require more
> than around 5 packets to trigger it.
>
I didn't see the same problem with libfuzzer, which oss fuzz is using now,
but haven't tried with AFL. Perhaps we can start with the test vectors for
now and I can add shorter vectors later.
> > - decides on decoder setup and FEC independently of the packet data
>
> I'm a little concerned with that one because it means you can never test
> the case where there's resampling in the first frame. For example, you
> cannot have a 16000 decoder that starts with a narrowband packet.
>
> That being said, I consider those minor issues and I don't see a problem
> with addressing them in a separate patch if it means we can start
> fuzzing earlier.
>
OK, I'll merge this first, and follow up in a later patch.
Thanks for the review.
Cheers,
Felicia
>
> Cheers,
>
> Jean-Marc
>
> > Cheers,
> > Felicia
> >
> > On Sun, Jan 29, 2017 at 9:48 PM Jean-Marc Valin <jmvalin at jmvalin.ca
> > <mailto:jmvalin at jmvalin.ca>> wrote:
> >
> > Hi Felicia,
> >
> > Here's a few comments/questions on your patch:
> >
> > > static void ParseToc(const uint8_t toc, TocInfo *const info) {
> >
> > Any particular reason you don't use the Opus functions for parsing
> the
> > ToC? It seems like opus_packet_get_nb_samples(),
> > opus_packet_get_bandwidth(), and opus_packet_get_nb_channels()
> should do
> > the trick.
> >
> > > int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size)
> >
> > How is that function supposed to work? I noticed that it will only
> > decode a single packet. How does the fuzzer handle testing for bugs
> that
> > only happen for a given sequence of input packets?
> >
> > Cheers,
> >
> > Jean-Marc
> >
> >
> >
> >
> > On 27/01/17 02:48 PM, Felicia Lim wrote:
> > > Hi all,
> > >
> > > I'm working on fuzzing Opus with OSS-Fuzz and have started with the
> > > decoder. Attached is a patch to add the corresponding fuzz target.
> > > Please let me know if there are any concerns?
> > >
> > > Thanks,
> > > Felicia
> > >
> > >
> > > _______________________________________________
> > > opus mailing list
> > > opus at xiph.org <mailto:opus at xiph.org>
> > > http://lists.xiph.org/mailman/listinfo/opus
> > >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.xiph.org/pipermail/opus/attachments/20170210/6551f4b4/attachment-0001.html>