[FFmpeg-devel] [PATCH] avformat/mpegenc: Fix memleaks and return values
Paul B Mahol
onemda at gmail.com
Wed Oct 16 19:53:44 EEST 2019
LGTM
On 10/16/19, Andreas Rheinhardt <andreas.rheinhardt at gmail.com> wrote:
> 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 at gmail.com>
> ---
> libavformat/mpegenc.c | 31 +++++++++++++++++++------------
> 1 file changed, 19 insertions(+), 12 deletions(-)
>
> diff --git a/libavformat/mpegenc.c b/libavformat/mpegenc.c
> index 43ebc46e0e..93f40b202c 100644
> --- a/libavformat/mpegenc.c
> +++ b/libavformat/mpegenc.c
> @@ -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
> --
> 2.20.1
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
More information about the ffmpeg-devel
mailing list