From e42a8d537f575cf61747e9f67c5a9e34f511cb2b Mon Sep 17 00:00:00 2001 From: der richter Date: Wed, 10 Apr 2024 19:41:38 +0200 Subject: [PATCH] mac/log: fix use after free when freeing mpv handle the mp_log is freed when the corresponding mpv_handle (ta_parent) is freed in the EventHelper, though it could still be used from different threads. instead don't use a ta_parent and manually free on manual dereferencing. on app shutdown (memory cleanup) this is not called but instead is freed by the usual cleanup and freeing. the LogHelper is only able to be manually dereferenced in the AppHub, so no race conditions are possible in all other cases (vo).in the AppHub it's impossible to hit a race condition atm, because of how the init process works and how/where the log is used. only manually forcing logging in the exit process itself could theoretically trigger a use after free. Fixes #13823 --- osdep/mac/app_hub.swift | 2 +- osdep/mac/log_helper.swift | 8 +++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/osdep/mac/app_hub.swift b/osdep/mac/app_hub.swift index e86e707dd3..997cc33847 100644 --- a/osdep/mac/app_hub.swift +++ b/osdep/mac/app_hub.swift @@ -55,7 +55,7 @@ class AppHub: NSObject { event = EventHelper(self, mpv) if let mpv = event?.mpv { self.mpv = mpv - log.log = mp_log_new(UnsafeMutablePointer(mpv), mp_client_get_log(mpv), "app") + log.log = mp_log_new(nil, mp_client_get_log(mpv), "app") option = OptionHelper(UnsafeMutablePointer(mpv), mp_client_get_global(mpv)) input.option = option } diff --git a/osdep/mac/log_helper.swift b/osdep/mac/log_helper.swift index 0f65975da1..3d349a487c 100644 --- a/osdep/mac/log_helper.swift +++ b/osdep/mac/log_helper.swift @@ -50,7 +50,7 @@ class LogHelper { } func send(message: String, type: Int) { - guard let log = log, UnsafeRawPointer(log).load(as: UInt8.self) != 0 else { + guard let log = log else { logger.log(level: loggerMapping[type] ?? .default, "\(message, privacy: .public)") return } @@ -58,4 +58,10 @@ class LogHelper { let args: [CVarArg] = [(message as NSString).utf8String ?? "NO MESSAGE"] mp_msg_va(log, Int32(type), "%s\n", getVaList(args)) } + + deinit { + // only a manual dereferencing will trigger this, cleanup properly in that case + ta_free(UnsafeMutablePointer(log)) + log = nil + } }