From patchwork Wed Oct 16 16:47:39 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 15791 Return-Path: X-Original-To: patchwork@ffaux-bg.ffmpeg.org Delivered-To: patchwork@ffaux-bg.ffmpeg.org Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by ffaux.localdomain (Postfix) with ESMTP id 1235E4481DA for ; Wed, 16 Oct 2019 19:47:54 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id DDA2568A56E; Wed, 16 Oct 2019 19:47:53 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wm1-f65.google.com (mail-wm1-f65.google.com [209.85.128.65]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 1620D68A3ED for ; Wed, 16 Oct 2019 19:47:48 +0300 (EEST) Received: by mail-wm1-f65.google.com with SMTP id y21so3539250wmi.0 for ; Wed, 16 Oct 2019 09:47:48 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=uACWJkk3MWQ7LSBIk4k9YZyWzaRulUc9U7pPtirGclQ=; b=sLvpLQDXqjlhIThUsDjL32zHlscyLV9ddmYrrMoo9GIAuEjEghTa5ciTgwpRvSrbST iQzyL2D/Xz/Y/DDk4SPhgKzn+LQGJdReQ1A58ucCdlJDgJBEwqDX08drBKWFs/Q1MmST Wxs7AzVluiXKBKS7dPGPLA79uX2PKP0ASKokXT9TBLXCMygPPj/nm6XrLtiFnsUmaAsL sOTxidSgmyd2u1gSqmImjZCtMG2m1sDiQrHcj2OB/R+fe1vz3sTxNrTsgldibmM25FI/ wbbyGT0u64plRMEb8acG/D8a99Qkis3OPiknafnbKPPe8s7sHNNpUqwLZEna+N1PeQh7 zAmA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=uACWJkk3MWQ7LSBIk4k9YZyWzaRulUc9U7pPtirGclQ=; b=h+TRbMJwZ+P1oIOwBwfWloFZPKHB7Ys4D1NOC0b8mDbhKbc/XE6QcEMJ4IK6k1BwG4 WXoicSMeyu6DYN/EsMLdMk3FjlSJd5zgvMNot1/P0OLxAbyAgsbiTkTyiUKTygee7ozh HArBCkDwcW3EO2yz6mn+0ehoZPl++kq4P+pz/eWMiNcL/d3T0tiFBVc8KmXA0UKskq+w muJV02ru6HsKEz6qcnIJd1Zm5L3K+FDE6bzMzdrXs54c0nsjhb3T7H4Z27nPbak3Gure lJ8itomfmdylaSJRdkceX5oHsmJ0UM2Nl8/cMeQZ/ehL7E7JZaNySjreLqBIRwvpx8Yf RMHw== X-Gm-Message-State: APjAAAWotYVlTnKEoyWgqaPWXUoSrEmXe3N6/hRYdV399OCV6mOO4o25 nE2OcVHyTJcR6R9LNuxjqcdB0rMc X-Google-Smtp-Source: APXvYqz1wLNBnnGFRBhCcGTHzp7iH94adfvY1gGoQcY6yezTsnG9QOoBQ8FO/NQqRxkDPS6/5ig88A== X-Received: by 2002:a1c:740a:: with SMTP id p10mr4325489wmc.130.1571244467309; Wed, 16 Oct 2019 09:47:47 -0700 (PDT) Received: from sblaptop.fritz.box (ipbcc08937.dynamic.kabel-deutschland.de. [188.192.137.55]) by smtp.gmail.com with ESMTPSA id e17sm3038105wma.15.2019.10.16.09.47.46 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 16 Oct 2019 09:47:46 -0700 (PDT) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Wed, 16 Oct 2019 18:47:39 +0200 Message-Id: <20191016164739.15583-1-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.20.1 MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH] avformat/mpegenc: Fix memleaks and return values X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Cc: Andreas Rheinhardt Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" 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 --- 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