[FFmpeg-devel] [PATCH v5 2/3] avformat/tee: Fix leaks in tee muxer when open_slave fails

[sebechlebskyjan at gmail.com]

From: Jan Sebechlebsky <sebechlebskyjan at gmail.com>

Calling close_slave in case error is to be returned from open_slave
will free allocated resources.

Since failure can happen before bsfs array is initialized,
close_slave must check that bsfs is not NULL before accessing
tee_slave->bsfs[i] element.

Slave muxer expects write_trailer to be called if it's
write_header suceeded (so resources allocated in write_header
are freed). Therefore if failure happens after successfull
write_header call, we must ensure that write_trailer of
that particular slave is called.

Signed-off-by: Jan Sebechlebsky <sebechlebskyjan at gmail.com>
---
 libavformat/tee.c | 44 +++++++++++++++++++++++++++++---------------
 1 file changed, 29 insertions(+), 15 deletions(-)

diff --git a/libavformat/tee.c b/libavformat/tee.c
index ab6cd32..222826a 100644
--- a/libavformat/tee.c
+++ b/libavformat/tee.c
@@ -141,12 +141,17 @@ static void close_slave(TeeSlave *tee_slave)
     unsigned i;
 
     avf = tee_slave->avf;
-    for (i = 0; i < avf->nb_streams; ++i) {
-        AVBitStreamFilterContext *bsf_next, *bsf = tee_slave->bsfs[i];
-        while (bsf) {
-            bsf_next = bsf->next;
-            av_bitstream_filter_close(bsf);
-            bsf = bsf_next;
+    if (!avf)
+        return;
+
+    if (tee_slave->bsfs) {
+        for (i = 0; i < avf->nb_streams; ++i) {
+            AVBitStreamFilterContext *bsf_next, *bsf = tee_slave->bsfs[i];
+            while (bsf) {
+                bsf_next = bsf->next;
+                av_bitstream_filter_close(bsf);
+                bsf = bsf_next;
+            }
         }
     }
     av_freep(&tee_slave->stream_map);
@@ -197,6 +202,7 @@ static int open_slave(AVFormatContext *avf, char *slave, TeeSlave *tee_slave)
     ret = avformat_alloc_output_context2(&avf2, NULL, format, filename);
     if (ret < 0)
         goto end;
+    tee_slave->avf = avf2;
     av_dict_copy(&avf2->metadata, avf->metadata, 0);
     avf2->opaque   = avf->opaque;
     avf2->io_open  = avf->io_open;
@@ -276,11 +282,10 @@ static int open_slave(AVFormatContext *avf, char *slave, TeeSlave *tee_slave)
         goto end;
     }
 
-    tee_slave->avf = avf2;
     tee_slave->bsfs = av_calloc(avf2->nb_streams, sizeof(TeeSlave));
     if (!tee_slave->bsfs) {
         ret = AVERROR(ENOMEM);
-        goto end;
+        goto fail;
     }
 
     entry = NULL;
@@ -291,7 +296,8 @@ static int open_slave(AVFormatContext *avf, char *slave, TeeSlave *tee_slave)
                 av_log(avf, AV_LOG_ERROR,
                        "Specifier separator in '%s' is '%c', but only characters '%s' "
                        "are allowed\n", entry->key, *spec, slave_bsfs_spec_sep);
-                return AVERROR(EINVAL);
+                ret = AVERROR(EINVAL);
+                goto fail;
             }
             spec++; /* consume separator */
         }
@@ -302,7 +308,7 @@ static int open_slave(AVFormatContext *avf, char *slave, TeeSlave *tee_slave)
                 av_log(avf, AV_LOG_ERROR,
                        "Invalid stream specifier '%s' in bsfs option '%s' for slave "
                        "output '%s'\n", spec, entry->key, filename);
-                goto end;
+                goto fail;
             }
 
             if (ret > 0) {
@@ -319,7 +325,7 @@ static int open_slave(AVFormatContext *avf, char *slave, TeeSlave *tee_slave)
                     av_log(avf, AV_LOG_ERROR,
                            "Error parsing bitstream filter sequence '%s' associated to "
                            "stream %d of slave output '%s'\n", entry->value, i, filename);
-                    goto end;
+                    goto fail;
                 }
             }
         }
@@ -332,7 +338,7 @@ static int open_slave(AVFormatContext *avf, char *slave, TeeSlave *tee_slave)
         while ((entry = av_dict_get(options, "", entry, AV_DICT_IGNORE_SUFFIX)))
             av_log(avf2, AV_LOG_ERROR, "Unknown option '%s'\n", entry->key);
         ret = AVERROR_OPTION_NOT_FOUND;
-        goto end;
+        goto fail;
     }
 
 end:
@@ -341,6 +347,9 @@ end:
     av_dict_free(&options);
     av_freep(&tmp_select);
     return ret;
+fail:
+    av_write_trailer(avf2);
+    goto end;
 }
 
 static void log_slave(TeeSlave *slave, void *log_ctx, int log_level)
@@ -390,15 +399,18 @@ static int tee_write_header(AVFormatContext *avf)
             filename++;
     }
 
+    tee->nb_slaves = 0;
+
     for (i = 0; i < nb_slaves; i++) {
-        if ((ret = open_slave(avf, slaves[i], &tee->slaves[i])) < 0)
+        if ((ret = open_slave(avf, slaves[i], &tee->slaves[i])) < 0) {
+            close_slave(&tee->slaves[i]);
             goto fail;
+        }
         log_slave(&tee->slaves[i], avf, AV_LOG_VERBOSE);
         av_freep(&slaves[i]);
+        tee->nb_slaves++;
     }
 
-    tee->nb_slaves = nb_slaves;
-
     for (i = 0; i < avf->nb_streams; i++) {
         int j, mapped = 0;
         for (j = 0; j < tee->nb_slaves; j++)
@@ -412,6 +424,8 @@ static int tee_write_header(AVFormatContext *avf)
 fail:
     for (i = 0; i < nb_slaves; i++)
         av_freep(&slaves[i]);
+    for (i = 0; i < tee->nb_slaves; i++)
+        av_write_trailer(tee->slaves[i].avf);
     close_slaves(avf);
     return ret;
 }
-- 
1.9.1