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}
This commit is contained in:
Michael Olbrich
2019-08-21 13:05:18 +02:00
committed by Commit Bot
parent c6d7d58c13
commit 63173d5bef
3 changed files with 41 additions and 22 deletions

View File

@ -95,6 +95,7 @@ Mozilla Foundation <*@mozilla.com>
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>

View File

@ -248,16 +248,22 @@ BaseCapturerPipeWire::~BaseCapturerPipeWire() {
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 @@ void BaseCapturerPipeWire::OnProxyRequested(GObject* /*object*/,
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 @@ void BaseCapturerPipeWire::SessionRequest() {
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 @@ void BaseCapturerPipeWire::OnSessionRequested(GDBusConnection* connection,
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 @@ void BaseCapturerPipeWire::SourcesRequest() {
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 @@ void BaseCapturerPipeWire::OnSourcesRequested(GDBusConnection* connection,
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 @@ void BaseCapturerPipeWire::StartRequest() {
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 @@ void BaseCapturerPipeWire::OnStartRequested(GDBusConnection* connection,
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 @@ void BaseCapturerPipeWire::OpenPipeWireRemote() {
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 @@ void BaseCapturerPipeWire::OnOpenPipeWireRemoteRequested(
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);

View File

@ -70,6 +70,7 @@ class BaseCapturerPipeWire : public DesktopCapturer {
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 @@ class BaseCapturerPipeWire : public DesktopCapturer {
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 @@ class BaseCapturerPipeWire : public DesktopCapturer {
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 @@ class BaseCapturerPipeWire : public DesktopCapturer {
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 @@ class BaseCapturerPipeWire : public DesktopCapturer {
gpointer user_data);
void OpenPipeWireRemote();
static void OnOpenPipeWireRemoteRequested(GDBusConnection* connection,
static void OnOpenPipeWireRemoteRequested(GDBusProxy *proxy,
GAsyncResult* result,
gpointer user_data);