[FFmpeg-devel] [PATCH] check for failed extradata malloc in avidec

[Michael Niedermayer]

On Thu, Jul 02, 2009 at 02:49:59PM +0200, Reimar D?ffinger wrote:
> On Wed, Jul 01, 2009 at 11:27:02PM +0200, Michael Niedermayer wrote:
> > On Wed, Jul 01, 2009 at 08:16:56PM +0200, Reimar D?ffinger wrote:
> > > Hello,
> > > this fixes one of the files in issue1240.
> > > Not sure if this is the best solution (the extradata size limit of 2^30 seems
> > > rather extreme) but anyway...
> > > Index: libavformat/avidec.c
> > > ===================================================================
> > > --- libavformat/avidec.c	(revision 19317)
> > > +++ libavformat/avidec.c	(working copy)
> > > @@ -478,6 +478,10 @@
> > >                      if(size > 10*4 && size<(1<<30)){
> > >                          st->codec->extradata_size= size - 10*4;
> > >                          st->codec->extradata= av_malloc(st->codec->extradata_size + FF_INPUT_BUFFER_PADDING_SIZE);
> > > +                        if (!st->codec->extradata) {
> > > +                            st->codec->extradata_size= 0;
> > > +                            return AVERROR(ENOMEM);
> > > +                        }
> > >                          get_buffer(pb, st->codec->extradata, st->codec->extradata_size);
> > >                      }
> > 
> > hmm, id treat malloc failure like a too large extradata
> > also a error message could be usefull for both
> 
> Well, here is it done that way, I don't like it too much.
> And I think it creates the strange situation that on my small PC the
> file now plays well thanks to the failing malloc, whereas on my normal
> PC it would at least pointlessly try to read in the whole file (maybe
> we should check how many bytes get_buffer actually read an resize the
> extradata buffer?).
> I also think the limit for extradata should be at most 1 << 27, that is
> 128MB and would already take 2 seconds to load from an average disk (and
> it wouldn't fail on the average PC)...

i think the size of the surrounding LIST chunk (i think there is one IIRC)
and fsize should be used to validate the chunk sizes of the header loop.
if thats done, iam ok with your original variant of ENOMEM

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

I wish the Xiph folks would stop pretending they've got something they
do not.  Somehow I fear this will remain a wish. -- M?ns Rullg?rd
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090702/203e4c0a/attachment.pgp>