[FFmpeg-devel] [PATCH] avformat/avidec: Fix memleak when error happens after creating DV stream

[Andreas Rheinhardt]

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
---
The memleak can be reproduced with e.g. the first 163 bytes of
https://samples.ffmpeg.org/archive/all/avi+dvvideo+pcm_s16le++ffmpeg-avidec554-crash.avi

 libavformat/avidec.c | 31 +++++++++++++++++--------------
 1 file changed, 17 insertions(+), 14 deletions(-)

diff --git a/libavformat/avidec.c b/libavformat/avidec.c
index 5fc3e01aa9..08b864f19a 100644
--- a/libavformat/avidec.c
+++ b/libavformat/avidec.c
@@ -113,6 +113,7 @@ static const AVMetadataConv avi_metadata_conv[] = {
     { 0 },
 };
 
+static int avi_read_close(AVFormatContext *s);
 static int avi_load_index(AVFormatContext *s);
 static int guess_ni_flag(AVFormatContext *s);
 
@@ -464,6 +465,7 @@ static int calculate_bitrate(AVFormatContext *s)
     return 1;
 }
 
+#define RETURN_ERROR(code) do { ret = (code); goto fail; } while (0)
 static int avi_read_header(AVFormatContext *s)
 {
     AVIContext *avi = s->priv_data;
@@ -499,7 +501,7 @@ static int avi_read_header(AVFormatContext *s)
     frame_period = 0;
     for (;;) {
         if (avio_feof(pb))
-            goto fail;
+            RETURN_ERROR(AVERROR_INVALIDDATA);
         tag  = avio_rl32(pb);
         size = avio_rl32(pb);
 
@@ -571,12 +573,12 @@ static int avi_read_header(AVFormatContext *s)
                 stream_index++;
                 st = avformat_new_stream(s, NULL);
                 if (!st)
-                    goto fail;
+                    RETURN_ERROR(AVERROR(ENOMEM));
 
                 st->id = stream_index;
                 ast    = av_mallocz(sizeof(AVIStream));
                 if (!ast)
-                    goto fail;
+                    RETURN_ERROR(AVERROR(ENOMEM));
                 st->priv_data = ast;
             }
             if (amv_file_format)
@@ -592,12 +594,12 @@ static int avi_read_header(AVFormatContext *s)
                 /* After some consideration -- I don't think we
                  * have to support anything but DV in type1 AVIs. */
                 if (s->nb_streams != 1)
-                    goto fail;
+                    RETURN_ERROR(AVERROR_INVALIDDATA);
 
                 if (handler != MKTAG('d', 'v', 's', 'd') &&
                     handler != MKTAG('d', 'v', 'h', 'd') &&
                     handler != MKTAG('d', 'v', 's', 'l'))
-                    goto fail;
+                    return AVERROR_INVALIDDATA;
 
                 if (!CONFIG_DV_DEMUXER)
                     return AVERROR_DEMUXER_NOT_FOUND;
@@ -697,7 +699,7 @@ static int avi_read_header(AVFormatContext *s)
                            "Invalid sample_size %d at stream %d\n",
                            ast->sample_size,
                            stream_index);
-                    goto fail;
+                    RETURN_ERROR(AVERROR_INVALIDDATA);
                 }
                 av_log(s, AV_LOG_WARNING,
                        "Invalid sample_size %d at stream %d "
@@ -927,7 +929,7 @@ static int avi_read_header(AVFormatContext *s)
                         av_log(s, AV_LOG_WARNING, "New extradata in strd chunk, freeing previous one.\n");
                     }
                     if ((ret = ff_get_extradata(s, st->codecpar, pb, size)) < 0)
-                        return ret;
+                        goto fail;
                 }
 
                 if (st->codecpar->extradata_size & 1) //FIXME check if the encoder really did this correctly
@@ -945,7 +947,7 @@ static int avi_read_header(AVFormatContext *s)
                 avi->use_odml &&
                 read_odml_index(s, 0) < 0 &&
                 (s->error_recognition & AV_EF_EXPLODE))
-                goto fail;
+                RETURN_ERROR(AVERROR_INVALIDDATA);
             avio_seek(pb, pos + size, SEEK_SET);
             break;
         case MKTAG('v', 'p', 'r', 'p'):
@@ -980,7 +982,7 @@ static int avi_read_header(AVFormatContext *s)
             if (s->nb_streams) {
                 ret = avi_read_tag(s, s->streams[s->nb_streams - 1], tag, size);
                 if (ret < 0)
-                    return ret;
+                    goto fail;
                 break;
             }
         default:
@@ -991,7 +993,7 @@ static int avi_read_header(AVFormatContext *s)
                        "I will ignore it and try to continue anyway.\n",
                        av_fourcc2str(tag), size);
                 if (s->error_recognition & AV_EF_EXPLODE)
-                    goto fail;
+                    RETURN_ERROR(AVERROR_INVALIDDATA);
                 avi->movi_list = avio_tell(pb) - 4;
                 avi->movi_end  = avi->fsize;
                 goto end_of_header;
@@ -1008,9 +1010,7 @@ static int avi_read_header(AVFormatContext *s)
 end_of_header:
     /* check stream number */
     if (stream_index != s->nb_streams - 1) {
-
-fail:
-        return AVERROR_INVALIDDATA;
+        RETURN_ERROR(AVERROR_INVALIDDATA);
     }
 
     if (!avi->index_loaded && (pb->seekable & AVIO_SEEKABLE_NORMAL))
@@ -1019,7 +1019,7 @@ fail:
     avi->index_loaded    |= 1;
 
     if ((ret = guess_ni_flag(s)) < 0)
-        return ret;
+        goto fail;
 
     avi->non_interleaved |= ret | (s->flags & AVFMT_FLAG_SORT_DTS);
 
@@ -1056,6 +1056,9 @@ fail:
     ff_metadata_conv_ctx(s, NULL, ff_riff_info_conv);
 
     return 0;
+fail:
+    avi_read_close(s);
+    return ret;
 }
 
 static int read_gab2_sub(AVFormatContext *s, AVStream *st, AVPacket *pkt)
-- 
2.20.1