[FFmpeg-devel] [PATCH] avformat/librist: fix logging setting

[Zhao Zhili]

The librist logging API is confusing. It looks like a per instance
setting but saves a copy to global static variable quietly. So there
is a potential use-after-free issue with log_cb_arg.

librist took zero as invalid file descriptor at first. After the problem
was fixed, now it will close the zero file desscriptor. So log_socket
must be initialized to -1. See
https://code.videolan.org/rist/librist/-/issues/98
---
 libavformat/librist.c | 37 +++++++++++++++++++++++++++++++++++--
 1 file changed, 35 insertions(+), 2 deletions(-)

diff --git a/libavformat/librist.c b/libavformat/librist.c
index 01a3f9c122..46bb71cf87 100644
--- a/libavformat/librist.c
+++ b/libavformat/librist.c
@@ -24,6 +24,7 @@
 #include "libavutil/avassert.h"
 #include "libavutil/opt.h"
 #include "libavutil/parseutils.h"
+#include "libavutil/thread.h"
 #include "libavutil/time.h"
 
 #include "avformat.h"
@@ -99,6 +100,38 @@ static int log_cb(void *arg, enum rist_log_level log_level, const char *msg)
     return 0;
 }
 
+static void librist_set_global_log_callback(void)
+{
+    static struct rist_logging_settings logging_settings = {
+        .log_level = RIST_LOG_INFO,
+        .log_cb = log_cb,
+        .log_cb_arg = NULL,
+        .log_socket = -1,
+        .log_stream = NULL,
+    };
+    rist_logging_set_global(&logging_settings);
+}
+
+static int librist_setup_log(RISTContext *s, struct rist_logging_settings *logging_settings)
+{
+    int ret;
+    static AVOnce init_static_once = AV_ONCE_INIT;
+
+    if (!logging_settings)
+        return AVERROR(EINVAL);
+
+    // set global log callback first, otherwise rist_logging_set() will copy
+    // logging_settings to a global static variable, which can leads to
+    // use-after-free
+    ff_thread_once(&init_static_once, librist_set_global_log_callback);
+
+    logging_settings->log_socket = -1;
+    ret = rist_logging_set(&logging_settings, s->log_level, log_cb, s, NULL, NULL);
+    if (ret < 0)
+        return risterr2ret(ret);
+    return 0;
+}
+
 static int librist_close(URLContext *h)
 {
     RISTContext *s = h->priv_data;
@@ -123,9 +156,9 @@ static int librist_open(URLContext *h, const char *uri, int flags)
     if ((flags & AVIO_FLAG_READ_WRITE) == AVIO_FLAG_READ_WRITE)
         return AVERROR(EINVAL);
 
-    ret = rist_logging_set(&logging_settings, s->log_level, log_cb, h, NULL, NULL);
+    ret = librist_setup_log(s, logging_settings);
     if (ret < 0)
-        return risterr2ret(ret);
+        return ret;
 
     if (flags & AVIO_FLAG_WRITE) {
         h->max_packet_size = s->packet_size;
-- 
2.31.1