diff mbox series

[FFmpeg-devel,06/10] avformat/segment: Fix leak and invalid free of AVIOContext

Message ID 20200907024952.11697-6-andreas.rheinhardt@gmail.com
State Accepted
Commit 30de02998df3b253dce68904cfdd50cdfe6fb3ed
Headers show
Series [FFmpeg-devel,01/10] avformat/segment: Don't overwrite AVCodecParameters after init | expand


Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Andreas Rheinhardt Sept. 7, 2020, 2:49 a.m. UTC
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 <andreas.rheinhardt@gmail.com>
 libavformat/segment.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)
diff mbox series


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;
+    }
@@ -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)
     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 {
+            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);
     } else {