luc14n0/glib
1
0
Fork 0
mirror of https://gitlab.gnome.org/GNOME/glib.git synced 2025-04-26 00:56:53 +02:00
Code Issues Packages Projects Releases Wiki Activity

gdbus: Fix race in name watching on connection teardown

Browse Source 
If g_dbus_unwatch_name() is called from one thread at the same time as
the GDBusConnection is emitting ::disconnected in another thread, there
will be a race and the handler for ::disconnected may end up using
memory after it’s freed.

Fix this by serialising through the map_id_to_client, so that
on_connection_disconnected() atomically gets a strong reference to the
Client, or NULL.

https://bugzilla.gnome.org/show_bug.cgi?id=777307
This commit is contained in:
Philip Withnall 2017-02-06 09:41:10 +01:00
parent 97068f363e
commit 4dd1b17c24
1 changed files with 43 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

48
gio/gdbusnamewatching.c
View File

@ -249,13 +249,43 @@ call_vanished_handler (Client *client,
/* ---------------------------------------------------------------------------------------------------- */ /* ---------------------------------------------------------------------------------------------------- */
/* Return a reference to the #Client for @watcher_id, or %NULL if it’s been
* unwatched. This is safe to call from any thread. */
static Client *
dup_client (guint watcher_id)
{
Client *client;
G_LOCK (lock);
g_assert (watcher_id != 0);
g_assert (map_id_to_client != NULL);
client = g_hash_table_lookup (map_id_to_client, GUINT_TO_POINTER (watcher_id));
if (client != NULL)
client_ref (client);
G_UNLOCK (lock);
return client;
}
/* Could be called from any thread, so it could be called after client_unref()
* has started finalising the #Client. Avoid that by looking up the #Client
* atomically. */
static void static void
on_connection_disconnected (GDBusConnection *connection, on_connection_disconnected (GDBusConnection *connection,
gboolean remote_peer_vanished, gboolean remote_peer_vanished,
GError *error, GError *error,
gpointer user_data) gpointer user_data)
{ {
Client *client = user_data; guint watcher_id = GPOINTER_TO_UINT (user_data);
Client *client = NULL;
client = dup_client (watcher_id);
if (client == NULL)
return;
if (client->name_owner_changed_subscription_id > 0) if (client->name_owner_changed_subscription_id > 0)
g_dbus_connection_signal_unsubscribe (client->connection, client->name_owner_changed_subscription_id); g_dbus_connection_signal_unsubscribe (client->connection, client->name_owner_changed_subscription_id);
@ -267,10 +297,13 @@ on_connection_disconnected (GDBusConnection *connection,
client->connection = NULL; client->connection = NULL;
call_vanished_handler (client, FALSE); call_vanished_handler (client, FALSE);
client_unref (client);
} }
/* ---------------------------------------------------------------------------------------------------- */ /* ---------------------------------------------------------------------------------------------------- */
/* Will always be called from the thread which acquired client->main_context. */
static void static void
on_name_owner_changed (GDBusConnection *connection, on_name_owner_changed (GDBusConnection *connection,
const gchar *sender_name, const gchar *sender_name,
@ -280,11 +313,16 @@ on_name_owner_changed (GDBusConnection *connection,
GVariant *parameters, GVariant *parameters,
gpointer user_data) gpointer user_data)
{ {
Client *client = user_data; guint watcher_id = GPOINTER_TO_UINT (user_data);
Client *client = NULL;
const gchar *name; const gchar *name;
const gchar *old_owner; const gchar *old_owner;
const gchar *new_owner; const gchar *new_owner;
client = dup_client (watcher_id);
if (client == NULL)
return;
if (!client->initialized) if (!client->initialized)
goto out; goto out;
@ -319,7 +357,7 @@ on_name_owner_changed (GDBusConnection *connection,
} }
out: out:
; client_unref (client);
} }
/* ---------------------------------------------------------------------------------------------------- */ /* ---------------------------------------------------------------------------------------------------- */
@ -444,7 +482,7 @@ has_connection (Client *client)
client->disconnected_signal_handler_id = g_signal_connect (client->connection, client->disconnected_signal_handler_id = g_signal_connect (client->connection,
"closed", "closed",
G_CALLBACK (on_connection_disconnected), G_CALLBACK (on_connection_disconnected),
client); GUINT_TO_POINTER (client->id));
/* start listening to NameOwnerChanged messages immediately */ /* start listening to NameOwnerChanged messages immediately */
client->name_owner_changed_subscription_id = g_dbus_connection_signal_subscribe (client->connection, client->name_owner_changed_subscription_id = g_dbus_connection_signal_subscribe (client->connection,
@ -455,7 +493,7 @@ has_connection (Client *client)
client->name, client->name,
G_DBUS_SIGNAL_FLAGS_NONE, G_DBUS_SIGNAL_FLAGS_NONE,
on_name_owner_changed, on_name_owner_changed,
client, GUINT_TO_POINTER (client->id),
NULL); NULL);
if (client->flags & G_BUS_NAME_WATCHER_FLAGS_AUTO_START) if (client->flags & G_BUS_NAME_WATCHER_FLAGS_AUTO_START)