From 30de02998df3b253dce68904cfdd50cdfe6fb3ed Mon Sep 17 00:00:00 2001 From: Andreas Rheinhardt Date: Sun, 6 Sep 2020 00:29:33 +0200 Subject: [PATCH] avformat/segment: Fix leak and invalid free of AVIOContext 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. Reviewed-by: Ridley Combs 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 5cc9eb812f..cf29d336be 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 @@ -660,8 +661,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); @@ -767,6 +774,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); @@ -781,7 +789,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; @@ -824,6 +831,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; @@ -974,6 +982,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 {