From patchwork Mon Sep 7 02:49:48 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 22138 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 9ABE044BB3D for ; Mon, 7 Sep 2020 05:51:18 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 7B39A68A561; Mon, 7 Sep 2020 05:51:18 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-ej1-f66.google.com (mail-ej1-f66.google.com [209.85.218.66]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 790C768A212 for ; Mon, 7 Sep 2020 05:51:11 +0300 (EEST) Received: by mail-ej1-f66.google.com with SMTP id nw23so16188349ejb.4 for ; Sun, 06 Sep 2020 19:51:11 -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=K9t+LsdzkQ2VNUSwoiGEWYtZhporhG3WcwPpp6XTMmo=; b=dekW+KkHr5TnPakavS0g4g4uE8rcQ4FoIsRjn5oBf7FTB6mYU8l+IgQpbchTl0UBSn lrMNvQWbfFMYgHPbsPS05jmC4+O1If2T6Hc7yfHHeIrpeYCsf5Kjmy591QBWVTxmagkr PEb+X2qPr/8NXDlewtPoMh6Zh04hEGfOk7wMMiiozW6snjnnoZZlxqPp3cudW20c9gyi lRxJpi7i12/OHxJJLEmnEpijnfxH0Hkc9RKoWrT8PzGqW+VQ9wFQp6xKOiHc+v6ndWVD rPkfqVjZlJJWU/17UkrVBnOSNT02RiW0UuIKolqIzS02EDsDhR5ykGbDBr8wxxU9SSCc M7Gw== 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=K9t+LsdzkQ2VNUSwoiGEWYtZhporhG3WcwPpp6XTMmo=; b=Voic5ePozbdIOkEvJR61lAh7aSUDLERuUitqGrHdkV+/f9vaNA4eoZJkSVIbhwSfol 1b2SFdqBgDQYTs6sCdSxOiL8IQoSH2fHxt9kUiaHHTTB5oKZoJEPFKCogOMB+OmMWHCM NCKZj5qs+kxCpd5ylJbG8xptxNznBU4aERBnHoKWj87mi+gfeZD6GgZ8tEyGk3BIBmgS jFQqJrdWXSYh0nh3znFP4XjcRyWyu/K7jTnHn1MfDJo04AT9Dbx5KBSf3/HvCpbdyPLP WTzl21czfjidkQ2u0ccgA8R3D9ErOXDo4wtWYUGzfkuxOgwb07fGjOY81MGJwWgrBGb4 A2Yg== X-Gm-Message-State: AOAM530xiXxnszT1ceSx9aAUmNoJffVsxX+H1kkQWHAkUxHye+qxsL74 BWzammU60OOzmCgb2YkA6Pc1V7LaWtM= X-Google-Smtp-Source: ABdhPJyHP2/29sY4kS8ev2q2UHqiQfYlP5UDRc7BWpsSnaMctPYcOqNigD/R3FfJgZ4sEFIhr46uWg== X-Received: by 2002:a17:906:2552:: with SMTP id j18mr18551724ejb.476.1599447070662; Sun, 06 Sep 2020 19:51:10 -0700 (PDT) Received: from sblaptop.fritz.box (ipbcc1fb0f.dynamic.kabel-deutschland.de. [188.193.251.15]) by smtp.gmail.com with ESMTPSA id a15sm13802048eje.16.2020.09.06.19.51.09 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 06 Sep 2020 19:51:10 -0700 (PDT) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Mon, 7 Sep 2020 04:49:48 +0200 Message-Id: <20200907024952.11697-6-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20200907024952.11697-1-andreas.rheinhardt@gmail.com> References: <20200907024952.11697-1-andreas.rheinhardt@gmail.com> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH 06/10] avformat/segment: Fix leak and invalid free of AVIOContext 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" seg_init() and seg_write_header() currently contain a few error paths in which an already opened AVIOContext for the child muxer leaks (namely if there are unrecognized options for the child muxer or if writing the header of the child muxer fails); the reason for this is that this AVIOContext is not closed in the deinit function. If all goes well, it is closed when writing the trailer. From this it also follows that the AVIOContext also leaks when the trailer is never written, even when writing the header succeeds. But simply freeing said AVIOContext in the deinit function is complicated by the fact that the AVIOContext may or may not have been opened via the io_open callback: If options are set to discard header and trailer, said AVIOContext can also be a null context which must not be closed via the io_close callback. This may lead to crashes, as io_close may presume the AVIOContext's opaque to be set. It currently works with the default io_close callback which simply calls avio_close(), because avio_close() doesn't care about opaque being NULL since commit 6e8e8431e15a58aa44cfdd8c11f9ea096837c0fa. Therefore this commit records which of the two kinds of AVIOContext is currently in use to use the right way to close it. Finally there was one instance (namely if initializing the child muxer fails with no unrecognized options) where the AVIOContext was always closed via the io_close callback. The above remark applies to this; it has been fixed, too. Signed-off-by: Andreas Rheinhardt --- libavformat/segment.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/libavformat/segment.c b/libavformat/segment.c index b8eb0289c4..55d7f62ca0 100644 --- a/libavformat/segment.c +++ b/libavformat/segment.c @@ -80,6 +80,7 @@ typedef struct SegmentContext { int list_flags; ///< flags affecting list generation int list_size; ///< number of entries for the segment list file + int is_nullctx; ///< whether avf->pb is a nullctx int use_clocktime; ///< flag to cut segments at regular clock time int64_t clocktime_offset; //< clock offset for cutting the segments at regular clock time int64_t clocktime_wrap_duration; //< wrapping duration considered for starting a new segment @@ -661,8 +662,14 @@ static void seg_free(AVFormatContext *s) { SegmentContext *seg = s->priv_data; ff_format_io_close(s, &seg->list_pb); - avformat_free_context(seg->avf); - seg->avf = NULL; + if (seg->avf) { + if (seg->is_nullctx) + close_null_ctxp(&seg->avf->pb); + else + ff_format_io_close(s, &seg->avf->pb); + avformat_free_context(seg->avf); + seg->avf = NULL; + } av_freep(&seg->times); av_freep(&seg->frames); av_freep(&seg->cur_entry.filename); @@ -777,6 +784,7 @@ static int seg_init(AVFormatContext *s) } else { if ((ret = open_null_ctx(&oc->pb)) < 0) return ret; + seg->is_nullctx = 1; } av_dict_copy(&options, seg->format_options, 0); @@ -791,7 +799,6 @@ static int seg_init(AVFormatContext *s) av_dict_free(&options); if (ret < 0) { - ff_format_io_close(oc, &oc->pb); return ret; } seg->segment_frame_count = 0; @@ -834,6 +841,7 @@ static int seg_write_header(AVFormatContext *s) ff_format_io_close(oc, &oc->pb); } else { close_null_ctxp(&oc->pb); + seg->is_nullctx = 0; } if ((ret = oc->io_open(oc, &oc->pb, oc->url, AVIO_FLAG_WRITE, NULL)) < 0) return ret; @@ -984,6 +992,7 @@ static int seg_write_trailer(struct AVFormatContext *s) goto fail; if ((ret = open_null_ctx(&oc->pb)) < 0) goto fail; + seg->is_nullctx = 1; ret = av_write_trailer(oc); close_null_ctxp(&oc->pb); } else {