[FFmpeg-devel] [PATCH] mxfdec: fix double free

Michael Niedermayer michaelni at gmx.at
Sun Dec 9 19:29:53 CET 2012


On Sun, Dec 09, 2012 at 06:55:57PM +0100, Tomas Härdin wrote:
> On Sat, 2012-12-08 at 05:23 +0100, Michael Niedermayer wrote:
> > Found-by: Mateusz "j00ru" Jurczyk and Gynvael Coldwind
> > Signed-off-by: Michael Niedermayer <michaelni at gmx.at>
> > ---
> >  libavformat/mxfdec.c |    8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
> > index 921dc42..a1884f1 100644
> > --- a/libavformat/mxfdec.c
> > +++ b/libavformat/mxfdec.c
> > @@ -1499,8 +1499,9 @@ static int mxf_parse_structural_metadata(MXFContext *mxf)
> >          codec_ul = mxf_get_codec_ul(ff_mxf_codec_uls, &descriptor->essence_codec_ul);
> >          st->codec->codec_id = (enum AVCodecID)codec_ul->id;
> >          if (descriptor->extradata) {
> > -            st->codec->extradata = descriptor->extradata;
> > -            st->codec->extradata_size = descriptor->extradata_size;
> > +            st->codec->extradata = av_mallocz(descriptor->extradata_size + FF_INPUT_BUFFER_PADDING_SIZE);
> > +            if (st->codec->extradata)
> > +                memcpy(st->codec->extradata, descriptor->extradata, descriptor->extradata_size);
> >          }
> >          if (st->codec->codec_type == AVMEDIA_TYPE_VIDEO) {
> >              source_track->intra_only = mxf_is_intra_only(descriptor);
> > @@ -2224,6 +2225,9 @@ static int mxf_read_close(AVFormatContext *s)
> >  
> >      for (i = 0; i < mxf->metadata_sets_count; i++) {
> >          switch (mxf->metadata_sets[i]->type) {
> > +        case Descriptor:
> > +            av_freep(&((MXFDescriptor *)mxf->metadata_sets[i])->extradata);
> > +            break;
> >          case MultipleDescriptor:
> >              av_freep(&((MXFDescriptor *)mxf->metadata_sets[i])->sub_descriptors_refs);
> >              break;
> 
> OK.
> 
> Future patch idea: check that extradata hasn't already been parsed when
> parsing the descriptor. That would avoid a potential memory leak.

something like this: ?

diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
index a1884f1..7af9193 100644
--- a/libavformat/mxfdec.c
+++ b/libavformat/mxfdec.c
@@ -866,6 +866,10 @@ static int mxf_read_generic_descriptor(void *arg, AVIOContext *pb, int tag, int
     default:
         /* Private uid used by SONY C0023S01.mxf */
         if (IS_KLV_KEY(uid, mxf_sony_mpeg4_extradata)) {
+            if (descriptor->extradata)
+                av_log(NULL, AV_LOG_WARNING, "Duplicate sony_mpeg4_extradata\n");
+            av_free(descriptor->extradata);
+            descriptor->extradata_size = 0;
             descriptor->extradata = av_malloc(size + FF_INPUT_BUFFER_PADDING_SIZE);
             if (!descriptor->extradata)
                 return AVERROR(ENOMEM);


[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Observe your enemies, for they first find out your faults. -- Antisthenes
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20121209/09a0f3dd/attachment.asc>


More information about the ffmpeg-devel mailing list