avformat/mpegenc: Fix memleaks and return values
authorAndreas Rheinhardt <andreas.rheinhardt@gmail.com>
Wed, 16 Oct 2019 16:47:39 +0000 (18:47 +0200)
committerMichael Niedermayer <michael@niedermayer.cc>
Thu, 17 Oct 2019 16:04:37 +0000 (18:04 +0200)
If there is an error in mpeg_mux_init() (the write_header function of
the various MPEG-PS muxers), two things might happen:

1. Several fifos might leak. Instead of freeing them, the goto fail part
of the functions freed the private data of the AVStreams instead,
although this will be freed later in free_stream() anyway.
2. And if the function is exited via goto fail, it automatically
returned AVERROR(ENOMEM), although this is also used when the error is
not a memory allocation failure.

Both of these issues happened in ticket #8284 and have been fixed.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Reviewed-by: Paul B Mahol <onemda@gmail.com>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
libavformat/mpegenc.c

index 43ebc46..93f40b2 100644 (file)
@@ -315,7 +315,7 @@ static av_cold int mpeg_mux_init(AVFormatContext *ctx)
         if (ctx->packet_size < 20 || ctx->packet_size > (1 << 23) + 10) {
             av_log(ctx, AV_LOG_ERROR, "Invalid packet size %d\n",
                    ctx->packet_size);
-            goto fail;
+            return AVERROR(EINVAL);
         }
         s->packet_size = ctx->packet_size;
     } else
@@ -343,7 +343,7 @@ static av_cold int mpeg_mux_init(AVFormatContext *ctx)
         st     = ctx->streams[i];
         stream = av_mallocz(sizeof(StreamInfo));
         if (!stream)
-            goto fail;
+            return AVERROR(ENOMEM);
         st->priv_data = stream;
 
         avpriv_set_pts_info(st, 64, 1, 90000);
@@ -377,11 +377,11 @@ static av_cold int mpeg_mux_init(AVFormatContext *ctx)
                     for (sr = 0; sr < 4; sr++)
                          av_log(ctx, AV_LOG_INFO, " %d", lpcm_freq_tab[sr]);
                     av_log(ctx, AV_LOG_INFO, "\n");
-                    goto fail;
+                    return AVERROR(EINVAL);
                 }
                 if (st->codecpar->channels > 8) {
                     av_log(ctx, AV_LOG_ERROR, "At most 8 channels allowed for LPCM streams.\n");
-                    goto fail;
+                    return AVERROR(EINVAL);
                 }
                 stream->lpcm_header[0] = 0x0c;
                 stream->lpcm_header[1] = (st->codecpar->channels - 1) | (j << 4);
@@ -416,7 +416,7 @@ static av_cold int mpeg_mux_init(AVFormatContext *ctx)
                        st->codecpar->codec_id != AV_CODEC_ID_MP2 &&
                        st->codecpar->codec_id != AV_CODEC_ID_MP3) {
                        av_log(ctx, AV_LOG_ERROR, "Unsupported audio codec. Must be one of mp1, mp2, mp3, 16-bit pcm_dvd, pcm_s16be, ac3 or dts.\n");
-                       goto fail;
+                       return AVERROR(EINVAL);
             } else {
                 stream->id = mpa_id++;
             }
@@ -460,7 +460,7 @@ static av_cold int mpeg_mux_init(AVFormatContext *ctx)
         }
         stream->fifo = av_fifo_alloc(16);
         if (!stream->fifo)
-            goto fail;
+            return AVERROR(ENOMEM);
     }
     bitrate       = 0;
     audio_bitrate = 0;
@@ -560,11 +560,6 @@ static av_cold int mpeg_mux_init(AVFormatContext *ctx)
     s->system_header_size = get_system_header_size(ctx);
     s->last_scr           = AV_NOPTS_VALUE;
     return 0;
-
-fail:
-    for (i = 0; i < ctx->nb_streams; i++)
-        av_freep(&ctx->streams[i]->priv_data);
-    return AVERROR(ENOMEM);
 }
 
 static inline void put_timestamp(AVIOContext *pb, int id, int64_t timestamp)
@@ -1255,11 +1250,18 @@ static int mpeg_mux_end(AVFormatContext *ctx)
         stream = ctx->streams[i]->priv_data;
 
         av_assert0(av_fifo_size(stream->fifo) == 0);
-        av_fifo_freep(&stream->fifo);
     }
     return 0;
 }
 
+static void mpeg_mux_deinit(AVFormatContext *ctx)
+{
+    for (int i = 0; i < ctx->nb_streams; i++) {
+        StreamInfo *stream = ctx->streams[i]->priv_data;
+        av_fifo_freep(&stream->fifo);
+    }
+}
+
 #define OFFSET(x) offsetof(MpegMuxContext, x)
 #define E AV_OPT_FLAG_ENCODING_PARAM
 static const AVOption options[] = {
@@ -1289,6 +1291,7 @@ AVOutputFormat ff_mpeg1system_muxer = {
     .write_header      = mpeg_mux_init,
     .write_packet      = mpeg_mux_write_packet,
     .write_trailer     = mpeg_mux_end,
+    .deinit            = mpeg_mux_deinit,
     .priv_class        = &mpeg_class,
 };
 #endif
@@ -1305,6 +1308,7 @@ AVOutputFormat ff_mpeg1vcd_muxer = {
     .write_header      = mpeg_mux_init,
     .write_packet      = mpeg_mux_write_packet,
     .write_trailer     = mpeg_mux_end,
+    .deinit            = mpeg_mux_deinit,
     .priv_class        = &vcd_class,
 };
 #endif
@@ -1322,6 +1326,7 @@ AVOutputFormat ff_mpeg2vob_muxer = {
     .write_header      = mpeg_mux_init,
     .write_packet      = mpeg_mux_write_packet,
     .write_trailer     = mpeg_mux_end,
+    .deinit            = mpeg_mux_deinit,
     .priv_class        = &vob_class,
 };
 #endif
@@ -1340,6 +1345,7 @@ AVOutputFormat ff_mpeg2svcd_muxer = {
     .write_header      = mpeg_mux_init,
     .write_packet      = mpeg_mux_write_packet,
     .write_trailer     = mpeg_mux_end,
+    .deinit            = mpeg_mux_deinit,
     .priv_class        = &svcd_class,
 };
 #endif
@@ -1358,6 +1364,7 @@ AVOutputFormat ff_mpeg2dvd_muxer = {
     .write_header      = mpeg_mux_init,
     .write_packet      = mpeg_mux_write_packet,
     .write_trailer     = mpeg_mux_end,
+    .deinit            = mpeg_mux_deinit,
     .priv_class        = &dvd_class,
 };
 #endif