pipewire: handle deleting the capturer while a D-Bus call is in progress
If a D-Bus call is in progress when a BaseCapturerPipeWire is deleted, then
the user_data is invalid when the callback function is called. This results
in memory corruption.
To fix this, use a GCancellable. If it is canceled, the callback will be
called with a corresponding error. Detect this error and abort before
accessing the user_data.
Note: The first argument is the 'source_object'. For g_dbus_proxy_call()
this is the proxy object not the connection. This was not a problem before,
because it was not used.
Bug: None
Change-Id: I8d5e3fb5c49fcc9afd61cdb8e8249f78b9434faf
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/149817
Reviewed-by: Jamie Walch <jamiewalch@chromium.org>
Commit-Queue: Jamie Walch <jamiewalch@chromium.org>
Cr-Commit-Position: refs/heads/master@{#29326}
diff --git a/AUTHORS b/AUTHORS
index fbf9353..ac4d742 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -95,6 +95,7 @@
NVIDIA Corporation <*@nvidia.com>
Opera Software ASA <*@opera.com>
Optical Tone Ltd <*@opticaltone.com>
+Pengutronix e.K. <*@pengutronix.de>
Sinch AB <*@sinch.com>
struktur AG <*@struktur.de>
Telenor Digital AS <*@telenor.com>
diff --git a/modules/desktop_capture/linux/base_capturer_pipewire.cc b/modules/desktop_capture/linux/base_capturer_pipewire.cc
index 46a4aea..e4f7d86 100644
--- a/modules/desktop_capture/linux/base_capturer_pipewire.cc
+++ b/modules/desktop_capture/linux/base_capturer_pipewire.cc
@@ -248,16 +248,22 @@
g_free(session_handle_);
g_free(portal_handle_);
+ if (cancellable_) {
+ g_cancellable_cancel(cancellable_);
+ g_clear_object(&cancellable_);
+ }
+
if (proxy_) {
g_clear_object(&proxy_);
}
}
void BaseCapturerPipeWire::InitPortal() {
+ cancellable_ = g_cancellable_new();
g_dbus_proxy_new_for_bus(
G_BUS_TYPE_SESSION, G_DBUS_PROXY_FLAGS_NONE, /*info=*/nullptr,
kDesktopBusName, kDesktopObjectPath, kScreenCastInterfaceName,
- /*cancellable=*/nullptr,
+ cancellable_,
reinterpret_cast<GAsyncReadyCallback>(OnProxyRequested), this);
}
@@ -434,14 +440,17 @@
RTC_DCHECK(that);
GError* error = nullptr;
- that->proxy_ = g_dbus_proxy_new_finish(result, &error);
- if (!that->proxy_) {
+ GDBusProxy *proxy = g_dbus_proxy_new_finish(result, &error);
+ if (!proxy) {
+ if (g_error_matches(error, G_IO_ERROR, G_IO_ERROR_CANCELLED))
+ return;
RTC_LOG(LS_ERROR) << "Failed to create a proxy for the screen cast portal: "
<< error->message;
g_error_free(error);
that->portal_init_failed_ = true;
return;
}
+ that->proxy_ = proxy;
that->connection_ = g_dbus_proxy_get_connection(that->proxy_);
RTC_LOG(LS_INFO) << "Created proxy for the screen cast portal.";
@@ -487,20 +496,22 @@
RTC_LOG(LS_INFO) << "Screen cast session requested.";
g_dbus_proxy_call(
proxy_, "CreateSession", g_variant_new("(a{sv})", &builder),
- G_DBUS_CALL_FLAGS_NONE, /*timeout=*/-1, /*cancellable=*/nullptr,
+ G_DBUS_CALL_FLAGS_NONE, /*timeout=*/-1, cancellable_,
reinterpret_cast<GAsyncReadyCallback>(OnSessionRequested), this);
}
// static
-void BaseCapturerPipeWire::OnSessionRequested(GDBusConnection* connection,
+void BaseCapturerPipeWire::OnSessionRequested(GDBusProxy *proxy,
GAsyncResult* result,
gpointer user_data) {
BaseCapturerPipeWire* that = static_cast<BaseCapturerPipeWire*>(user_data);
RTC_DCHECK(that);
GError* error = nullptr;
- GVariant* variant = g_dbus_proxy_call_finish(that->proxy_, result, &error);
+ GVariant* variant = g_dbus_proxy_call_finish(proxy, result, &error);
if (!variant) {
+ if (g_error_matches(error, G_IO_ERROR, G_IO_ERROR_CANCELLED))
+ return;
RTC_LOG(LS_ERROR) << "Failed to create a screen cast session: "
<< error->message;
g_error_free(error);
@@ -515,7 +526,7 @@
if (!handle) {
RTC_LOG(LS_ERROR) << "Failed to initialize the screen cast session.";
if (that->session_request_signal_id_) {
- g_dbus_connection_signal_unsubscribe(connection,
+ g_dbus_connection_signal_unsubscribe(that->connection_,
that->session_request_signal_id_);
that->session_request_signal_id_ = 0;
}
@@ -584,20 +595,22 @@
g_dbus_proxy_call(
proxy_, "SelectSources",
g_variant_new("(oa{sv})", session_handle_, &builder),
- G_DBUS_CALL_FLAGS_NONE, /*timeout=*/-1, /*cancellable=*/nullptr,
+ G_DBUS_CALL_FLAGS_NONE, /*timeout=*/-1, cancellable_,
reinterpret_cast<GAsyncReadyCallback>(OnSourcesRequested), this);
}
// static
-void BaseCapturerPipeWire::OnSourcesRequested(GDBusConnection* connection,
+void BaseCapturerPipeWire::OnSourcesRequested(GDBusProxy *proxy,
GAsyncResult* result,
gpointer user_data) {
BaseCapturerPipeWire* that = static_cast<BaseCapturerPipeWire*>(user_data);
RTC_DCHECK(that);
GError* error = nullptr;
- GVariant* variant = g_dbus_proxy_call_finish(that->proxy_, result, &error);
+ GVariant* variant = g_dbus_proxy_call_finish(proxy, result, &error);
if (!variant) {
+ if (g_error_matches(error, G_IO_ERROR, G_IO_ERROR_CANCELLED))
+ return;
RTC_LOG(LS_ERROR) << "Failed to request the sources: " << error->message;
g_error_free(error);
that->portal_init_failed_ = true;
@@ -612,7 +625,7 @@
if (!handle) {
RTC_LOG(LS_ERROR) << "Failed to initialize the screen cast session.";
if (that->sources_request_signal_id_) {
- g_dbus_connection_signal_unsubscribe(connection,
+ g_dbus_connection_signal_unsubscribe(that->connection_,
that->sources_request_signal_id_);
that->sources_request_signal_id_ = 0;
}
@@ -672,20 +685,22 @@
g_dbus_proxy_call(
proxy_, "Start",
g_variant_new("(osa{sv})", session_handle_, parent_window, &builder),
- G_DBUS_CALL_FLAGS_NONE, /*timeout=*/-1, /*cancellable=*/nullptr,
+ G_DBUS_CALL_FLAGS_NONE, /*timeout=*/-1, cancellable_,
reinterpret_cast<GAsyncReadyCallback>(OnStartRequested), this);
}
// static
-void BaseCapturerPipeWire::OnStartRequested(GDBusConnection* connection,
+void BaseCapturerPipeWire::OnStartRequested(GDBusProxy *proxy,
GAsyncResult* result,
gpointer user_data) {
BaseCapturerPipeWire* that = static_cast<BaseCapturerPipeWire*>(user_data);
RTC_DCHECK(that);
GError* error = nullptr;
- GVariant* variant = g_dbus_proxy_call_finish(that->proxy_, result, &error);
+ GVariant* variant = g_dbus_proxy_call_finish(proxy, result, &error);
if (!variant) {
+ if (g_error_matches(error, G_IO_ERROR, G_IO_ERROR_CANCELLED))
+ return;
RTC_LOG(LS_ERROR) << "Failed to start the screen cast session: "
<< error->message;
g_error_free(error);
@@ -702,7 +717,7 @@
RTC_LOG(LS_ERROR)
<< "Failed to initialize the start of the screen cast session.";
if (that->start_request_signal_id_) {
- g_dbus_connection_signal_unsubscribe(connection,
+ g_dbus_connection_signal_unsubscribe(that->connection_,
that->start_request_signal_id_);
that->start_request_signal_id_ = 0;
}
@@ -777,14 +792,14 @@
proxy_, "OpenPipeWireRemote",
g_variant_new("(oa{sv})", session_handle_, &builder),
G_DBUS_CALL_FLAGS_NONE, /*timeout=*/-1, /*fd_list=*/nullptr,
- /*cancellable=*/nullptr,
+ cancellable_,
reinterpret_cast<GAsyncReadyCallback>(OnOpenPipeWireRemoteRequested),
this);
}
// static
void BaseCapturerPipeWire::OnOpenPipeWireRemoteRequested(
- GDBusConnection* connection,
+ GDBusProxy *proxy,
GAsyncResult* result,
gpointer user_data) {
BaseCapturerPipeWire* that = static_cast<BaseCapturerPipeWire*>(user_data);
@@ -793,8 +808,10 @@
GError* error = nullptr;
GUnixFDList* outlist = nullptr;
GVariant* variant = g_dbus_proxy_call_with_unix_fd_list_finish(
- that->proxy_, &outlist, result, &error);
+ proxy, &outlist, result, &error);
if (!variant) {
+ if (g_error_matches(error, G_IO_ERROR, G_IO_ERROR_CANCELLED))
+ return;
RTC_LOG(LS_ERROR) << "Failed to open the PipeWire remote: "
<< error->message;
g_error_free(error);
diff --git a/modules/desktop_capture/linux/base_capturer_pipewire.h b/modules/desktop_capture/linux/base_capturer_pipewire.h
index d7910aa..f28d7a5 100644
--- a/modules/desktop_capture/linux/base_capturer_pipewire.h
+++ b/modules/desktop_capture/linux/base_capturer_pipewire.h
@@ -70,6 +70,7 @@
GDBusConnection* connection_ = nullptr;
GDBusProxy* proxy_ = nullptr;
+ GCancellable *cancellable_ = nullptr;
gchar* portal_handle_ = nullptr;
gchar* session_handle_ = nullptr;
gchar* sources_handle_ = nullptr;
@@ -119,7 +120,7 @@
const gchar* token);
void SessionRequest();
- static void OnSessionRequested(GDBusConnection* connection,
+ static void OnSessionRequested(GDBusProxy *proxy,
GAsyncResult* result,
gpointer user_data);
static void OnSessionRequestResponseSignal(GDBusConnection* connection,
@@ -131,7 +132,7 @@
gpointer user_data);
void SourcesRequest();
- static void OnSourcesRequested(GDBusConnection* connection,
+ static void OnSourcesRequested(GDBusProxy *proxy,
GAsyncResult* result,
gpointer user_data);
static void OnSourcesRequestResponseSignal(GDBusConnection* connection,
@@ -143,7 +144,7 @@
gpointer user_data);
void StartRequest();
- static void OnStartRequested(GDBusConnection* connection,
+ static void OnStartRequested(GDBusProxy *proxy,
GAsyncResult* result,
gpointer user_data);
static void OnStartRequestResponseSignal(GDBusConnection* connection,
@@ -155,7 +156,7 @@
gpointer user_data);
void OpenPipeWireRemote();
- static void OnOpenPipeWireRemoteRequested(GDBusConnection* connection,
+ static void OnOpenPipeWireRemoteRequested(GDBusProxy *proxy,
GAsyncResult* result,
gpointer user_data);
