[FFmpeg-cvslog] avfilter/vf_overlay: Fix double-free of AVFilterFormats on error

[Andreas Rheinhardt]

ffmpeg | branch: master | Andreas Rheinhardt <andreas.rheinhardt at gmail.com> | Fri Aug  7 14:42:57 2020 +0200| [a86ee5fd79840dc4af3e3f5c90ff8ce19b9ae993] | committer: Andreas Rheinhardt

avfilter/vf_overlay: Fix double-free of AVFilterFormats on error

The query_formats function of the overlay filter tries to allocate
two lists (only one in a special case) of formats which on success
are attached to more permanent objects (AVFilterLinks) for storage
afterwards. If attaching a list to an AVFilterLink succeeds, it is
in turn owned by the AVFilterLink (or more exactly, the AVFilterLink
becomes one of the common owners of the list). Yet if attaching a list
to one of its links succeeds and an error happens lateron, both lists
were manually freed, whic is wrong if the list is already owned by one
or more links; these links' pointers to their lists will become dangling
and there will be a double-free/use-after-free when these links are
cleaned up automatically.

This commit fixes this by removing the custom freeing code; this will
temporarily add a leaking codepath (if attaching a list not already
owned by a link to a link fails, the list will leak), but this will
be fixed soon by making sure that an AVFilterFormats without owner will
be automatically freed when attaching it to an AVFilterLink fails.
Notice that at most one list leaks because a new list is only allocated
after the old list has been successfully attached to a link.

Reviewed-by: Nicolas George <george at nsup.org>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>

> http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=a86ee5fd79840dc4af3e3f5c90ff8ce19b9ae993
---

 libavfilter/vf_overlay.c | 84 +++++++++++++-----------------------------------
 1 file changed, 23 insertions(+), 61 deletions(-)

diff --git a/libavfilter/vf_overlay.c b/libavfilter/vf_overlay.c
index 79c654bb2d..5f6ca2b9e2 100644
--- a/libavfilter/vf_overlay.c
+++ b/libavfilter/vf_overlay.c
@@ -221,90 +221,52 @@ static int query_formats(AVFilterContext *ctx)
         AV_PIX_FMT_NONE
     };
 
-    AVFilterFormats *main_formats = NULL;
-    AVFilterFormats *overlay_formats = NULL;
+    const enum AVPixelFormat *main_formats, *overlay_formats;
+    AVFilterFormats *formats;
     int ret;
 
     switch (s->format) {
     case OVERLAY_FORMAT_YUV420:
-        if (!(main_formats    = ff_make_format_list(main_pix_fmts_yuv420)) ||
-            !(overlay_formats = ff_make_format_list(overlay_pix_fmts_yuv420))) {
-                ret = AVERROR(ENOMEM);
-                goto fail;
-            }
+        main_formats    = main_pix_fmts_yuv420;
+        overlay_formats = overlay_pix_fmts_yuv420;
         break;
     case OVERLAY_FORMAT_YUV420P10:
-        if (!(main_formats    = ff_make_format_list(main_pix_fmts_yuv420p10)) ||
-            !(overlay_formats = ff_make_format_list(overlay_pix_fmts_yuv420p10))) {
-            ret = AVERROR(ENOMEM);
-            goto fail;
-        }
+        main_formats    = main_pix_fmts_yuv420p10;
+        overlay_formats = overlay_pix_fmts_yuv420p10;
         break;
     case OVERLAY_FORMAT_YUV422:
-        if (!(main_formats    = ff_make_format_list(main_pix_fmts_yuv422)) ||
-            !(overlay_formats = ff_make_format_list(overlay_pix_fmts_yuv422))) {
-                ret = AVERROR(ENOMEM);
-                goto fail;
-            }
+        main_formats    = main_pix_fmts_yuv422;
+        overlay_formats = overlay_pix_fmts_yuv422;
         break;
     case OVERLAY_FORMAT_YUV422P10:
-        if (!(main_formats    = ff_make_format_list(main_pix_fmts_yuv422p10)) ||
-            !(overlay_formats = ff_make_format_list(overlay_pix_fmts_yuv422p10))) {
-                ret = AVERROR(ENOMEM);
-                goto fail;
-            }
+        main_formats    = main_pix_fmts_yuv422p10;
+        overlay_formats = overlay_pix_fmts_yuv422p10;
         break;
     case OVERLAY_FORMAT_YUV444:
-        if (!(main_formats    = ff_make_format_list(main_pix_fmts_yuv444)) ||
-            !(overlay_formats = ff_make_format_list(overlay_pix_fmts_yuv444))) {
-                ret = AVERROR(ENOMEM);
-                goto fail;
-            }
+        main_formats    = main_pix_fmts_yuv444;
+        overlay_formats = overlay_pix_fmts_yuv444;
         break;
     case OVERLAY_FORMAT_RGB:
-        if (!(main_formats    = ff_make_format_list(main_pix_fmts_rgb)) ||
-            !(overlay_formats = ff_make_format_list(overlay_pix_fmts_rgb))) {
-                ret = AVERROR(ENOMEM);
-                goto fail;
-            }
+        main_formats    = main_pix_fmts_rgb;
+        overlay_formats = overlay_pix_fmts_rgb;
         break;
     case OVERLAY_FORMAT_GBRP:
-        if (!(main_formats    = ff_make_format_list(main_pix_fmts_gbrp)) ||
-            !(overlay_formats = ff_make_format_list(overlay_pix_fmts_gbrp))) {
-                ret = AVERROR(ENOMEM);
-                goto fail;
-            }
+        main_formats    = main_pix_fmts_gbrp;
+        overlay_formats = overlay_pix_fmts_gbrp;
         break;
     case OVERLAY_FORMAT_AUTO:
-        if (!(main_formats    = ff_make_format_list(alpha_pix_fmts))) {
-                ret = AVERROR(ENOMEM);
-                goto fail;
-            }
-        break;
+        return ff_set_common_formats(ctx, ff_make_format_list(alpha_pix_fmts));
     default:
         av_assert0(0);
     }
 
-    if (s->format == OVERLAY_FORMAT_AUTO) {
-        ret = ff_set_common_formats(ctx, main_formats);
-        if (ret < 0)
-            goto fail;
-    } else {
-        if ((ret = ff_formats_ref(main_formats   , &ctx->inputs[MAIN]->out_formats   )) < 0 ||
-            (ret = ff_formats_ref(overlay_formats, &ctx->inputs[OVERLAY]->out_formats)) < 0 ||
-            (ret = ff_formats_ref(main_formats   , &ctx->outputs[MAIN]->in_formats   )) < 0)
-                goto fail;
-    }
+    formats = ff_make_format_list(main_formats);
+    if ((ret = ff_formats_ref(formats, &ctx->inputs[MAIN]->out_formats)) < 0 ||
+        (ret = ff_formats_ref(formats, &ctx->outputs[MAIN]->in_formats)) < 0)
+        return ret;
 
-    return 0;
-fail:
-    if (main_formats)
-        av_freep(&main_formats->formats);
-    av_freep(&main_formats);
-    if (overlay_formats)
-        av_freep(&overlay_formats->formats);
-    av_freep(&overlay_formats);
-    return ret;
+    return ff_formats_ref(ff_make_format_list(overlay_formats),
+                          &ctx->inputs[OVERLAY]->out_formats);
 }
 
 static int config_input_overlay(AVFilterLink *inlink)