[FFmpeg-devel] [PATCH] avformat: Add max_streams option

Michael Niedermayer michael at niedermayer.cc
Fri Nov 18 19:18:08 EET 2016


On Fri, Nov 18, 2016 at 05:17:10PM +0100, wm4 wrote:
> On Fri, 18 Nov 2016 17:03:15 +0100
> Michael Niedermayer <michael at niedermayer.cc> wrote:
> 
> > This allows user apps to stop OOM due to excessive number of streams
> > TODO: bump & docs
> > 
> > Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
> > ---
> >  libavformat/avformat.h      | 7 +++++++
> >  libavformat/options_table.h | 1 +
> >  libavformat/utils.c         | 2 +-
> >  3 files changed, 9 insertions(+), 1 deletion(-)
> > 
> > diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> > index f9f4d72..c81a916 100644
> > --- a/libavformat/avformat.h
> > +++ b/libavformat/avformat.h
> > @@ -1899,6 +1899,13 @@ typedef struct AVFormatContext {
> >       * - decoding: set by user through AVOptions (NO direct access)
> >       */
> >      char *protocol_blacklist;
> > +
> > +    /**
> > +     * The maximum number of streams.
> > +     * - encoding: unused
> > +     * - decoding: set by user through AVOptions (NO direct access)
> 
> Can we stop this. It makes no sense at all to document something like
> it's public API, and then not allowing access to it. Besides, "NO
> direct access" doesn't even make clear _how_ to access it.

If the previous field cannot be accessed directly due to it being
possibly (re)moved then all following fields must have the same note.

If this note is removed it has to be removed from all the fields in
the struct in git first, i cannot just ommit it in a newly added field


> 
> > +     */
> > +    int max_streams;
> >  } AVFormatContext;
> >  
> >  int av_format_get_probe_score(const AVFormatContext *s);
> > diff --git a/libavformat/options_table.h b/libavformat/options_table.h
> > index 9d61d5a..d5448e5 100644
> > --- a/libavformat/options_table.h
> > +++ b/libavformat/options_table.h
> > @@ -105,6 +105,7 @@ static const AVOption avformat_options[] = {
> >  {"format_whitelist", "List of demuxers that are allowed to be used", OFFSET(format_whitelist), AV_OPT_TYPE_STRING, { .str = NULL },  CHAR_MIN, CHAR_MAX, D },
> >  {"protocol_whitelist", "List of protocols that are allowed to be used", OFFSET(protocol_whitelist), AV_OPT_TYPE_STRING, { .str = NULL },  CHAR_MIN, CHAR_MAX, D },
> >  {"protocol_blacklist", "List of protocols that are not allowed to be used", OFFSET(protocol_blacklist), AV_OPT_TYPE_STRING, { .str = NULL },  CHAR_MIN, CHAR_MAX, D },
> > +{"max_streams", "maximum number of streams", OFFSET(max_streams), AV_OPT_TYPE_INT, { .i64 = INT_MAX }, 0, INT_MAX, D },
> >  {NULL},
> >  };
> >  
> > diff --git a/libavformat/utils.c b/libavformat/utils.c
> > index 5664646..ded0b52 100644
> > --- a/libavformat/utils.c
> > +++ b/libavformat/utils.c
> > @@ -4210,7 +4210,7 @@ AVStream *avformat_new_stream(AVFormatContext *s, const AVCodec *c)
> >      int i;
> >      AVStream **streams;
> >  
> > -    if (s->nb_streams >= INT_MAX/sizeof(*streams))
> > +    if (s->nb_streams >= s->max_streams/sizeof(*streams))
> >          return NULL;
> >      streams = av_realloc_array(s->streams, s->nb_streams + 1, sizeof(*streams));
> >      if (!streams)
> 
> I wonder how this option makes sense? There are a lot of other things
> that are limited by available memory only. If this is supposed to be
> some sort of DoS protection it's not effective.

I agree
People report OOM bugs due this and other things though. (this patch
was the reult of one such report today)
If we can cut the number of such reports down by 50% by limiting
things which are easy to limit during fuzz testing that should make
fuzzing more efficient

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

While the State exists there can be no freedom; when there is freedom there
will be no State. -- Vladimir Lenin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20161118/60ecf03a/attachment.sig>


More information about the ffmpeg-devel mailing list