[FFmpeg-devel] [PATCH] swscale/graph: fix double-free when legacy pass fails initializing (PR #20351)

[Niklas Haas via ffmpeg-devel <ffmpeg-devel@ffmpeg.org>]

PR #20351 opened by Niklas Haas (haasn)
URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/20351
Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/20351.patch

If this function returns an error after ff_sws_graph_add_pass() has been
called, and the pass->free callback is therefore already set up to free the
context, the graph will end up freed twice: once by the pass->free callback
(during ff_sws_graph_free()), and once before that by failure path of the
caller (e.g. add_legacy_sws_pass(), or init_legacy_subpass() itself for
cascaded contexts.)

The solution is to redefine the ownership of SwsGraph to pass clearly from
the caller of add_legacy_sws_pass() to init_legacy_subpass(), which can then
deal with appropriately freeing the context conditional on whether or not the
pass was already registered in the pass list.

Reported-by: 김영민 <kunshim@naver.com>
Signed-off-by: Niklas Haas <git@haasn.dev>


From f136e966e3afaae9d85efebeec4cfeda415084bb Mon Sep 17 00:00:00 2001
From: Niklas Haas <git@haasn.dev>
Date: Wed, 27 Aug 2025 12:14:45 +0200
Subject: [PATCH] swscale/graph: fix double-free when legacy pass fails
 initializing
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

If this function returns an error after ff_sws_graph_add_pass() has been
called, and the pass->free callback is therefore already set up to free the
context, the graph will end up freed twice: once by the pass->free callback
(during ff_sws_graph_free()), and once before that by failure path of the
caller (e.g. add_legacy_sws_pass(), or init_legacy_subpass() itself for
cascaded contexts.)

The solution is to redefine the ownership of SwsGraph to pass clearly from
the caller of add_legacy_sws_pass() to init_legacy_subpass(), which can then
deal with appropriately freeing the context conditional on whether or not the
pass was already registered in the pass list.

Reported-by: 김영민 <kunshim@naver.com>
Signed-off-by: Niklas Haas <git@haasn.dev>
---
 libswscale/graph.c | 33 ++++++++++++++++++---------------
 1 file changed, 18 insertions(+), 15 deletions(-)

diff --git a/libswscale/graph.c b/libswscale/graph.c
index 5bc524f1e3..9d46f8f27f 100644
--- a/libswscale/graph.c
+++ b/libswscale/graph.c
@@ -279,6 +279,7 @@ static void legacy_chr_pos(SwsGraph *graph, int *chr_pos, int override, int *war
     *chr_pos = override;
 }
 
+/* Takes over ownership of `sws` */
 static int init_legacy_subpass(SwsGraph *graph, SwsContext *sws,
                                SwsPass *input, SwsPass **output)
 {
@@ -293,17 +294,19 @@ static int init_legacy_subpass(SwsGraph *graph, SwsContext *sws,
     if (c->cascaded_context[0]) {
         const int num_cascaded = c->cascaded_context[2] ? 3 : 2;
         for (int i = 0; i < num_cascaded; i++) {
-            SwsContext *sub = c->cascaded_context[i];
             const int is_last = i + 1 == num_cascaded;
+
+            /* Steal cascaded context, so we can manage its lifetime independently */
+            SwsContext *sub = c->cascaded_context[i];
+            c->cascaded_context[i] = NULL;
+
             ret = init_legacy_subpass(graph, sub, input, is_last ? output : &input);
             if (ret < 0)
-                return ret;
-            /* Steal cascaded context, so we can free the parent */
-            c->cascaded_context[i] = NULL;
+                break;
         }
 
         sws_free_context(&sws);
-        return 0;
+        return ret;
     }
 
     if (sws->dither == SWS_DITHER_ED && !c->convert_unscaled)
@@ -311,20 +314,26 @@ static int init_legacy_subpass(SwsGraph *graph, SwsContext *sws,
 
     if (c->src0Alpha && !c->dst0Alpha && isALPHA(sws->dst_format)) {
         ret = pass_append(graph, AV_PIX_FMT_RGBA, src_w, src_h, &input, 1, c, run_rgb0);
-        if (ret < 0)
+        if (ret < 0) {
+            sws_free_context(&sws);
             return ret;
+        }
     }
 
     if (c->srcXYZ && !(c->dstXYZ && unscaled)) {
         ret = pass_append(graph, AV_PIX_FMT_RGB48, src_w, src_h, &input, 1, c, run_xyz2rgb);
-        if (ret < 0)
+        if (ret < 0) {
+            sws_free_context(&sws);
             return ret;
+        }
     }
 
     pass = ff_sws_graph_add_pass(graph, sws->dst_format, dst_w, dst_h, input, align, sws,
                                  c->convert_unscaled ? run_legacy_unscaled : run_legacy_swscale);
-    if (!pass)
+    if (!pass) {
+        sws_free_context(&sws);
         return AVERROR(ENOMEM);
+    }
     pass->setup = setup_legacy_swscale;
     pass->free = free_legacy_swscale;
 
@@ -444,13 +453,7 @@ static int add_legacy_sws_pass(SwsGraph *graph, SwsFormat src, SwsFormat dst,
                                 brightness, contrast, saturation);
     }
 
-    ret = init_legacy_subpass(graph, sws, input, output);
-    if (ret < 0) {
-        sws_free_context(&sws);
-        return ret;
-    }
-
-    return 0;
+    return init_legacy_subpass(graph, sws, input, output);
 }
 
 /**************************
-- 
2.49.1

_______________________________________________
ffmpeg-devel mailing list -- ffmpeg-devel@ffmpeg.org
To unsubscribe send an email to ffmpeg-devel-leave@ffmpeg.org