mirror of
https://gitlab.gnome.org/GNOME/glib.git
synced 2024-11-10 03:16:17 +01:00
gdbus: Track name owners for signal subscriptions
We will use this in a subsequent commit to prevent signals from an impostor from being delivered to a subscriber. To avoid message reordering leading to misleading situations, this does not use the existing mechanism for watching bus name ownership, which delivers the ownership changes to other main-contexts. Instead, it all happens on the single thread used by the GDBusWorker, so the order in which messages are received is the order in which they are processed. Signed-off-by: Simon McVittie <smcv@collabora.com>
This commit is contained in:
Simon McVittie
parent
26a3fb8518
commit
683b14b981
@ -324,6 +324,31 @@ signal_subscriber_unref (SignalSubscriber *subscriber)
|
||||
}
|
||||
|
||||
typedef struct
|
||||
{
|
||||
/*
|
||||
* 1 reference while waiting for GetNameOwner() to finish
|
||||
* 1 reference for each SignalData that points to this one as its
|
||||
* shared_name_watcher
|
||||
*/
|
||||
grefcount ref_count;
|
||||
|
||||
gchar *owner;
|
||||
guint32 get_name_owner_serial;
|
||||
} WatchedName;
|
||||
|
||||
static WatchedName *
|
||||
watched_name_new (void)
|
||||
{
|
||||
WatchedName *watched_name = g_new0 (WatchedName, 1);
|
||||
|
||||
g_ref_count_init (&watched_name->ref_count);
|
||||
watched_name->owner = NULL;
|
||||
return g_steal_pointer (&watched_name);
|
||||
}
|
||||
|
||||
typedef struct SignalData SignalData;
|
||||
|
||||
struct SignalData
|
||||
{
|
||||
gchar *rule;
|
||||
gchar *sender;
|
||||
@ -333,13 +358,36 @@ typedef struct
|
||||
gchar *arg0;
|
||||
GDBusSignalFlags flags;
|
||||
GPtrArray *subscribers; /* (owned) (element-type SignalSubscriber) */
|
||||
gboolean sender_is_its_own_owner;
|
||||
} SignalData;
|
||||
|
||||
/*
|
||||
* If the sender is a well-known name, this is an unowned SignalData
|
||||
* representing the NameOwnerChanged signal that tracks its owner.
|
||||
* NULL if sender is NULL.
|
||||
* NULL if sender is its own owner (a unique name or DBUS_SERVICE_DBUS).
|
||||
*
|
||||
* Invariants: if not NULL, then
|
||||
* shared_name_watcher->sender == DBUS_SERVICE_DBUS
|
||||
* shared_name_watcher->interface_name == DBUS_INTERFACE_DBUS
|
||||
* shared_name_watcher->member == "NameOwnerChanged"
|
||||
* shared_name_watcher->object_path == DBUS_PATH_DBUS
|
||||
* shared_name_watcher->arg0 == sender
|
||||
* shared_name_watcher->flags == NONE
|
||||
* shared_name_watcher->watched_name == NULL
|
||||
*/
|
||||
SignalData *shared_name_watcher;
|
||||
|
||||
/*
|
||||
* Non-NULL if this SignalData is another SignalData's shared_name_watcher.
|
||||
* One reference for each SignalData that has this one as its
|
||||
* shared_name_watcher.
|
||||
* Otherwise NULL.
|
||||
*/
|
||||
WatchedName *watched_name;
|
||||
};
|
||||
|
||||
static SignalData *
|
||||
signal_data_new_take (gchar *rule,
|
||||
gchar *sender,
|
||||
gboolean sender_is_its_own_owner,
|
||||
gchar *interface_name,
|
||||
gchar *member,
|
||||
gchar *object_path,
|
||||
@ -350,7 +398,6 @@ signal_data_new_take (gchar *rule,
|
||||
|
||||
signal_data->rule = rule;
|
||||
signal_data->sender = sender;
|
||||
signal_data->sender_is_its_own_owner = sender_is_its_own_owner;
|
||||
signal_data->interface_name = interface_name;
|
||||
signal_data->member = member;
|
||||
signal_data->object_path = object_path;
|
||||
@ -363,6 +410,17 @@ signal_data_new_take (gchar *rule,
|
||||
static void
|
||||
signal_data_free (SignalData *signal_data)
|
||||
{
|
||||
/* The SignalData should not be freed while it still has subscribers */
|
||||
g_assert (signal_data->subscribers->len == 0);
|
||||
|
||||
/* The SignalData should not be freed while it is watching for
|
||||
* NameOwnerChanged on behalf of another SignalData */
|
||||
g_assert (signal_data->watched_name == NULL);
|
||||
|
||||
/* The SignalData should be detached from its name watcher, if any,
|
||||
* before it is freed */
|
||||
g_assert (signal_data->shared_name_watcher == NULL);
|
||||
|
||||
g_free (signal_data->rule);
|
||||
g_free (signal_data->sender);
|
||||
g_free (signal_data->interface_name);
|
||||
@ -370,6 +428,7 @@ signal_data_free (SignalData *signal_data)
|
||||
g_free (signal_data->object_path);
|
||||
g_free (signal_data->arg0);
|
||||
g_ptr_array_unref (signal_data->subscribers);
|
||||
|
||||
g_free (signal_data);
|
||||
}
|
||||
|
||||
@ -493,6 +552,7 @@ struct _GDBusConnection
|
||||
|
||||
/* Map used for managing method replies, protected by @lock */
|
||||
GHashTable *map_method_serial_to_task; /* guint32 -> owned GTask* */
|
||||
GHashTable *map_method_serial_to_name_watcher; /* guint32 -> unowned SignalData* */
|
||||
|
||||
/* Maps used for managing signal subscription, protected by @lock */
|
||||
GHashTable *map_rule_to_signal_data; /* match rule (gchar*) -> SignalData */
|
||||
@ -741,6 +801,7 @@ g_dbus_connection_finalize (GObject *object)
|
||||
g_error_free (connection->initialization_error);
|
||||
|
||||
g_hash_table_unref (connection->map_method_serial_to_task);
|
||||
g_hash_table_unref (connection->map_method_serial_to_name_watcher);
|
||||
|
||||
g_hash_table_unref (connection->map_rule_to_signal_data);
|
||||
g_hash_table_unref (connection->map_id_to_signal_data);
|
||||
@ -1127,6 +1188,7 @@ g_dbus_connection_init (GDBusConnection *connection)
|
||||
g_mutex_init (&connection->init_lock);
|
||||
|
||||
connection->map_method_serial_to_task = g_hash_table_new_full (g_direct_hash, g_direct_equal, NULL, g_object_unref);
|
||||
connection->map_method_serial_to_name_watcher = g_hash_table_new_full (g_direct_hash, g_direct_equal, NULL, NULL);
|
||||
|
||||
connection->map_rule_to_signal_data = g_hash_table_new (g_str_hash,
|
||||
g_str_equal);
|
||||
@ -2254,6 +2316,191 @@ g_dbus_connection_send_message_with_reply_sync (GDBusConnection *connecti
|
||||
|
||||
/* ---------------------------------------------------------------------------------------------------- */
|
||||
|
||||
/*
|
||||
* Called in any thread.
|
||||
* Must hold the connection lock when calling this, unless
|
||||
* connection->finalizing is TRUE.
|
||||
*/
|
||||
static void
|
||||
name_watcher_unref_watched_name (GDBusConnection *connection,
|
||||
SignalData *name_watcher)
|
||||
{
|
||||
WatchedName *watched_name = name_watcher->watched_name;
|
||||
|
||||
g_assert (watched_name != NULL);
|
||||
|
||||
if (!g_ref_count_dec (&watched_name->ref_count))
|
||||
return;
|
||||
|
||||
/* Removing watched_name from the name_watcher may result in
|
||||
* name_watcher being freed, so we must make sure name_watcher is no
|
||||
* longer in map_method_serial_to_name_watcher.
|
||||
*
|
||||
* If we stop watching the name while our GetNameOwner call was still
|
||||
* in-flight, then when the reply eventually arrives, we will not find
|
||||
* its serial number in the map and harmlessly ignore it as a result. */
|
||||
if (watched_name->get_name_owner_serial != 0)
|
||||
g_hash_table_remove (connection->map_method_serial_to_name_watcher,
|
||||
GUINT_TO_POINTER (watched_name->get_name_owner_serial));
|
||||
|
||||
name_watcher->watched_name = NULL;
|
||||
g_free (watched_name->owner);
|
||||
g_free (watched_name);
|
||||
}
|
||||
|
||||
/* called in GDBusWorker thread with lock held */
|
||||
static void
|
||||
name_watcher_set_name_owner_unlocked (SignalData *name_watcher,
|
||||
const char *new_owner)
|
||||
{
|
||||
if (new_owner != NULL && new_owner[0] == '\0')
|
||||
new_owner = NULL;
|
||||
|
||||
g_assert (name_watcher->watched_name != NULL);
|
||||
g_set_str (&name_watcher->watched_name->owner, new_owner);
|
||||
}
|
||||
|
||||
/* called in GDBusWorker thread with lock held */
|
||||
static void
|
||||
name_watcher_deliver_name_owner_changed_unlocked (SignalData *name_watcher,
|
||||
GDBusMessage *message)
|
||||
{
|
||||
GVariant *body;
|
||||
|
||||
body = g_dbus_message_get_body (message);
|
||||
|
||||
if (G_LIKELY (body != NULL && g_variant_is_of_type (body, G_VARIANT_TYPE ("(sss)"))))
|
||||
{
|
||||
const char *name;
|
||||
const char *new_owner;
|
||||
|
||||
g_variant_get (body, "(&s&s&s)", &name, NULL, &new_owner);
|
||||
|
||||
/* Our caller already checked this */
|
||||
g_assert (g_strcmp0 (name_watcher->arg0, name) == 0);
|
||||
|
||||
if (G_LIKELY (new_owner[0] == '\0' || g_dbus_is_unique_name (new_owner)))
|
||||
name_watcher_set_name_owner_unlocked (name_watcher, new_owner);
|
||||
else
|
||||
g_warning ("Received NameOwnerChanged signal with invalid owner \"%s\" for \"%s\"",
|
||||
new_owner, name);
|
||||
}
|
||||
else
|
||||
{
|
||||
g_warning ("Received NameOwnerChanged signal with unexpected "
|
||||
"signature %s",
|
||||
body == NULL ? "()" : g_variant_get_type_string (body));
|
||||
|
||||
}
|
||||
}
|
||||
|
||||
/* called in GDBusWorker thread with lock held */
|
||||
static void
|
||||
name_watcher_deliver_get_name_owner_reply_unlocked (SignalData *name_watcher,
|
||||
GDBusConnection *connection,
|
||||
GDBusMessage *message)
|
||||
{
|
||||
GDBusMessageType type;
|
||||
GVariant *body;
|
||||
WatchedName *watched_name;
|
||||
|
||||
watched_name = name_watcher->watched_name;
|
||||
g_assert (watched_name != NULL);
|
||||
g_assert (watched_name->get_name_owner_serial != 0);
|
||||
|
||||
type = g_dbus_message_get_message_type (message);
|
||||
body = g_dbus_message_get_body (message);
|
||||
|
||||
if (type == G_DBUS_MESSAGE_TYPE_ERROR)
|
||||
{
|
||||
if (g_strcmp0 (g_dbus_message_get_error_name (message),
|
||||
"org.freedesktop.DBus.Error.NameHasNoOwner"))
|
||||
name_watcher_set_name_owner_unlocked (name_watcher, NULL);
|
||||
/* else it's something like NoReply or AccessDenied, which tells
|
||||
* us nothing - leave the owner set to whatever we most recently
|
||||
* learned from NameOwnerChanged, or NULL */
|
||||
}
|
||||
else if (type != G_DBUS_MESSAGE_TYPE_METHOD_RETURN)
|
||||
{
|
||||
g_warning ("Received GetNameOwner reply with unexpected type %d",
|
||||
type);
|
||||
}
|
||||
else if (G_LIKELY (body != NULL && g_variant_is_of_type (body, G_VARIANT_TYPE ("(s)"))))
|
||||
{
|
||||
const char *new_owner;
|
||||
|
||||
g_variant_get (body, "(&s)", &new_owner);
|
||||
|
||||
if (G_LIKELY (g_dbus_is_unique_name (new_owner)))
|
||||
name_watcher_set_name_owner_unlocked (name_watcher, new_owner);
|
||||
else
|
||||
g_warning ("Received GetNameOwner reply with invalid owner \"%s\" for \"%s\"",
|
||||
new_owner, name_watcher->arg0);
|
||||
}
|
||||
else
|
||||
{
|
||||
g_warning ("Received GetNameOwner reply with unexpected signature %s",
|
||||
body == NULL ? "()" : g_variant_get_type_string (body));
|
||||
}
|
||||
|
||||
g_hash_table_remove (connection->map_method_serial_to_name_watcher,
|
||||
GUINT_TO_POINTER (watched_name->get_name_owner_serial));
|
||||
watched_name->get_name_owner_serial = 0;
|
||||
}
|
||||
|
||||
/* Called in a user thread, lock is held */
|
||||
static void
|
||||
name_watcher_call_get_name_owner_unlocked (GDBusConnection *connection,
|
||||
SignalData *name_watcher)
|
||||
{
|
||||
GDBusMessage *message;
|
||||
GError *local_error = NULL;
|
||||
WatchedName *watched_name;
|
||||
|
||||
g_assert (g_strcmp0 (name_watcher->sender, DBUS_SERVICE_DBUS) == 0);
|
||||
g_assert (g_strcmp0 (name_watcher->interface_name, DBUS_INTERFACE_DBUS) == 0);
|
||||
g_assert (g_strcmp0 (name_watcher->member, "NameOwnerChanged") == 0);
|
||||
g_assert (g_strcmp0 (name_watcher->object_path, DBUS_PATH_DBUS) == 0);
|
||||
/* arg0 of the NameOwnerChanged message is the well-known name whose owner
|
||||
* we are interested in */
|
||||
g_assert (g_dbus_is_name (name_watcher->arg0));
|
||||
g_assert (name_watcher->flags == G_DBUS_SIGNAL_FLAGS_NONE);
|
||||
|
||||
watched_name = name_watcher->watched_name;
|
||||
g_assert (watched_name != NULL);
|
||||
g_assert (watched_name->owner == NULL);
|
||||
g_assert (watched_name->get_name_owner_serial == 0);
|
||||
g_assert (name_watcher->shared_name_watcher == NULL);
|
||||
|
||||
message = g_dbus_message_new_method_call (DBUS_SERVICE_DBUS,
|
||||
DBUS_PATH_DBUS,
|
||||
DBUS_INTERFACE_DBUS,
|
||||
"GetNameOwner");
|
||||
g_dbus_message_set_body (message, g_variant_new ("(s)", name_watcher->arg0));
|
||||
|
||||
if (g_dbus_connection_send_message_unlocked (connection, message,
|
||||
G_DBUS_SEND_MESSAGE_FLAGS_NONE,
|
||||
&watched_name->get_name_owner_serial,
|
||||
&local_error))
|
||||
{
|
||||
g_assert (watched_name->get_name_owner_serial != 0);
|
||||
g_hash_table_insert (connection->map_method_serial_to_name_watcher,
|
||||
GUINT_TO_POINTER (watched_name->get_name_owner_serial),
|
||||
name_watcher);
|
||||
}
|
||||
else
|
||||
{
|
||||
g_critical ("Error while sending GetNameOwner() message: %s",
|
||||
local_error->message);
|
||||
g_clear_error (&local_error);
|
||||
g_assert (watched_name->get_name_owner_serial == 0);
|
||||
}
|
||||
|
||||
g_object_unref (message);
|
||||
}
|
||||
|
||||
/* ---------------------------------------------------------------------------------------------------- */
|
||||
|
||||
typedef struct
|
||||
{
|
||||
guint id;
|
||||
@ -2377,6 +2624,7 @@ on_worker_message_received (GDBusWorker *worker,
|
||||
{
|
||||
guint32 reply_serial;
|
||||
GTask *task;
|
||||
SignalData *name_watcher;
|
||||
|
||||
reply_serial = g_dbus_message_get_reply_serial (message);
|
||||
CONNECTION_LOCK (connection);
|
||||
@ -2392,6 +2640,19 @@ on_worker_message_received (GDBusWorker *worker,
|
||||
{
|
||||
//g_debug ("message reply/error for serial %d but no SendMessageData found for %p", reply_serial, connection);
|
||||
}
|
||||
|
||||
name_watcher = g_hash_table_lookup (connection->map_method_serial_to_name_watcher,
|
||||
GUINT_TO_POINTER (reply_serial));
|
||||
|
||||
if (name_watcher != NULL)
|
||||
{
|
||||
g_assert (name_watcher->watched_name != NULL);
|
||||
g_assert (name_watcher->watched_name->get_name_owner_serial == reply_serial);
|
||||
name_watcher_deliver_get_name_owner_reply_unlocked (name_watcher,
|
||||
connection,
|
||||
message);
|
||||
}
|
||||
|
||||
CONNECTION_UNLOCK (connection);
|
||||
}
|
||||
else if (message_type == G_DBUS_MESSAGE_TYPE_SIGNAL)
|
||||
@ -3580,6 +3841,7 @@ g_dbus_connection_signal_subscribe (GDBusConnection *connection,
|
||||
{
|
||||
gchar *rule;
|
||||
SignalData *signal_data;
|
||||
SignalData *name_watcher = NULL;
|
||||
SignalSubscriber *subscriber;
|
||||
gboolean sender_is_its_own_owner;
|
||||
const gchar *sender_unique_name;
|
||||
@ -3645,13 +3907,59 @@ g_dbus_connection_signal_subscribe (GDBusConnection *connection,
|
||||
|
||||
signal_data = signal_data_new_take (g_steal_pointer (&rule),
|
||||
g_strdup (sender),
|
||||
sender_is_its_own_owner,
|
||||
g_strdup (interface_name),
|
||||
g_strdup (member),
|
||||
g_strdup (object_path),
|
||||
g_strdup (arg0),
|
||||
flags);
|
||||
g_ptr_array_add (signal_data->subscribers, subscriber);
|
||||
|
||||
/* If subscribing to a signal from a specific sender with a well-known
|
||||
* name, we must first subscribe to NameOwnerChanged signals for that
|
||||
* well-known name, so that we can match the current owner of the name
|
||||
* against the sender of each signal. */
|
||||
if (sender != NULL && !sender_is_its_own_owner)
|
||||
{
|
||||
gchar *name_owner_rule = NULL;
|
||||
|
||||
/* We already checked that sender != NULL implies MESSAGE_BUS_CONNECTION */
|
||||
g_assert (connection->flags & G_DBUS_CONNECTION_FLAGS_MESSAGE_BUS_CONNECTION);
|
||||
|
||||
name_owner_rule = args_to_rule (DBUS_SERVICE_DBUS,
|
||||
DBUS_INTERFACE_DBUS,
|
||||
"NameOwnerChanged",
|
||||
DBUS_PATH_DBUS,
|
||||
sender,
|
||||
G_DBUS_SIGNAL_FLAGS_NONE);
|
||||
name_watcher = g_hash_table_lookup (connection->map_rule_to_signal_data, name_owner_rule);
|
||||
|
||||
if (name_watcher == NULL)
|
||||
{
|
||||
name_watcher = signal_data_new_take (g_steal_pointer (&name_owner_rule),
|
||||
g_strdup (DBUS_SERVICE_DBUS),
|
||||
g_strdup (DBUS_INTERFACE_DBUS),
|
||||
g_strdup ("NameOwnerChanged"),
|
||||
g_strdup (DBUS_PATH_DBUS),
|
||||
g_strdup (sender),
|
||||
G_DBUS_SIGNAL_FLAGS_NONE);
|
||||
add_signal_data (connection, name_watcher, DBUS_SERVICE_DBUS);
|
||||
}
|
||||
|
||||
if (name_watcher->watched_name == NULL)
|
||||
{
|
||||
name_watcher->watched_name = watched_name_new ();
|
||||
name_watcher_call_get_name_owner_unlocked (connection, name_watcher);
|
||||
}
|
||||
else
|
||||
{
|
||||
g_ref_count_inc (&name_watcher->watched_name->ref_count);
|
||||
}
|
||||
|
||||
signal_data->shared_name_watcher = name_watcher;
|
||||
|
||||
g_clear_pointer (&name_owner_rule, g_free);
|
||||
}
|
||||
|
||||
add_signal_data (connection, signal_data, sender_unique_name);
|
||||
|
||||
out:
|
||||
@ -3679,10 +3987,18 @@ remove_signal_data_if_unused (GDBusConnection *connection,
|
||||
const gchar *sender_unique_name;
|
||||
GPtrArray *signal_data_array;
|
||||
|
||||
/* Cannot remove while there are still subscribers */
|
||||
if (signal_data->subscribers->len != 0)
|
||||
return;
|
||||
|
||||
if (signal_data->sender_is_its_own_owner)
|
||||
/* Cannot remove while another SignalData is still using this one
|
||||
* as its shared_name_watcher, which holds watched_name->ref_count > 0 */
|
||||
if (signal_data->watched_name != NULL)
|
||||
return;
|
||||
|
||||
/* Point of no return: we have committed to removing it */
|
||||
|
||||
if (signal_data->sender != NULL && signal_data->shared_name_watcher == NULL)
|
||||
sender_unique_name = signal_data->sender;
|
||||
else
|
||||
sender_unique_name = "";
|
||||
@ -3715,6 +4031,15 @@ remove_signal_data_if_unused (GDBusConnection *connection,
|
||||
remove_match_rule (connection, signal_data->rule);
|
||||
}
|
||||
|
||||
if (signal_data->shared_name_watcher != NULL)
|
||||
{
|
||||
SignalData *name_watcher = g_steal_pointer (&signal_data->shared_name_watcher);
|
||||
|
||||
name_watcher_unref_watched_name (connection, name_watcher);
|
||||
/* May free signal_data */
|
||||
remove_signal_data_if_unused (connection, name_watcher);
|
||||
}
|
||||
|
||||
signal_data_free (signal_data);
|
||||
}
|
||||
|
||||
@ -3991,6 +4316,17 @@ schedule_callbacks (GDBusConnection *connection,
|
||||
continue;
|
||||
}
|
||||
|
||||
if (signal_data->watched_name != NULL)
|
||||
{
|
||||
/* Invariant: SignalData should only have a watched_name if it
|
||||
* represents the NameOwnerChanged signal */
|
||||
g_assert (g_strcmp0 (sender, DBUS_SERVICE_DBUS) == 0);
|
||||
g_assert (g_strcmp0 (interface, DBUS_INTERFACE_DBUS) == 0);
|
||||
g_assert (g_strcmp0 (path, DBUS_PATH_DBUS) == 0);
|
||||
g_assert (g_strcmp0 (member, "NameOwnerChanged") == 0);
|
||||
name_watcher_deliver_name_owner_changed_unlocked (signal_data, message);
|
||||
}
|
||||
|
||||
for (m = 0; m < signal_data->subscribers->len; m++)
|
||||
{
|
||||
SignalSubscriber *subscriber = signal_data->subscribers->pdata[m];
|
||||
@ -4062,7 +4398,7 @@ distribute_signals (GDBusConnection *connection,
|
||||
schedule_callbacks (connection, signal_data_array, message, sender);
|
||||
}
|
||||
|
||||
/* collect subscribers not matching on sender */
|
||||
/* collect subscribers not matching on sender, or matching a well-known name */
|
||||
signal_data_array = g_hash_table_lookup (connection->map_sender_unique_name_to_signal_data_array, "");
|
||||
if (signal_data_array != NULL)
|
||||
schedule_callbacks (connection, signal_data_array, message, sender);
|
||||
|
||||
Loading…
Reference in New Issue
Block a user