From patchwork Tue Jul 21 02:12:12 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 21211 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 3073344B6A8 for ; Tue, 21 Jul 2020 05:12:45 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 22A4468BAFA; Tue, 21 Jul 2020 05:12:45 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-ed1-f66.google.com (mail-ed1-f66.google.com [209.85.208.66]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 4558968BAE5 for ; Tue, 21 Jul 2020 05:12:38 +0300 (EEST) Received: by mail-ed1-f66.google.com with SMTP id e22so14166722edq.8 for ; Mon, 20 Jul 2020 19:12:38 -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:in-reply-to:references :mime-version:content-transfer-encoding; bh=N/j9ICfi3cn1RMeK6qBFhpWSw5kLXt6XNEJow/B3FeY=; b=fiwHv384snUqBL9ly6FRWQVBKx2pTo4m8hOlDg8XsuTBwqJbdjmloU4A7dZ9qbpwUu rL2KzzgiMwG8E45bq+MiQYYOrMHpIXR3Sy3iRnRFag4W3/S2k6Pj/D3MKGRuDCs29Ayh DBRfQG/5oVEo2m9kG09GsonAQQ9YaCGV3TgLjeTKyZC1o5NE8/YKI9QClol9WE2GtYLI 8Ix3oRJ/XnLhpal/2qLjfi9IrnlxfFSD8zwwbkofg4cdWIWw6ulFOd5M0Og3VuLtMa4E maYYqkWGOg3wgGiF7OthMojGpqV/LyIYZOwyFyOJh2VX7FtJqdTI9RhOsfB5k4SGFPHg p9eQ== 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:in-reply-to :references:mime-version:content-transfer-encoding; bh=N/j9ICfi3cn1RMeK6qBFhpWSw5kLXt6XNEJow/B3FeY=; b=DEzj6usRSv9KHvoM0z1emF/390BRKY7RDUO7mQn2Lv3UlcovPwf2lRoXvTXMQhZRRb drDWSKGQxCWTcA/C3s+zKV1ItpVf/hCoErwx4TmtSYGg+0eeKC6d8cT8NpIdlObR+ezz Giit844nbgnbsN9ivhSuOYmJZbbFRq+uKkQ1sf+YS6OLxMf2AnNn9UmnLidV/bCPdaUW wfEfajOs/A0d41eoriw3riFE++1AXa+ArcvhW+Lnhqxk27es5//XnFXPRRF5oD8wgxdX Yc2crs6rUggKfroyORESPdwBt6BL38xrDDUDEBMdyZSEwClvbaOWNXUuK8exxQb8bp5+ afmw== X-Gm-Message-State: AOAM531zLBP5A+IQxycJORqlXPc/RM52UFwGdNHmqCujMyIoWL/C1cx9 nCyPwEU64SALqnP4OyQGi00KxdTD X-Google-Smtp-Source: ABdhPJyIaWr7xDLNMumnxxvVO3X6Gn53+O+ktWFabzOw3l8+sXFfd4oCymr+wHH3BWDf+6pN8MlGjg== X-Received: by 2002:a05:6402:1c86:: with SMTP id cy6mr23628931edb.30.1595297557143; Mon, 20 Jul 2020 19:12:37 -0700 (PDT) Received: from sblaptop.fritz.box (ipbcc10296.dynamic.kabel-deutschland.de. [188.193.2.150]) by smtp.gmail.com with ESMTPSA id p9sm15528563ejd.50.2020.07.20.19.12.35 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 20 Jul 2020 19:12:36 -0700 (PDT) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Tue, 21 Jul 2020 04:12:12 +0200 Message-Id: <20200721021215.32647-4-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20200719204755.32269-1-andreas.rheinhardt@gmail.com> References: <20200719204755.32269-1-andreas.rheinhardt@gmail.com> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH 6/9] avformat/rmdec: Fix memleaks upon read_header failure 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" For both the RealMedia as well as the IVR demuxer (which share the same context) each AVStream's priv_data contains an AVPacket that might contain data (even when reading the header) and therefore needs to be unreferenced. Up until now, this has not always been done: The RealMedia demuxer didn't do it when allocating a new stream's priv_data failed although there might other streams with packets to unreference. (The reason for this was that until recently rm_read_close() couldn't handle an AVStream without priv_data, so one had to choose between a potential crash and a memleak.) The IVR demuxer meanwhile never ever called read_close so that the data already contained in packets leaks upon error. This patch fixes both demuxers by setting the AVFMT_HEADER_CLEANUP flag, thereby ensuring that rm_read_close() is always called when reading the header fails. This also allows to remove several "goto fail" in rm_read_header(). Signed-off-by: Andreas Rheinhardt --- libavformat/rmdec.c | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/libavformat/rmdec.c b/libavformat/rmdec.c index 72b8dba741..c88f41c121 100644 --- a/libavformat/rmdec.c +++ b/libavformat/rmdec.c @@ -66,8 +66,6 @@ typedef struct RMDemuxContext { int data_end; } RMDemuxContext; -static int rm_read_close(AVFormatContext *s); - static inline void get_strl(AVIOContext *pb, char *buf, int buf_size, int len) { int read = avio_get_str(pb, len, buf, buf_size); @@ -557,16 +555,15 @@ static int rm_read_header(AVFormatContext *s) avio_skip(pb, tag_size - 8); for(;;) { - ret = AVERROR_INVALIDDATA; if (avio_feof(pb)) - goto fail; + return AVERROR_INVALIDDATA; tag = avio_rl32(pb); tag_size = avio_rb32(pb); avio_rb16(pb); av_log(s, AV_LOG_TRACE, "tag=%s size=%d\n", av_fourcc2str(tag), tag_size); if (tag_size < 10 && tag != MKTAG('D', 'A', 'T', 'A')) - goto fail; + return AVERROR_INVALIDDATA; switch(tag) { case MKTAG('P', 'R', 'O', 'P'): /* file header */ @@ -589,8 +586,7 @@ static int rm_read_header(AVFormatContext *s) case MKTAG('M', 'D', 'P', 'R'): st = avformat_new_stream(s, NULL); if (!st) { - ret = AVERROR(ENOMEM); - goto fail; + return AVERROR(ENOMEM); } st->id = avio_rb16(pb); avio_rb32(pb); /* max bit rate */ @@ -619,14 +615,14 @@ static int rm_read_header(AVFormatContext *s) if (v == MKBETAG('M', 'L', 'T', 'I')) { ret = rm_read_multi(s, s->pb, st, mime); if (ret < 0) - goto fail; + return ret; avio_seek(pb, codec_pos + size, SEEK_SET); } else { avio_skip(pb, -4); ret = ff_rm_read_mdpr_codecdata(s, s->pb, st, st->priv_data, size, mime); if (ret < 0) - goto fail; + return ret; } break; @@ -654,10 +650,6 @@ static int rm_read_header(AVFormatContext *s) } return 0; - -fail: - rm_read_close(s); - return ret; } static int get_num(AVIOContext *pb, int *len) @@ -1141,6 +1133,7 @@ static int rm_read_seek(AVFormatContext *s, int stream_index, AVInputFormat ff_rm_demuxer = { .name = "rm", .long_name = NULL_IF_CONFIG_SMALL("RealMedia"), + .flags = AVFMT_HEADER_CLEANUP, .priv_data_size = sizeof(RMDemuxContext), .read_probe = rm_probe, .read_header = rm_read_header, @@ -1393,6 +1386,7 @@ static int ivr_read_packet(AVFormatContext *s, AVPacket *pkt) AVInputFormat ff_ivr_demuxer = { .name = "ivr", .long_name = NULL_IF_CONFIG_SMALL("IVR (Internet Video Recording)"), + .flags = AVFMT_HEADER_CLEANUP, .priv_data_size = sizeof(RMDemuxContext), .read_probe = ivr_probe, .read_header = ivr_read_header,