From patchwork Tue Aug 18 22:00:37 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 21725 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 7AA70449705 for ; Wed, 19 Aug 2020 01:00:53 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 62EE96880D5; Wed, 19 Aug 2020 01:00:53 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-ed1-f68.google.com (mail-ed1-f68.google.com [209.85.208.68]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 61BF7680C49 for ; Wed, 19 Aug 2020 01:00:47 +0300 (EEST) Received: by mail-ed1-f68.google.com with SMTP id df16so16492829edb.9 for ; Tue, 18 Aug 2020 15:00:47 -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=j6z8tT18RtrVaTbRPY2t8cTzIYpr16HMPxvOo5GKO9s=; b=AGMysIV+Y64+TpdlzgausZZTjQGvIlTwWrzc00f8VtWT5Omw2nqclCqbuZPvq7VITc FJqH+l8/YNLmXwakAzbsPsD7zgn182tWX5qWd1U4YdbgzLISN6LQX1ZTtIPIxrSYDb9c BBJIeFSmTigVeSpk+TzfhVHlqFl5W3lpCN+UyMnMzNXPpVjzsIBv/IOWq+ynLnnrRwJE RPrpnnOGE7JOBGlCniNcmqquc7HTfhtMpNRtgVfZLdjbBrSwTeRkqOvd1tyLxzQon+08 s7iDp07VOFA5SHf8ZUWjPvFxESH7mI8gJ1D/WikHaTNlEAjTv4hKMBXOj7oiW0HH27u+ 9kbA== 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=j6z8tT18RtrVaTbRPY2t8cTzIYpr16HMPxvOo5GKO9s=; b=Et4RRBMuW93NClSbj220uH1p3uA13hFU/nNLFOEgGtMO2Unm+UvzsCGlG21jAny9js BFFWlQUHA7B7s82fTnPnZo90nUAOr109uHplDs09PsmO9A8LOZjT+4RyIrEqHAPePTIC 6ZBfzO3ZrGIkdL1WLJzOd9GjrLeGpLmZShoxQ5rWHEDqNZH45Xkc7QJ1Qmhl+B0LeJUi H2sgAmZIJizA1g0rSfhPPdNuR6MrT6yKov5Fwy1MCc/hb81KSPupWfMgc/G0Vs/F9Eal E4ov+AfaD8njhzV+O7v5zi9f1K4V/8jlWZ+xNr6MjKBrxvIsRt0nbJhicrdTHrN03XzW F9Mw== X-Gm-Message-State: AOAM533A9J0C+z/1sPCG12iPmP4u5JZG1YwGBNzf+I+IN6qycMw0iq85 KSwNZ6/n7G2RCO1J+fQYmqv8DtO8B48= X-Google-Smtp-Source: ABdhPJwsJvGgJJyB0HN/TFIQucaajkzP9jsA1EL2suZiXAZM5OZ5uJttBoMx5jTafrBQc2w8lp5YaA== X-Received: by 2002:a05:6402:c84:: with SMTP id cm4mr21567119edb.20.1597788046221; Tue, 18 Aug 2020 15:00:46 -0700 (PDT) Received: from sblaptop.fritz.box (ipbcc1fb0f.dynamic.kabel-deutschland.de. [188.193.251.15]) by smtp.gmail.com with ESMTPSA id h16sm16907577ejf.120.2020.08.18.15.00.45 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 18 Aug 2020 15:00:45 -0700 (PDT) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Wed, 19 Aug 2020 00:00:37 +0200 Message-Id: <20200818220037.10562-1-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.20.1 MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH] avformat/avidec: Fix memleak when error happens after creating DV stream 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" Signed-off-by: Andreas Rheinhardt --- 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)