On Thu, Jan 26, 2017 at 10:44:33AM -0600, Joel Cunningham wrote:
> Michael,
>
> Thanks for the review and testing! New patch included, see comments below
>
> >
> > this segfaults with many fuzzed files
> > backtrace looks like this:
> >
> > #0 0x00007fffffffb368 in ?? ()
> > #1 0x00000000005f9280 in avio_seek (s=0x7fffffffb278, offset=31, whence=0)
> > at libavformat/aviobuf.c:259
> >
>
> I found I wasn’t setting s->short_seek_get to NULL in ffio_init_context() and
> was relying on the zeroing aspect of av_mallocz() in avio_alloc_context().
> From a grep of the source, it looks like fifo_init_context() can be called
> from other functions without having zero’d the AVIOContext first.
>
> Is the fuzz test exercising the AVIOContext in this manner? I’d also like to
> reproduce this test if there are instructions
>
> >>
> >> diff --git a/libavformat/avio.c b/libavformat/avio.c
> >> index 3606eb0..ecf6801 100644
> >> --- a/libavformat/avio.c
> >> +++ b/libavformat/avio.c
> >> @@ -645,6 +645,13 @@ int ffurl_get_multi_file_handle(URLContext *h, int
> >> **handles, int *numhandles)
> >> return h->prot->url_get_multi_file_handle(h, handles, numhandles);
> >> }
> >>
> >> +int ffurl_get_short_seek(URLContext *h)
> >> +{
> >> + if (!h->prot->url_get_short_seek)
> >
> >> + return -1;
> >
> > should be some AVERROR code
>
> Fixed
>
> >> + return h->prot->url_get_short_seek(h);
> >> +}
> >> +
> >> int ffurl_shutdown(URLContext *h, int flags)
> >> {
> >> if (!h->prot->url_shutdown)
> >> diff --git a/libavformat/avio.h b/libavformat/avio.h
> >> index b1ce1d1..0480981 100644
> >> --- a/libavformat/avio.h
> >> +++ b/libavformat/avio.h
> >> @@ -287,6 +287,12 @@ typedef struct AVIOContext {
> >> int short_seek_threshold;
> >>
> >> /**
> >> + * A callback that is used instead of short_seek_threshold.
> >> + * This is current internal only, do not use from outside.
> >> + */
> >> + int (*short_seek_get)(void *opaque);
> >> +
> >> + /**
> >> * ',' separated list of allowed protocols.
> >> */
> >> const char *protocol_whitelist;
> >
> > thats not ok to add this way, the docs say this:
> > /**
> > * Bytestream IO Context.
> > * New fields can be added to the end with minor version bumps.
> > * Removal, reordering and changes to existing fields require a major
> > * version bump.
> > * sizeof(AVIOContext) must not be used outside libav*.
> > *
> > * @note None of the function pointers in AVIOContext should be called
> > * directly, they should only be set by the client application
> > * when implementing custom I/O. Normally these are set to the
> > * function pointers specified in avio_alloc_context()
> > */
> >
>
> I moved short_seek_get to the end of AVIOContext. I’m not sure if the minor
> version bump referenced in the comment requires any in-code changes. Is this
> an acceptable magnitude of change for this feature?
>
> > [...]
> >> --- a/libavformat/tcp.c
> >> +++ b/libavformat/tcp.c
> >> @@ -265,6 +265,26 @@ static int tcp_get_file_handle(URLContext *h)
> >> return s->fd;
> >> }
> >>
> >> +static int tcp_get_window_size(URLContext *h)
> >> +{
> >> + TCPContext *s = h->priv_data;
> >> + int avail;
> >> + int avail_len = sizeof(avail);
> >> +
> >
> >> + #if HAVE_WINSOCK2_H
> >
> > this formating is IIRC not allowed in C the # must be in the first
> > column
> >
> >
> >> + /* SO_RCVBUF with winsock only reports the actual TCP window size when
> >> + auto-tuning has been disabled via setting SO_RCVBUF */
> >> + if (s->recv_buffer_size < 0) {
> >> + return AVERROR(ENOSYS);
> >> + }
> >> + #endif
> >> +
> >> + if (getsockopt(s->fd, SOL_SOCKET, SO_RCVBUF, &avail, &avail_len)) {
> >> + return ff_neterrno();
> >> + }
> >
> > the indention is inconsisntent
>
> Both formatting issues have been corrected
>
> From bac0f351afd14c56c56376d20557b7330bcea23e Mon Sep 17 00:00:00 2001
> From: Joel Cunningham <joel.cunning...@me.com>
> Date: Fri, 13 Jan 2017 10:52:25 -0600
> Subject: [PATCH] HTTP: improve performance by reducing forward seeksplease attach a proper git format-patch or use git send-email applying this is not as smooth and easy as it should be [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB If you fake or manipulate statistics in a paper in physics you will never get a job again. If you fake or manipulate statistics in a paper in medicin you will get a job for life at the pharma industry.
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
