diff --git a/SOURCES/CVE-2024-34397.patch b/SOURCES/CVE-2024-34397.patch new file mode 100644 index 0000000..30da391 --- /dev/null +++ b/SOURCES/CVE-2024-34397.patch @@ -0,0 +1,3170 @@ +From f649bef8e83ed37bb53940f93ecb7c059a792659 Mon Sep 17 00:00:00 2001 +From: Simon McVittie +Date: Thu, 14 Mar 2024 19:18:15 +0000 +Subject: [PATCH 01/17] gdbusprivate: Add symbolic constants for the message + bus itself + +Using these is a bit more clearly correct than repeating them everywhere. +To avoid excessive diffstat in a branch for a bug fix, I'm not +immediately replacing all existing occurrences of the same literals with +these names. + +The names of these constants are chosen to be consistent with libdbus, +despite using somewhat outdated terminology (D-Bus now uses the term +"well-known bus name" for what used to be called a service name, +reserving the word "service" to mean specifically the programs that +have .service files and participate in service activation). + +Signed-off-by: Simon McVittie +--- + gio/gdbusprivate.h | 5 +++++ + 1 file changed, 5 insertions(+) + +diff --git a/gio/gdbusprivate.h b/gio/gdbusprivate.h +index 6a6a08014..8c4432e99 100644 +--- a/gio/gdbusprivate.h ++++ b/gio/gdbusprivate.h +@@ -29,6 +29,11 @@ + + G_BEGIN_DECLS + ++/* Bus name, interface and object path of the message bus itself */ ++#define DBUS_SERVICE_DBUS "org.freedesktop.DBus" ++#define DBUS_INTERFACE_DBUS DBUS_SERVICE_DBUS ++#define DBUS_PATH_DBUS "/org/freedesktop/DBus" ++ + /* ---------------------------------------------------------------------------------------------------- */ + + typedef struct GDBusWorker GDBusWorker; +-- +2.50.0 + + +From bc005d78375d823edb3f8de3ffa0bbf9c99832ad Mon Sep 17 00:00:00 2001 +From: Simon McVittie +Date: Thu, 14 Mar 2024 19:24:24 +0000 +Subject: [PATCH 02/17] gdbusconnection: Move SignalData, SignalSubscriber + higher up + +Subsequent changes will need to access these data structures from +on_worker_message_received(). No functional change here, only moving +code around. + +Signed-off-by: Simon McVittie +--- + gio/gdbusconnection.c | 128 +++++++++++++++++++++--------------------- + 1 file changed, 65 insertions(+), 63 deletions(-) + +diff --git a/gio/gdbusconnection.c b/gio/gdbusconnection.c +index 4d99f8570..c2189110f 100644 +--- a/gio/gdbusconnection.c ++++ b/gio/gdbusconnection.c +@@ -308,6 +308,71 @@ _g_strv_has_string (const gchar* const *haystack, + + /* ---------------------------------------------------------------------------------------------------- */ + ++typedef struct ++{ ++ /* All fields are immutable after construction. */ ++ gatomicrefcount ref_count; ++ GDBusSignalCallback callback; ++ gpointer user_data; ++ GDestroyNotify user_data_free_func; ++ guint id; ++ GMainContext *context; ++} SignalSubscriber; ++ ++static SignalSubscriber * ++signal_subscriber_ref (SignalSubscriber *subscriber) ++{ ++ g_atomic_ref_count_inc (&subscriber->ref_count); ++ return subscriber; ++} ++ ++static void ++signal_subscriber_unref (SignalSubscriber *subscriber) ++{ ++ if (g_atomic_ref_count_dec (&subscriber->ref_count)) ++ { ++ /* Destroy the user data. It doesn’t matter which thread ++ * signal_subscriber_unref() is called in (or whether it’s called with a ++ * lock held), as call_destroy_notify() always defers to the next ++ * #GMainContext iteration. */ ++ call_destroy_notify (subscriber->context, ++ subscriber->user_data_free_func, ++ subscriber->user_data); ++ ++ g_main_context_unref (subscriber->context); ++ g_free (subscriber); ++ } ++} ++ ++typedef struct ++{ ++ gchar *rule; ++ gchar *sender; ++ gchar *sender_unique_name; /* if sender is unique or org.freedesktop.DBus, then that name... otherwise blank */ ++ gchar *interface_name; ++ gchar *member; ++ gchar *object_path; ++ gchar *arg0; ++ GDBusSignalFlags flags; ++ GPtrArray *subscribers; /* (owned) (element-type SignalSubscriber) */ ++} SignalData; ++ ++static void ++signal_data_free (SignalData *signal_data) ++{ ++ g_free (signal_data->rule); ++ g_free (signal_data->sender); ++ g_free (signal_data->sender_unique_name); ++ g_free (signal_data->interface_name); ++ g_free (signal_data->member); ++ g_free (signal_data->object_path); ++ g_free (signal_data->arg0); ++ g_ptr_array_unref (signal_data->subscribers); ++ g_free (signal_data); ++} ++ ++/* ---------------------------------------------------------------------------------------------------- */ ++ + #ifdef G_OS_WIN32 + #define CONNECTION_ENSURE_LOCK(obj) do { ; } while (FALSE) + #else +@@ -3219,69 +3284,6 @@ g_dbus_connection_remove_filter (GDBusConnection *connection, + + /* ---------------------------------------------------------------------------------------------------- */ + +-typedef struct +-{ +- gchar *rule; +- gchar *sender; +- gchar *sender_unique_name; /* if sender is unique or org.freedesktop.DBus, then that name... otherwise blank */ +- gchar *interface_name; +- gchar *member; +- gchar *object_path; +- gchar *arg0; +- GDBusSignalFlags flags; +- GPtrArray *subscribers; /* (owned) (element-type SignalSubscriber) */ +-} SignalData; +- +-static void +-signal_data_free (SignalData *signal_data) +-{ +- g_free (signal_data->rule); +- g_free (signal_data->sender); +- g_free (signal_data->sender_unique_name); +- g_free (signal_data->interface_name); +- g_free (signal_data->member); +- g_free (signal_data->object_path); +- g_free (signal_data->arg0); +- g_ptr_array_unref (signal_data->subscribers); +- g_free (signal_data); +-} +- +-typedef struct +-{ +- /* All fields are immutable after construction. */ +- gatomicrefcount ref_count; +- GDBusSignalCallback callback; +- gpointer user_data; +- GDestroyNotify user_data_free_func; +- guint id; +- GMainContext *context; +-} SignalSubscriber; +- +-static SignalSubscriber * +-signal_subscriber_ref (SignalSubscriber *subscriber) +-{ +- g_atomic_ref_count_inc (&subscriber->ref_count); +- return subscriber; +-} +- +-static void +-signal_subscriber_unref (SignalSubscriber *subscriber) +-{ +- if (g_atomic_ref_count_dec (&subscriber->ref_count)) +- { +- /* Destroy the user data. It doesn’t matter which thread +- * signal_subscriber_unref() is called in (or whether it’s called with a +- * lock held), as call_destroy_notify() always defers to the next +- * #GMainContext iteration. */ +- call_destroy_notify (subscriber->context, +- subscriber->user_data_free_func, +- subscriber->user_data); +- +- g_main_context_unref (subscriber->context); +- g_free (subscriber); +- } +-} +- + static gchar * + args_to_rule (const gchar *sender, + const gchar *interface_name, +-- +2.50.0 + + +From 41b9ee0f7882d706c7b1965c69595bdca687dad5 Mon Sep 17 00:00:00 2001 +From: Simon McVittie +Date: Thu, 14 Mar 2024 19:30:12 +0000 +Subject: [PATCH 03/17] gdbusconnection: Factor out signal_data_new_take() + +No functional changes, except that the implicit ownership-transfer +for the rule field becomes explicit (the local variable is set to NULL +afterwards). + +Signed-off-by: Simon McVittie +--- + gio/gdbusconnection.c | 42 ++++++++++++++++++++++++++++++++---------- + 1 file changed, 32 insertions(+), 10 deletions(-) + +diff --git a/gio/gdbusconnection.c b/gio/gdbusconnection.c +index c2189110f..127218df4 100644 +--- a/gio/gdbusconnection.c ++++ b/gio/gdbusconnection.c +@@ -357,6 +357,30 @@ typedef struct + GPtrArray *subscribers; /* (owned) (element-type SignalSubscriber) */ + } SignalData; + ++static SignalData * ++signal_data_new_take (gchar *rule, ++ gchar *sender, ++ gchar *sender_unique_name, ++ gchar *interface_name, ++ gchar *member, ++ gchar *object_path, ++ gchar *arg0, ++ GDBusSignalFlags flags) ++{ ++ SignalData *signal_data = g_new0 (SignalData, 1); ++ ++ signal_data->rule = rule; ++ signal_data->sender = sender; ++ signal_data->sender_unique_name = sender_unique_name; ++ signal_data->interface_name = interface_name; ++ signal_data->member = member; ++ signal_data->object_path = object_path; ++ signal_data->arg0 = arg0; ++ signal_data->flags = flags; ++ signal_data->subscribers = g_ptr_array_new_with_free_func ((GDestroyNotify) signal_subscriber_unref); ++ return g_steal_pointer (&signal_data); ++} ++ + static void + signal_data_free (SignalData *signal_data) + { +@@ -3527,16 +3551,14 @@ g_dbus_connection_signal_subscribe (GDBusConnection *connection, + goto out; + } + +- signal_data = g_new0 (SignalData, 1); +- signal_data->rule = rule; +- signal_data->sender = g_strdup (sender); +- signal_data->sender_unique_name = g_strdup (sender_unique_name); +- signal_data->interface_name = g_strdup (interface_name); +- signal_data->member = g_strdup (member); +- signal_data->object_path = g_strdup (object_path); +- signal_data->arg0 = g_strdup (arg0); +- signal_data->flags = flags; +- signal_data->subscribers = g_ptr_array_new_with_free_func ((GDestroyNotify) signal_subscriber_unref); ++ signal_data = signal_data_new_take (g_steal_pointer (&rule), ++ g_strdup (sender), ++ g_strdup (sender_unique_name), ++ g_strdup (interface_name), ++ g_strdup (member), ++ g_strdup (object_path), ++ g_strdup (arg0), ++ flags); + g_ptr_array_add (signal_data->subscribers, subscriber); + + g_hash_table_insert (connection->map_rule_to_signal_data, +-- +2.50.0 + + +From 1144aa6fbd6e8694251f0aaab02a6c460905e00e Mon Sep 17 00:00:00 2001 +From: Simon McVittie +Date: Tue, 23 Apr 2024 20:31:57 +0100 +Subject: [PATCH 04/17] gdbusconnection: Factor out add_signal_data() + +No functional changes. + +Signed-off-by: Simon McVittie +--- + gio/gdbusconnection.c | 64 +++++++++++++++++++++++++------------------ + 1 file changed, 37 insertions(+), 27 deletions(-) + +diff --git a/gio/gdbusconnection.c b/gio/gdbusconnection.c +index 127218df4..7c8f6f573 100644 +--- a/gio/gdbusconnection.c ++++ b/gio/gdbusconnection.c +@@ -3428,6 +3428,42 @@ is_signal_data_for_name_lost_or_acquired (SignalData *signal_data) + + /* ---------------------------------------------------------------------------------------------------- */ + ++/* called in any thread, connection lock is held */ ++static void ++add_signal_data (GDBusConnection *connection, ++ SignalData *signal_data) ++{ ++ GPtrArray *signal_data_array; ++ ++ g_hash_table_insert (connection->map_rule_to_signal_data, ++ signal_data->rule, ++ signal_data); ++ ++ /* Add the match rule to the bus... ++ * ++ * Avoid adding match rules for NameLost and NameAcquired messages - the bus will ++ * always send such messages to us. ++ */ ++ if (connection->flags & G_DBUS_CONNECTION_FLAGS_MESSAGE_BUS_CONNECTION) ++ { ++ if (!is_signal_data_for_name_lost_or_acquired (signal_data)) ++ add_match_rule (connection, signal_data->rule); ++ } ++ ++ signal_data_array = g_hash_table_lookup (connection->map_sender_unique_name_to_signal_data_array, ++ signal_data->sender_unique_name); ++ if (signal_data_array == NULL) ++ { ++ signal_data_array = g_ptr_array_new (); ++ g_hash_table_insert (connection->map_sender_unique_name_to_signal_data_array, ++ g_strdup (signal_data->sender_unique_name), ++ signal_data_array); ++ } ++ g_ptr_array_add (signal_data_array, signal_data); ++} ++ ++/* ---------------------------------------------------------------------------------------------------- */ ++ + /** + * g_dbus_connection_signal_subscribe: + * @connection: a #GDBusConnection +@@ -3494,7 +3530,6 @@ g_dbus_connection_signal_subscribe (GDBusConnection *connection, + gchar *rule; + SignalData *signal_data; + SignalSubscriber *subscriber; +- GPtrArray *signal_data_array; + const gchar *sender_unique_name; + + /* Right now we abort if AddMatch() fails since it can only fail with the bus being in +@@ -3560,32 +3595,7 @@ g_dbus_connection_signal_subscribe (GDBusConnection *connection, + g_strdup (arg0), + flags); + g_ptr_array_add (signal_data->subscribers, subscriber); +- +- g_hash_table_insert (connection->map_rule_to_signal_data, +- signal_data->rule, +- signal_data); +- +- /* Add the match rule to the bus... +- * +- * Avoid adding match rules for NameLost and NameAcquired messages - the bus will +- * always send such messages to us. +- */ +- if (connection->flags & G_DBUS_CONNECTION_FLAGS_MESSAGE_BUS_CONNECTION) +- { +- if (!is_signal_data_for_name_lost_or_acquired (signal_data)) +- add_match_rule (connection, signal_data->rule); +- } +- +- signal_data_array = g_hash_table_lookup (connection->map_sender_unique_name_to_signal_data_array, +- signal_data->sender_unique_name); +- if (signal_data_array == NULL) +- { +- signal_data_array = g_ptr_array_new (); +- g_hash_table_insert (connection->map_sender_unique_name_to_signal_data_array, +- g_strdup (signal_data->sender_unique_name), +- signal_data_array); +- } +- g_ptr_array_add (signal_data_array, signal_data); ++ add_signal_data (connection, signal_data); + + out: + g_hash_table_insert (connection->map_id_to_signal_data, +-- +2.50.0 + + +From a0adf817b6b5f02bf7c595f66c6f1d2041b0d817 Mon Sep 17 00:00:00 2001 +From: Simon McVittie +Date: Thu, 14 Mar 2024 19:51:59 +0000 +Subject: [PATCH 05/17] gdbusconnection: Factor out + remove_signal_data_if_unused + +No functional change, just removing some nesting. The check for whether +signal_data->subscribers is empty changes from a conditional that tests +whether it is into an early-return if it isn't. + +A subsequent commit will add additional conditions that make us consider +a SignalData to be still in use and therefore not eligible to be removed. + +Signed-off-by: Simon McVittie +--- + gio/gdbusconnection.c | 83 +++++++++++++++++++++++++------------------ + 1 file changed, 48 insertions(+), 35 deletions(-) + +diff --git a/gio/gdbusconnection.c b/gio/gdbusconnection.c +index 7c8f6f573..2f2141146 100644 +--- a/gio/gdbusconnection.c ++++ b/gio/gdbusconnection.c +@@ -3609,6 +3609,52 @@ g_dbus_connection_signal_subscribe (GDBusConnection *connection, + + /* ---------------------------------------------------------------------------------------------------- */ + ++/* ++ * Called in any thread. ++ * Must hold the connection lock when calling this, unless ++ * connection->finalizing is TRUE. ++ * May free signal_data, so do not dereference it after this. ++ */ ++static void ++remove_signal_data_if_unused (GDBusConnection *connection, ++ SignalData *signal_data) ++{ ++ GPtrArray *signal_data_array; ++ ++ if (signal_data->subscribers->len != 0) ++ return; ++ ++ g_warn_if_fail (g_hash_table_remove (connection->map_rule_to_signal_data, signal_data->rule)); ++ ++ signal_data_array = g_hash_table_lookup (connection->map_sender_unique_name_to_signal_data_array, ++ signal_data->sender_unique_name); ++ g_warn_if_fail (signal_data_array != NULL); ++ g_warn_if_fail (g_ptr_array_remove (signal_data_array, signal_data)); ++ ++ if (signal_data_array->len == 0) ++ { ++ g_warn_if_fail (g_hash_table_remove (connection->map_sender_unique_name_to_signal_data_array, ++ signal_data->sender_unique_name)); ++ } ++ ++ /* remove the match rule from the bus unless NameLost or NameAcquired (see subscribe()) */ ++ if ((connection->flags & G_DBUS_CONNECTION_FLAGS_MESSAGE_BUS_CONNECTION) && ++ !is_signal_data_for_name_lost_or_acquired (signal_data) && ++ !g_dbus_connection_is_closed (connection) && ++ !connection->finalizing) ++ { ++ /* The check for g_dbus_connection_is_closed() means that ++ * sending the RemoveMatch message can't fail with ++ * G_IO_ERROR_CLOSED, because we're holding the lock, ++ * so on_worker_closed() can't happen between the check we just ++ * did, and releasing the lock later. ++ */ ++ remove_match_rule (connection, signal_data->rule); ++ } ++ ++ signal_data_free (signal_data); ++} ++ + /* called in any thread */ + /* must hold lock when calling this (except if connection->finalizing is TRUE) + * returns the number of removed subscribers */ +@@ -3617,7 +3663,6 @@ unsubscribe_id_internal (GDBusConnection *connection, + guint subscription_id) + { + SignalData *signal_data; +- GPtrArray *signal_data_array; + guint n; + guint n_removed = 0; + +@@ -3644,40 +3689,8 @@ unsubscribe_id_internal (GDBusConnection *connection, + GUINT_TO_POINTER (subscription_id))); + n_removed++; + g_ptr_array_remove_index_fast (signal_data->subscribers, n); +- +- if (signal_data->subscribers->len == 0) +- { +- g_warn_if_fail (g_hash_table_remove (connection->map_rule_to_signal_data, signal_data->rule)); +- +- signal_data_array = g_hash_table_lookup (connection->map_sender_unique_name_to_signal_data_array, +- signal_data->sender_unique_name); +- g_warn_if_fail (signal_data_array != NULL); +- g_warn_if_fail (g_ptr_array_remove (signal_data_array, signal_data)); +- +- if (signal_data_array->len == 0) +- { +- g_warn_if_fail (g_hash_table_remove (connection->map_sender_unique_name_to_signal_data_array, +- signal_data->sender_unique_name)); +- } +- +- /* remove the match rule from the bus unless NameLost or NameAcquired (see subscribe()) */ +- if ((connection->flags & G_DBUS_CONNECTION_FLAGS_MESSAGE_BUS_CONNECTION) && +- !is_signal_data_for_name_lost_or_acquired (signal_data) && +- !g_dbus_connection_is_closed (connection) && +- !connection->finalizing) +- { +- /* The check for g_dbus_connection_is_closed() means that +- * sending the RemoveMatch message can't fail with +- * G_IO_ERROR_CLOSED, because we're holding the lock, +- * so on_worker_closed() can't happen between the check we just +- * did, and releasing the lock later. +- */ +- remove_match_rule (connection, signal_data->rule); +- } +- +- signal_data_free (signal_data); +- } +- ++ /* May free signal_data */ ++ remove_signal_data_if_unused (connection, signal_data); + goto out; + } + +-- +2.50.0 + + +From ca3d065bb0645c58075d2057fa904260bf7c66bd Mon Sep 17 00:00:00 2001 +From: Simon McVittie +Date: Tue, 23 Apr 2024 20:39:05 +0100 +Subject: [PATCH 06/17] gdbusconnection: Stop storing sender_unique_name in + SignalData + +This will become confusing when we start tracking the owner of a +well-known-name sender, and it's redundant anyway. Instead, track the +1 bit of data that we actually need: whether it's a well-known name. + +Strictly speaking this too is redundant, because it's syntactically +derivable from the sender, but only via extra string operations. +A subsequent commit will add a data structure to keep track of the +owner of a well-known-name sender, at which point this boolean will +be replaced by the presence or absence of that data structure. + +Signed-off-by: Simon McVittie +--- + gio/gdbusconnection.c | 36 ++++++++++++++++++++++++------------ + 1 file changed, 24 insertions(+), 12 deletions(-) + +diff --git a/gio/gdbusconnection.c b/gio/gdbusconnection.c +index 2f2141146..66e4bcee5 100644 +--- a/gio/gdbusconnection.c ++++ b/gio/gdbusconnection.c +@@ -348,19 +348,19 @@ typedef struct + { + gchar *rule; + gchar *sender; +- gchar *sender_unique_name; /* if sender is unique or org.freedesktop.DBus, then that name... otherwise blank */ + gchar *interface_name; + gchar *member; + gchar *object_path; + gchar *arg0; + GDBusSignalFlags flags; + GPtrArray *subscribers; /* (owned) (element-type SignalSubscriber) */ ++ gboolean sender_is_its_own_owner; + } SignalData; + + static SignalData * + signal_data_new_take (gchar *rule, + gchar *sender, +- gchar *sender_unique_name, ++ gboolean sender_is_its_own_owner, + gchar *interface_name, + gchar *member, + gchar *object_path, +@@ -371,7 +371,7 @@ signal_data_new_take (gchar *rule, + + signal_data->rule = rule; + signal_data->sender = sender; +- signal_data->sender_unique_name = sender_unique_name; ++ 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; +@@ -386,7 +386,6 @@ signal_data_free (SignalData *signal_data) + { + g_free (signal_data->rule); + g_free (signal_data->sender); +- g_free (signal_data->sender_unique_name); + g_free (signal_data->interface_name); + g_free (signal_data->member); + g_free (signal_data->object_path); +@@ -3419,7 +3418,7 @@ remove_match_rule (GDBusConnection *connection, + static gboolean + is_signal_data_for_name_lost_or_acquired (SignalData *signal_data) + { +- return g_strcmp0 (signal_data->sender_unique_name, "org.freedesktop.DBus") == 0 && ++ return g_strcmp0 (signal_data->sender, "org.freedesktop.DBus") == 0 && + g_strcmp0 (signal_data->interface_name, "org.freedesktop.DBus") == 0 && + g_strcmp0 (signal_data->object_path, "/org/freedesktop/DBus") == 0 && + (g_strcmp0 (signal_data->member, "NameLost") == 0 || +@@ -3431,7 +3430,8 @@ is_signal_data_for_name_lost_or_acquired (SignalData *signal_data) + /* called in any thread, connection lock is held */ + static void + add_signal_data (GDBusConnection *connection, +- SignalData *signal_data) ++ SignalData *signal_data, ++ const char *sender_unique_name) + { + GPtrArray *signal_data_array; + +@@ -3451,12 +3451,12 @@ add_signal_data (GDBusConnection *connection, + } + + signal_data_array = g_hash_table_lookup (connection->map_sender_unique_name_to_signal_data_array, +- signal_data->sender_unique_name); ++ sender_unique_name); + if (signal_data_array == NULL) + { + signal_data_array = g_ptr_array_new (); + g_hash_table_insert (connection->map_sender_unique_name_to_signal_data_array, +- g_strdup (signal_data->sender_unique_name), ++ g_strdup (sender_unique_name), + signal_data_array); + } + g_ptr_array_add (signal_data_array, signal_data); +@@ -3530,6 +3530,7 @@ g_dbus_connection_signal_subscribe (GDBusConnection *connection, + gchar *rule; + SignalData *signal_data; + SignalSubscriber *subscriber; ++ gboolean sender_is_its_own_owner; + const gchar *sender_unique_name; + + /* Right now we abort if AddMatch() fails since it can only fail with the bus being in +@@ -3565,6 +3566,11 @@ g_dbus_connection_signal_subscribe (GDBusConnection *connection, + rule = args_to_rule (sender, interface_name, member, object_path, arg0, flags); + + if (sender != NULL && (g_dbus_is_unique_name (sender) || g_strcmp0 (sender, "org.freedesktop.DBus") == 0)) ++ sender_is_its_own_owner = TRUE; ++ else ++ sender_is_its_own_owner = FALSE; ++ ++ if (sender_is_its_own_owner) + sender_unique_name = sender; + else + sender_unique_name = ""; +@@ -3588,14 +3594,14 @@ g_dbus_connection_signal_subscribe (GDBusConnection *connection, + + signal_data = signal_data_new_take (g_steal_pointer (&rule), + g_strdup (sender), +- g_strdup (sender_unique_name), ++ 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); +- add_signal_data (connection, signal_data); ++ add_signal_data (connection, signal_data, sender_unique_name); + + out: + g_hash_table_insert (connection->map_id_to_signal_data, +@@ -3619,22 +3625,28 @@ static void + remove_signal_data_if_unused (GDBusConnection *connection, + SignalData *signal_data) + { ++ const gchar *sender_unique_name; + GPtrArray *signal_data_array; + + if (signal_data->subscribers->len != 0) + return; + ++ if (signal_data->sender_is_its_own_owner) ++ sender_unique_name = signal_data->sender; ++ else ++ sender_unique_name = ""; ++ + g_warn_if_fail (g_hash_table_remove (connection->map_rule_to_signal_data, signal_data->rule)); + + signal_data_array = g_hash_table_lookup (connection->map_sender_unique_name_to_signal_data_array, +- signal_data->sender_unique_name); ++ sender_unique_name); + g_warn_if_fail (signal_data_array != NULL); + g_warn_if_fail (g_ptr_array_remove (signal_data_array, signal_data)); + + if (signal_data_array->len == 0) + { + g_warn_if_fail (g_hash_table_remove (connection->map_sender_unique_name_to_signal_data_array, +- signal_data->sender_unique_name)); ++ sender_unique_name)); + } + + /* remove the match rule from the bus unless NameLost or NameAcquired (see subscribe()) */ +-- +2.50.0 + + +From f2b82b3657ecc00e2c053223917d13be1596c5ba Mon Sep 17 00:00:00 2001 +From: Michael Catanzaro +Date: Thu, 10 Jul 2025 19:25:31 -0500 +Subject: [PATCH 07/17] gdbusconnection: Add copy of g_set_str() + +This makes it slightly easier to backport the next commit. +--- + gio/gdbusconnection.c | 17 +++++++++++++++++ + 1 file changed, 17 insertions(+) + +diff --git a/gio/gdbusconnection.c b/gio/gdbusconnection.c +index 66e4bcee5..00b23e039 100644 +--- a/gio/gdbusconnection.c ++++ b/gio/gdbusconnection.c +@@ -129,6 +129,23 @@ + + #include "glibintl.h" + ++static gboolean ++g_set_str (char **str_pointer, ++ const char *new_str) ++{ ++ char *copy; ++ ++ if (*str_pointer == new_str || ++ (*str_pointer && new_str && strcmp (*str_pointer, new_str) == 0)) ++ return FALSE; ++ ++ copy = g_strdup (new_str); ++ g_free (*str_pointer); ++ *str_pointer = copy; ++ ++ return TRUE; ++} ++ + /** + * SECTION:gdbusconnection + * @short_description: D-Bus Connections +-- +2.50.0 + + +From ae27ec237a3d7ae26e5f1f97ae1af325f0c90e77 Mon Sep 17 00:00:00 2001 +From: Simon McVittie +Date: Tue, 23 Apr 2024 20:42:17 +0100 +Subject: [PATCH 08/17] 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 +--- + gio/gdbusconnection.c | 350 +++++++++++++++++++++++++++++++++++++++++- + 1 file changed, 343 insertions(+), 7 deletions(-) + +diff --git a/gio/gdbusconnection.c b/gio/gdbusconnection.c +index 00b23e039..2795c97af 100644 +--- a/gio/gdbusconnection.c ++++ b/gio/gdbusconnection.c +@@ -362,6 +362,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; +@@ -371,13 +396,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, +@@ -388,7 +436,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; +@@ -401,6 +448,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); +@@ -408,6 +466,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); + } + +@@ -539,6 +598,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 */ +@@ -786,6 +846,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); +@@ -1173,6 +1234,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); +@@ -2272,6 +2334,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; +@@ -2383,6 +2630,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); +@@ -2398,6 +2646,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) +@@ -3546,6 +3807,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; +@@ -3611,13 +3873,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: +@@ -3645,10 +3953,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 = ""; +@@ -3681,6 +3997,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); + } + +@@ -3940,6 +4265,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]; +@@ -4001,7 +4337,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); +-- +2.50.0 + + +From 1bdfa0ea3fa438f855b1b6605a8337dc87c7a8ec Mon Sep 17 00:00:00 2001 +From: Simon McVittie +Date: Thu, 14 Mar 2024 20:42:41 +0000 +Subject: [PATCH 09/17] gdbusconnection: Don't deliver signals if the sender + doesn't match + +Otherwise a malicious connection on a shared bus, especially the system +bus, could trick GDBus clients into processing signals sent by the +malicious connection as though they had come from the real owner of a +well-known service name. + +Resolves: https://gitlab.gnome.org/GNOME/glib/-/issues/3268 +Signed-off-by: Simon McVittie +--- + gio/gdbusconnection.c | 40 ++++++++++++++++++++++++++++++++++++++++ + 1 file changed, 40 insertions(+) + +diff --git a/gio/gdbusconnection.c b/gio/gdbusconnection.c +index 2795c97af..478ab07c8 100644 +--- a/gio/gdbusconnection.c ++++ b/gio/gdbusconnection.c +@@ -4246,6 +4246,46 @@ schedule_callbacks (GDBusConnection *connection, + if (signal_data->object_path != NULL && g_strcmp0 (signal_data->object_path, path) != 0) + continue; + ++ if (signal_data->shared_name_watcher != NULL) ++ { ++ /* We want signals from a specified well-known name, which means ++ * the signal's sender needs to be the unique name that currently ++ * owns that well-known name, and we will have found this ++ * SignalData in ++ * connection->map_sender_unique_name_to_signal_data_array[""]. */ ++ const WatchedName *watched_name; ++ const char *current_owner; ++ ++ g_assert (signal_data->sender != NULL); ++ /* Invariant: We never need to watch for the owner of a unique ++ * name, or for the owner of DBUS_SERVICE_DBUS, either of which ++ * is always its own owner */ ++ g_assert (!g_dbus_is_unique_name (signal_data->sender)); ++ g_assert (g_strcmp0 (signal_data->sender, DBUS_SERVICE_DBUS) != 0); ++ ++ watched_name = signal_data->shared_name_watcher->watched_name; ++ g_assert (watched_name != NULL); ++ current_owner = watched_name->owner; ++ ++ /* Skip the signal if the actual sender is not known to own ++ * the required name */ ++ if (current_owner == NULL || g_strcmp0 (current_owner, sender) != 0) ++ continue; ++ } ++ else if (signal_data->sender != NULL) ++ { ++ /* We want signals from a unique name or o.fd.DBus... */ ++ g_assert (g_dbus_is_unique_name (signal_data->sender) ++ || g_str_equal (signal_data->sender, DBUS_SERVICE_DBUS)); ++ ++ /* ... which means we must have found this SignalData in ++ * connection->map_sender_unique_name_to_signal_data_array[signal_data->sender], ++ * therefore we would only have found it if the signal's ++ * actual sender matches the required signal_data->sender */ ++ g_assert (g_strcmp0 (signal_data->sender, sender) == 0); ++ } ++ /* else the sender is unspecified and we will accept anything */ ++ + if (signal_data->arg0 != NULL) + { + if (arg0 == NULL) +-- +2.50.0 + + +From ad49010fbf639d080238e97b89f526098dd9a747 Mon Sep 17 00:00:00 2001 +From: Simon McVittie +Date: Wed, 8 May 2024 14:46:08 +0000 +Subject: [PATCH 10/17] gdbusconnection: Allow name owners to have the syntax + of a well-known name + +In a D-Bus-Specification-compliant message bus, the owner of a well-known +name is a unique name. However, ibus has its own small implementation +of a message bus (src/ibusbus.c) in which org.freedesktop.IBus is +special-cased to also have itself as its owner (like org.freedesktop.DBus +on a standard message bus), and connects to that bus with the +G_DBUS_CONNECTION_FLAGS_MESSAGE_BUS_CONNECTION flag. The ability to do +this regressed when CVE-2024-34397 was fixed. + +Relax the checks to allow the owner of a well-known name to be any valid +D-Bus name, even if it is not syntactically a unique name. + +Fixes: 683b14b9 "gdbus: Track name owners for signal subscriptions" +Resolves: https://gitlab.gnome.org/GNOME/glib/-/issues/3353 +Bug-Debian: https://bugs.debian.org/1070730 +Bug-Debian: https://bugs.debian.org/1070736 +Bug-Debian: https://bugs.debian.org/1070743 +Bug-Debian: https://bugs.debian.org/1070745 +Signed-off-by: Simon McVittie +--- + gio/gdbusconnection.c | 10 ++++++++-- + 1 file changed, 8 insertions(+), 2 deletions(-) + +diff --git a/gio/gdbusconnection.c b/gio/gdbusconnection.c +index 478ab07c8..b4cdc7e8d 100644 +--- a/gio/gdbusconnection.c ++++ b/gio/gdbusconnection.c +@@ -2397,7 +2397,10 @@ name_watcher_deliver_name_owner_changed_unlocked (SignalData *name_watcher, + /* 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))) ++ /* FIXME: This should be validating that `new_owner` is a unique name, ++ * but IBus’ implementation of a message bus is not compliant with the spec. ++ * See https://gitlab.gnome.org/GNOME/glib/-/issues/3353 */ ++ if (G_LIKELY (new_owner[0] == '\0' || g_dbus_is_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\"", +@@ -2449,7 +2452,10 @@ name_watcher_deliver_get_name_owner_reply_unlocked (SignalData *name_watcher, + + g_variant_get (body, "(&s)", &new_owner); + +- if (G_LIKELY (g_dbus_is_unique_name (new_owner))) ++ /* FIXME: This should be validating that `new_owner` is a unique name, ++ * but IBus’ implementation of a message bus is not compliant with the spec. ++ * See https://gitlab.gnome.org/GNOME/glib/-/issues/3353 */ ++ if (G_LIKELY (g_dbus_is_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\"", +-- +2.50.0 + + +From 7ef0d99dc341206cd7f44a7c289debdfc12ea780 Mon Sep 17 00:00:00 2001 +From: Simon McVittie +Date: Fri, 8 Mar 2024 14:19:46 +0000 +Subject: [PATCH 11/17] tests: Add a data-driven test for signal subscriptions + +This somewhat duplicates test_connection_signals(), but is easier to +extend to cover different scenarios. + +Each scenario is tested three times: once with lower-level +GDBusConnection APIs, once with the higher-level GDBusProxy (which +cannot implement all of the subscription scenarios, so some message +counts are lower), and once with both (to check that delivery of the +same message to multiple destinations is handled appropriately). + +[Backported to glib-2-74, resolving conflicts in gio/tests/meson.build] +Signed-off-by: Simon McVittie +--- + gio/tests/gdbus-subscribe.c | 938 ++++++++++++++++++++++++++++++++++++ + gio/tests/meson.build | 1 + + 2 files changed, 939 insertions(+) + create mode 100644 gio/tests/gdbus-subscribe.c + +diff --git a/gio/tests/gdbus-subscribe.c b/gio/tests/gdbus-subscribe.c +new file mode 100644 +index 000000000..3f53e1d7f +--- /dev/null ++++ b/gio/tests/gdbus-subscribe.c +@@ -0,0 +1,938 @@ ++/* ++ * Copyright 2024 Collabora Ltd. ++ * SPDX-License-Identifier: LGPL-2.1-or-later ++ */ ++ ++#include ++ ++#include "gdbus-tests.h" ++ ++#define DBUS_SERVICE_DBUS "org.freedesktop.DBus" ++#define DBUS_PATH_DBUS "/org/freedesktop/DBus" ++#define DBUS_INTERFACE_DBUS DBUS_SERVICE_DBUS ++ ++/* A signal that each connection emits to indicate that it has finished ++ * emitting other signals */ ++#define FINISHED_PATH "/org/gtk/Test/Finished" ++#define FINISHED_INTERFACE "org.gtk.Test.Finished" ++#define FINISHED_SIGNAL "Finished" ++ ++/* A signal emitted during testing */ ++#define EXAMPLE_PATH "/org/gtk/GDBus/ExampleInterface" ++#define EXAMPLE_INTERFACE "org.gtk.GDBus.ExampleInterface" ++#define FOO_SIGNAL "Foo" ++ ++/* Log @s in a debug message. */ ++static inline const char * ++nonnull (const char *s, ++ const char *if_null) ++{ ++ return (s == NULL) ? if_null : s; ++} ++ ++typedef enum ++{ ++ TEST_CONN_NONE, ++ TEST_CONN_FIRST, ++ /* A connection that subscribes to signals */ ++ TEST_CONN_SUBSCRIBER = TEST_CONN_FIRST, ++ /* A mockup of a legitimate service */ ++ TEST_CONN_SERVICE, ++ /* A mockup of a second legitimate service */ ++ TEST_CONN_SERVICE2, ++ /* A connection that tries to trick @subscriber into processing its signals ++ * as if they came from @service */ ++ TEST_CONN_ATTACKER, ++ NUM_TEST_CONNS ++} TestConn; ++ ++static const char * const test_conn_descriptions[NUM_TEST_CONNS] = ++{ ++ "(unused)", ++ "subscriber", ++ "service", ++ "service 2", ++ "attacker" ++}; ++ ++typedef enum ++{ ++ SUBSCRIPTION_MODE_CONN, ++ SUBSCRIPTION_MODE_PROXY, ++ SUBSCRIPTION_MODE_PARALLEL ++} SubscriptionMode; ++ ++typedef struct ++{ ++ GDBusProxy *received_by_proxy; ++ TestConn sender; ++ char *path; ++ char *iface; ++ char *member; ++ GVariant *parameters; ++ char *arg0; ++ guint32 step; ++} ReceivedMessage; ++ ++static void ++received_message_free (ReceivedMessage *self) ++{ ++ ++ g_clear_object (&self->received_by_proxy); ++ g_free (self->path); ++ g_free (self->iface); ++ g_free (self->member); ++ g_clear_pointer (&self->parameters, g_variant_unref); ++ g_free (self->arg0); ++ g_free (self); ++} ++ ++typedef struct ++{ ++ TestConn sender; ++ TestConn unicast_to; ++ const char *path; ++ const char *iface; ++ const char *member; ++ const char *arg0; ++ guint received_by_conn; ++ guint received_by_proxy; ++} TestEmitSignal; ++ ++typedef struct ++{ ++ TestConn sender; ++ const char *path; ++ const char *iface; ++ const char *member; ++ const char *arg0; ++ GDBusSignalFlags flags; ++} TestSubscribe; ++ ++typedef enum ++{ ++ TEST_ACTION_NONE = 0, ++ TEST_ACTION_SUBSCRIBE, ++ TEST_ACTION_EMIT_SIGNAL, ++} TestAction; ++ ++typedef struct ++{ ++ TestAction action; ++ union { ++ TestEmitSignal signal; ++ TestSubscribe subscribe; ++ } u; ++} TestStep; ++ ++/* Arbitrary, extend as necessary to accommodate the longest test */ ++#define MAX_TEST_STEPS 10 ++ ++typedef struct ++{ ++ const char *description; ++ TestStep steps[MAX_TEST_STEPS]; ++} TestPlan; ++ ++static const TestPlan plan_simple = ++{ ++ .description = "A broadcast is only received after subscribing to it", ++ .steps = { ++ { ++ /* We don't receive a signal if we haven't subscribed yet */ ++ .action = TEST_ACTION_EMIT_SIGNAL, ++ .u.signal = { ++ .sender = TEST_CONN_SERVICE, ++ .path = EXAMPLE_PATH, ++ .iface = EXAMPLE_INTERFACE, ++ .member = FOO_SIGNAL, ++ .received_by_conn = 0, ++ .received_by_proxy = 0 ++ }, ++ }, ++ { ++ .action = TEST_ACTION_SUBSCRIBE, ++ .u.subscribe = { ++ .path = EXAMPLE_PATH, ++ .iface = EXAMPLE_INTERFACE, ++ }, ++ }, ++ { ++ /* Now it works */ ++ .action = TEST_ACTION_EMIT_SIGNAL, ++ .u.signal = { ++ .sender = TEST_CONN_SERVICE, ++ .path = EXAMPLE_PATH, ++ .iface = EXAMPLE_INTERFACE, ++ .member = FOO_SIGNAL, ++ .received_by_conn = 1, ++ /* The proxy can't be used in this case, because it needs ++ * a bus name to subscribe to */ ++ .received_by_proxy = 0 ++ }, ++ }, ++ }, ++}; ++ ++static const TestPlan plan_broadcast_from_anyone = ++{ ++ .description = "A subscription with NULL sender accepts broadcast and unicast", ++ .steps = { ++ { ++ /* Subscriber wants to receive signals from anyone */ ++ .action = TEST_ACTION_SUBSCRIBE, ++ .u.subscribe = { ++ .path = EXAMPLE_PATH, ++ .iface = EXAMPLE_INTERFACE, ++ }, ++ }, ++ { ++ /* First service sends a broadcast */ ++ .action = TEST_ACTION_EMIT_SIGNAL, ++ .u.signal = { ++ .sender = TEST_CONN_SERVICE, ++ .path = EXAMPLE_PATH, ++ .iface = EXAMPLE_INTERFACE, ++ .member = FOO_SIGNAL, ++ .received_by_conn = 1, ++ .received_by_proxy = 0 ++ }, ++ }, ++ { ++ /* Second service also sends a broadcast */ ++ .action = TEST_ACTION_EMIT_SIGNAL, ++ .u.signal = { ++ .sender = TEST_CONN_SERVICE2, ++ .path = EXAMPLE_PATH, ++ .iface = EXAMPLE_INTERFACE, ++ .member = FOO_SIGNAL, ++ .received_by_conn = 1, ++ .received_by_proxy = 0 ++ }, ++ }, ++ { ++ /* First service sends a unicast signal */ ++ .action = TEST_ACTION_EMIT_SIGNAL, ++ .u.signal = { ++ .sender = TEST_CONN_SERVICE, ++ .unicast_to = TEST_CONN_SUBSCRIBER, ++ .path = EXAMPLE_PATH, ++ .iface = EXAMPLE_INTERFACE, ++ .member = FOO_SIGNAL, ++ .received_by_conn = 1, ++ .received_by_proxy = 0 ++ }, ++ }, ++ { ++ /* Second service also sends a unicast signal */ ++ .action = TEST_ACTION_EMIT_SIGNAL, ++ .u.signal = { ++ .sender = TEST_CONN_SERVICE2, ++ .unicast_to = TEST_CONN_SUBSCRIBER, ++ .path = EXAMPLE_PATH, ++ .iface = EXAMPLE_INTERFACE, ++ .member = FOO_SIGNAL, ++ .received_by_conn = 1, ++ .received_by_proxy = 0 ++ }, ++ }, ++ }, ++}; ++ ++static const TestPlan plan_match_twice = ++{ ++ .description = "A message matching more than one subscription is received " ++ "once per subscription", ++ .steps = { ++ { ++ .action = TEST_ACTION_SUBSCRIBE, ++ .u.subscribe = { ++ .sender = TEST_CONN_SERVICE, ++ .path = EXAMPLE_PATH, ++ .iface = EXAMPLE_INTERFACE, ++ }, ++ }, ++ { ++ .action = TEST_ACTION_SUBSCRIBE, ++ .u.subscribe = { ++ .path = EXAMPLE_PATH, ++ }, ++ }, ++ { ++ .action = TEST_ACTION_SUBSCRIBE, ++ .u.subscribe = { ++ .iface = EXAMPLE_INTERFACE, ++ }, ++ }, ++ { ++ .action = TEST_ACTION_SUBSCRIBE, ++ .u.subscribe = { ++ .sender = TEST_CONN_SERVICE, ++ .path = EXAMPLE_PATH, ++ .iface = EXAMPLE_INTERFACE, ++ }, ++ }, ++ { ++ .action = TEST_ACTION_EMIT_SIGNAL, ++ .u.signal = { ++ .sender = TEST_CONN_SERVICE, ++ .path = EXAMPLE_PATH, ++ .iface = EXAMPLE_INTERFACE, ++ .member = FOO_SIGNAL, ++ .received_by_conn = 4, ++ /* Only the first and last work with GDBusProxy */ ++ .received_by_proxy = 2 ++ }, ++ }, ++ }, ++}; ++ ++static const TestPlan plan_limit_by_unique_name = ++{ ++ .description = "A subscription via a unique name only accepts messages " ++ "sent by that same unique name", ++ .steps = { ++ { ++ /* Subscriber wants to receive signals from service */ ++ .action = TEST_ACTION_SUBSCRIBE, ++ .u.subscribe = { ++ .sender = TEST_CONN_SERVICE, ++ .path = EXAMPLE_PATH, ++ .iface = EXAMPLE_INTERFACE, ++ }, ++ }, ++ { ++ /* Attacker wants to trick subscriber into thinking that service ++ * sent a signal */ ++ .action = TEST_ACTION_EMIT_SIGNAL, ++ .u.signal = { ++ .sender = TEST_CONN_ATTACKER, ++ .path = EXAMPLE_PATH, ++ .iface = EXAMPLE_INTERFACE, ++ .member = FOO_SIGNAL, ++ .received_by_conn = 0, ++ .received_by_proxy = 0 ++ }, ++ }, ++ { ++ /* Attacker tries harder, by sending a signal unicast directly to ++ * the subscriber */ ++ .action = TEST_ACTION_EMIT_SIGNAL, ++ .u.signal = { ++ .sender = TEST_CONN_ATTACKER, ++ .unicast_to = TEST_CONN_SUBSCRIBER, ++ .path = EXAMPLE_PATH, ++ .iface = EXAMPLE_INTERFACE, ++ .member = FOO_SIGNAL, ++ .received_by_conn = 0, ++ .received_by_proxy = 0 ++ }, ++ }, ++ { ++ /* When the real service sends a signal, it should still get through */ ++ .action = TEST_ACTION_EMIT_SIGNAL, ++ .u.signal = { ++ .sender = TEST_CONN_SERVICE, ++ .path = EXAMPLE_PATH, ++ .iface = EXAMPLE_INTERFACE, ++ .member = FOO_SIGNAL, ++ .received_by_conn = 1, ++ .received_by_proxy = 1 ++ }, ++ }, ++ }, ++}; ++ ++typedef struct ++{ ++ const TestPlan *plan; ++ SubscriptionMode mode; ++ GError *error; ++ /* (element-type ReceivedMessage) */ ++ GPtrArray *received; ++ /* conns[TEST_CONN_NONE] is unused and remains NULL */ ++ GDBusConnection *conns[NUM_TEST_CONNS]; ++ /* Proxies on conns[TEST_CONN_SUBSCRIBER] */ ++ GPtrArray *proxies; ++ /* unique_names[TEST_CONN_NONE] is unused and remains NULL */ ++ const char *unique_names[NUM_TEST_CONNS]; ++ /* finished[TEST_CONN_NONE] is unused and remains FALSE */ ++ gboolean finished[NUM_TEST_CONNS]; ++ /* Remains 0 for any step that is not a subscription */ ++ guint subscriptions[MAX_TEST_STEPS]; ++ /* Number of times the signal from step n was received */ ++ guint received_by_conn[MAX_TEST_STEPS]; ++ /* Number of times the signal from step n was received */ ++ guint received_by_proxy[MAX_TEST_STEPS]; ++ guint finished_subscription; ++} Fixture; ++ ++/* Wait for asynchronous messages from @conn to have been processed ++ * by the message bus, as a sequence point so that we can make ++ * "happens before" and "happens after" assertions relative to this. ++ * The easiest way to achieve this is to call a message bus method that has ++ * no arguments and wait for it to return: because the message bus processes ++ * messages in-order, anything we sent before this must have been processed ++ * by the time this call arrives. */ ++static void ++connection_wait_for_bus (GDBusConnection *conn) ++{ ++ GError *error = NULL; ++ GVariant *call_result; ++ ++ call_result = g_dbus_connection_call_sync (conn, ++ DBUS_SERVICE_DBUS, ++ DBUS_PATH_DBUS, ++ DBUS_INTERFACE_DBUS, ++ "GetId", ++ NULL, /* arguments */ ++ NULL, /* result type */ ++ G_DBUS_CALL_FLAGS_NONE, ++ -1, ++ NULL, ++ &error); ++ g_assert_no_error (error); ++ g_assert_nonnull (call_result); ++ g_variant_unref (call_result); ++} ++ ++/* ++ * Called when the subscriber receives a message from any connection ++ * announcing that it has emitted all the signals that it plans to emit. ++ */ ++static void ++subscriber_finished_cb (GDBusConnection *conn, ++ const char *sender_name, ++ const char *path, ++ const char *iface, ++ const char *member, ++ GVariant *parameters, ++ void *user_data) ++{ ++ Fixture *f = user_data; ++ GDBusConnection *subscriber = f->conns[TEST_CONN_SUBSCRIBER]; ++ guint i; ++ ++ g_assert_true (conn == subscriber); ++ ++ for (i = TEST_CONN_FIRST; i < G_N_ELEMENTS (f->conns); i++) ++ { ++ if (g_str_equal (sender_name, f->unique_names[i])) ++ { ++ g_assert_false (f->finished[i]); ++ f->finished[i] = TRUE; ++ ++ g_test_message ("Received Finished signal from %s %s", ++ test_conn_descriptions[i], sender_name); ++ return; ++ } ++ } ++ ++ g_error ("Received Finished signal from unknown sender %s", sender_name); ++} ++ ++/* ++ * Called when we receive a signal, either via the GDBusProxy (proxy != NULL) ++ * or via the GDBusConnection (proxy == NULL). ++ */ ++static void ++fixture_received_signal (Fixture *f, ++ GDBusProxy *proxy, ++ const char *sender_name, ++ const char *path, ++ const char *iface, ++ const char *member, ++ GVariant *parameters) ++{ ++ guint i; ++ ReceivedMessage *received; ++ ++ /* Ignore the Finished signal if it matches a wildcard subscription */ ++ if (g_str_equal (member, FINISHED_SIGNAL)) ++ return; ++ ++ received = g_new0 (ReceivedMessage, 1); ++ ++ if (proxy != NULL) ++ received->received_by_proxy = g_object_ref (proxy); ++ else ++ received->received_by_proxy = NULL; ++ ++ received->path = g_strdup (path); ++ received->iface = g_strdup (iface); ++ received->member = g_strdup (member); ++ received->parameters = g_variant_ref (parameters); ++ ++ for (i = TEST_CONN_FIRST; i < G_N_ELEMENTS (f->conns); i++) ++ { ++ if (g_str_equal (sender_name, f->unique_names[i])) ++ { ++ received->sender = i; ++ g_assert_false (f->finished[i]); ++ break; ++ } ++ } ++ ++ g_assert_cmpint (received->sender, !=, TEST_CONN_NONE); ++ ++ g_test_message ("Signal received from %s %s via %s", ++ test_conn_descriptions[received->sender], ++ sender_name, ++ proxy != NULL ? "proxy" : "connection"); ++ g_test_message ("\tPath: %s", path); ++ g_test_message ("\tInterface: %s", iface); ++ g_test_message ("\tMember: %s", member); ++ ++ if (g_variant_is_of_type (parameters, G_VARIANT_TYPE ("(su)"))) ++ { ++ g_variant_get (parameters, "(su)", &received->arg0, &received->step); ++ g_test_message ("\tString argument 0: %s", received->arg0); ++ g_test_message ("\tSent in step: %u", received->step); ++ } ++ else ++ { ++ g_assert_cmpstr (g_variant_get_type_string (parameters), ==, "(uu)"); ++ g_variant_get (parameters, "(uu)", NULL, &received->step); ++ g_test_message ("\tArgument 0: (not a string)"); ++ g_test_message ("\tSent in step: %u", received->step); ++ } ++ ++ g_ptr_array_add (f->received, g_steal_pointer (&received)); ++} ++ ++static void ++proxy_signal_cb (GDBusProxy *proxy, ++ const char *sender_name, ++ const char *member, ++ GVariant *parameters, ++ void *user_data) ++{ ++ Fixture *f = user_data; ++ ++ fixture_received_signal (f, proxy, sender_name, ++ g_dbus_proxy_get_object_path (proxy), ++ g_dbus_proxy_get_interface_name (proxy), ++ member, parameters); ++} ++ ++static void ++subscribed_signal_cb (GDBusConnection *conn, ++ const char *sender_name, ++ const char *path, ++ const char *iface, ++ const char *member, ++ GVariant *parameters, ++ void *user_data) ++{ ++ Fixture *f = user_data; ++ GDBusConnection *subscriber = f->conns[TEST_CONN_SUBSCRIBER]; ++ ++ g_assert_true (conn == subscriber); ++ ++ fixture_received_signal (f, NULL, sender_name, path, iface, member, parameters); ++} ++ ++static void ++fixture_subscribe (Fixture *f, ++ const TestSubscribe *subscribe, ++ guint step_number) ++{ ++ GDBusConnection *subscriber = f->conns[TEST_CONN_SUBSCRIBER]; ++ const char *sender; ++ ++ if (subscribe->sender != TEST_CONN_NONE) ++ { ++ sender = f->unique_names[subscribe->sender]; ++ g_test_message ("\tSender: %s %s", ++ test_conn_descriptions[subscribe->sender], ++ sender); ++ } ++ else ++ { ++ sender = NULL; ++ g_test_message ("\tSender: (any)"); ++ } ++ ++ g_test_message ("\tPath: %s", nonnull (subscribe->path, "(any)")); ++ g_test_message ("\tInterface: %s", ++ nonnull (subscribe->iface, "(any)")); ++ g_test_message ("\tMember: %s", ++ nonnull (subscribe->member, "(any)")); ++ g_test_message ("\tString argument 0: %s", ++ nonnull (subscribe->arg0, "(any)")); ++ g_test_message ("\tFlags: %x", subscribe->flags); ++ ++ if (f->mode != SUBSCRIPTION_MODE_PROXY) ++ { ++ /* CONN or PARALLEL */ ++ guint id; ++ ++ g_test_message ("\tSubscribing via connection"); ++ id = g_dbus_connection_signal_subscribe (subscriber, ++ sender, ++ subscribe->iface, ++ subscribe->member, ++ subscribe->path, ++ subscribe->arg0, ++ subscribe->flags, ++ subscribed_signal_cb, ++ f, NULL); ++ g_assert_cmpuint (id, !=, 0); ++ f->subscriptions[step_number] = id; ++ } ++ ++ if (f->mode != SUBSCRIPTION_MODE_CONN) ++ { ++ /* PROXY or PARALLEL */ ++ ++ if (sender == NULL) ++ { ++ g_test_message ("\tCannot subscribe via proxy: no bus name"); ++ } ++ else if (subscribe->path == NULL) ++ { ++ g_test_message ("\tCannot subscribe via proxy: no path"); ++ } ++ else if (subscribe->iface == NULL) ++ { ++ g_test_message ("\tCannot subscribe via proxy: no interface"); ++ } ++ else ++ { ++ GDBusProxy *proxy; ++ ++ g_test_message ("\tSubscribing via proxy"); ++ proxy = g_dbus_proxy_new_sync (subscriber, ++ (G_DBUS_PROXY_FLAGS_DO_NOT_LOAD_PROPERTIES ++ | G_DBUS_PROXY_FLAGS_DO_NOT_AUTO_START), ++ NULL, /* GDBusInterfaceInfo */ ++ sender, ++ subscribe->path, ++ subscribe->iface, ++ NULL, /* GCancellable */ ++ &f->error); ++ g_assert_no_error (f->error); ++ g_assert_nonnull (proxy); ++ g_signal_connect (proxy, "g-signal", G_CALLBACK (proxy_signal_cb), f); ++ g_ptr_array_add (f->proxies, g_steal_pointer (&proxy)); ++ } ++ } ++ ++ /* As in setup(), we need to wait for AddMatch to happen. */ ++ g_test_message ("Waiting for AddMatch to be processed"); ++ connection_wait_for_bus (subscriber); ++} ++ ++static void ++fixture_emit_signal (Fixture *f, ++ const TestEmitSignal *signal, ++ guint step_number) ++{ ++ GVariant *body; ++ const char *destination; ++ gboolean ok; ++ ++ g_test_message ("\tSender: %s", ++ test_conn_descriptions[signal->sender]); ++ ++ if (signal->unicast_to != TEST_CONN_NONE) ++ { ++ destination = f->unique_names[signal->unicast_to]; ++ g_test_message ("\tDestination: %s %s", ++ test_conn_descriptions[signal->unicast_to], ++ destination); ++ } ++ else ++ { ++ destination = NULL; ++ g_test_message ("\tDestination: (broadcast)"); ++ } ++ ++ g_assert_nonnull (signal->path); ++ g_test_message ("\tPath: %s", signal->path); ++ g_assert_nonnull (signal->iface); ++ g_test_message ("\tInterface: %s", signal->iface); ++ g_assert_nonnull (signal->member); ++ g_test_message ("\tMember: %s", signal->member); ++ ++ /* If arg0 is non-NULL, put it in the message's argument 0. ++ * Otherwise put something that will not match any arg0. ++ * Either way, put the sequence number in argument 1 so we can ++ * correlate sent messages with received messages later. */ ++ if (signal->arg0 != NULL) ++ { ++ g_test_message ("\tString argument 0: %s", signal->arg0); ++ /* floating */ ++ body = g_variant_new ("(su)", signal->arg0, (guint32) step_number); ++ } ++ else ++ { ++ g_test_message ("\tArgument 0: (not a string)"); ++ body = g_variant_new ("(uu)", (guint32) 0, (guint32) step_number); ++ } ++ ++ ok = g_dbus_connection_emit_signal (f->conns[signal->sender], ++ destination, ++ signal->path, ++ signal->iface, ++ signal->member, ++ /* steals floating reference */ ++ g_steal_pointer (&body), ++ &f->error); ++ g_assert_no_error (f->error); ++ g_assert_true (ok); ++ ++ /* Emitting the signal is asynchronous, so if we want subsequent steps ++ * to be guaranteed to happen after the signal from the message bus's ++ * perspective, we have to do a round-trip to the message bus to sync up. */ ++ g_test_message ("Waiting for signal to reach message bus"); ++ connection_wait_for_bus (f->conns[signal->sender]); ++} ++ ++static void ++fixture_run_plan (Fixture *f, ++ const TestPlan *plan, ++ SubscriptionMode mode) ++{ ++ guint i; ++ ++ G_STATIC_ASSERT (G_N_ELEMENTS (plan->steps) == G_N_ELEMENTS (f->subscriptions)); ++ G_STATIC_ASSERT (G_N_ELEMENTS (plan->steps) == G_N_ELEMENTS (f->received_by_conn)); ++ G_STATIC_ASSERT (G_N_ELEMENTS (plan->steps) == G_N_ELEMENTS (f->received_by_proxy)); ++ ++ f->mode = mode; ++ f->plan = plan; ++ ++ g_test_summary (plan->description); ++ ++ for (i = 0; i < G_N_ELEMENTS (plan->steps); i++) ++ { ++ const TestStep *step = &plan->steps[i]; ++ ++ switch (step->action) ++ { ++ case TEST_ACTION_SUBSCRIBE: ++ g_test_message ("Step %u: adding subscription", i); ++ fixture_subscribe (f, &step->u.subscribe, i); ++ break; ++ ++ case TEST_ACTION_EMIT_SIGNAL: ++ g_test_message ("Step %u: emitting signal", i); ++ fixture_emit_signal (f, &step->u.signal, i); ++ break; ++ ++ case TEST_ACTION_NONE: ++ /* Padding to fill the rest of the array, do nothing */ ++ break; ++ ++ default: ++ g_return_if_reached (); ++ } ++ } ++ ++ /* Now that we have done everything we wanted to do, emit Finished ++ * from each connection. */ ++ for (i = TEST_CONN_FIRST; i < G_N_ELEMENTS (f->conns); i++) ++ { ++ gboolean ok; ++ ++ ok = g_dbus_connection_emit_signal (f->conns[i], ++ NULL, ++ FINISHED_PATH, ++ FINISHED_INTERFACE, ++ FINISHED_SIGNAL, ++ NULL, ++ &f->error); ++ g_assert_no_error (f->error); ++ g_assert_true (ok); ++ } ++ ++ /* Wait until we have seen the Finished signal from each sender */ ++ while (TRUE) ++ { ++ gboolean all_finished = TRUE; ++ ++ for (i = TEST_CONN_FIRST; i < G_N_ELEMENTS (f->conns); i++) ++ all_finished = all_finished && f->finished[i]; ++ ++ if (all_finished) ++ break; ++ ++ g_main_context_iteration (NULL, TRUE); ++ } ++ ++ /* Assert that the correct things happened before each Finished signal */ ++ for (i = 0; i < f->received->len; i++) ++ { ++ const ReceivedMessage *received = g_ptr_array_index (f->received, i); ++ ++ g_assert_cmpuint (received->step, <, G_N_ELEMENTS (f->received_by_conn)); ++ g_assert_cmpuint (received->step, <, G_N_ELEMENTS (f->received_by_proxy)); ++ g_assert_cmpint (plan->steps[received->step].action, ++ ==, TEST_ACTION_EMIT_SIGNAL); ++ ++ if (received->received_by_proxy != NULL) ++ f->received_by_proxy[received->step] += 1; ++ else ++ f->received_by_conn[received->step] += 1; ++ } ++ ++ for (i = 0; i < G_N_ELEMENTS (plan->steps); i++) ++ { ++ const TestStep *step = &plan->steps[i]; ++ ++ if (step->action == TEST_ACTION_EMIT_SIGNAL) ++ { ++ const TestEmitSignal *signal = &plan->steps[i].u.signal; ++ ++ if (mode != SUBSCRIPTION_MODE_PROXY) ++ { ++ g_test_message ("Signal from step %u was received %u times by " ++ "GDBusConnection, expected %u", ++ i, f->received_by_conn[i], signal->received_by_conn); ++ g_assert_cmpuint (f->received_by_conn[i], ==, signal->received_by_conn); ++ } ++ else ++ { ++ g_assert_cmpuint (f->received_by_conn[i], ==, 0); ++ } ++ ++ if (mode != SUBSCRIPTION_MODE_CONN) ++ { ++ g_test_message ("Signal from step %u was received %u times by " ++ "GDBusProxy, expected %u", ++ i, f->received_by_proxy[i], signal->received_by_proxy); ++ g_assert_cmpuint (f->received_by_proxy[i], ==, signal->received_by_proxy); ++ } ++ else ++ { ++ g_assert_cmpuint (f->received_by_proxy[i], ==, 0); ++ } ++ } ++ } ++} ++ ++static void ++setup (Fixture *f, ++ G_GNUC_UNUSED const void *context) ++{ ++ GDBusConnection *subscriber; ++ guint i; ++ ++ session_bus_up (); ++ ++ f->proxies = g_ptr_array_new_full (MAX_TEST_STEPS, g_object_unref); ++ f->received = g_ptr_array_new_full (MAX_TEST_STEPS, ++ (GDestroyNotify) received_message_free); ++ ++ for (i = TEST_CONN_FIRST; i < G_N_ELEMENTS (f->conns); i++) ++ { ++ f->conns[i] = _g_bus_get_priv (G_BUS_TYPE_SESSION, NULL, &f->error); ++ g_assert_no_error (f->error); ++ g_assert_nonnull (f->conns[i]); ++ ++ f->unique_names[i] = g_dbus_connection_get_unique_name (f->conns[i]); ++ g_assert_nonnull (f->unique_names[i]); ++ g_test_message ("%s is %s", ++ test_conn_descriptions[i], ++ f->unique_names[i]); ++ } ++ ++ subscriber = f->conns[TEST_CONN_SUBSCRIBER]; ++ ++ /* Used to wait for all connections to finish sending whatever they ++ * wanted to send */ ++ f->finished_subscription = g_dbus_connection_signal_subscribe (subscriber, ++ NULL, ++ FINISHED_INTERFACE, ++ FINISHED_SIGNAL, ++ FINISHED_PATH, ++ NULL, ++ G_DBUS_SIGNAL_FLAGS_NONE, ++ subscriber_finished_cb, ++ f, NULL); ++ /* AddMatch is sent asynchronously, so we don't know how ++ * soon it will be processed. Before emitting signals, we ++ * need to wait for the message bus to get as far as processing ++ * AddMatch. */ ++ g_test_message ("Waiting for AddMatch to be processed"); ++ connection_wait_for_bus (subscriber); ++} ++ ++static void ++test_conn_subscribe (Fixture *f, ++ const void *context) ++{ ++ fixture_run_plan (f, context, SUBSCRIPTION_MODE_CONN); ++} ++ ++static void ++test_proxy_subscribe (Fixture *f, ++ const void *context) ++{ ++ fixture_run_plan (f, context, SUBSCRIPTION_MODE_PROXY); ++} ++ ++static void ++test_parallel_subscribe (Fixture *f, ++ const void *context) ++{ ++ fixture_run_plan (f, context, SUBSCRIPTION_MODE_PARALLEL); ++} ++ ++static void ++teardown (Fixture *f, ++ G_GNUC_UNUSED const void *context) ++{ ++ GDBusConnection *subscriber = f->conns[TEST_CONN_SUBSCRIBER]; ++ guint i; ++ ++ g_ptr_array_unref (f->proxies); ++ ++ if (f->finished_subscription != 0) ++ g_dbus_connection_signal_unsubscribe (subscriber, f->finished_subscription); ++ ++ for (i = 0; i < G_N_ELEMENTS (f->subscriptions); i++) ++ { ++ if (f->subscriptions[i] != 0) ++ g_dbus_connection_signal_unsubscribe (subscriber, f->subscriptions[i]); ++ } ++ ++ g_ptr_array_unref (f->received); ++ ++ for (i = TEST_CONN_FIRST; i < G_N_ELEMENTS (f->conns); i++) ++ g_clear_object (&f->conns[i]); ++ ++ g_clear_error (&f->error); ++ ++ session_bus_down (); ++} ++ ++int ++main (int argc, ++ char *argv[]) ++{ ++ g_test_init (&argc, &argv, G_TEST_OPTION_ISOLATE_DIRS, NULL); ++ ++ g_test_dbus_unset (); ++ ++#define ADD_SUBSCRIBE_TEST(name) \ ++ do { \ ++ g_test_add ("/gdbus/subscribe/conn/" #name, \ ++ Fixture, &plan_ ## name, \ ++ setup, test_conn_subscribe, teardown); \ ++ g_test_add ("/gdbus/subscribe/proxy/" #name, \ ++ Fixture, &plan_ ## name, \ ++ setup, test_proxy_subscribe, teardown); \ ++ g_test_add ("/gdbus/subscribe/parallel/" #name, \ ++ Fixture, &plan_ ## name, \ ++ setup, test_parallel_subscribe, teardown); \ ++ } while (0) ++ ++ ADD_SUBSCRIBE_TEST (simple); ++ ADD_SUBSCRIBE_TEST (broadcast_from_anyone); ++ ADD_SUBSCRIBE_TEST (match_twice); ++ ADD_SUBSCRIBE_TEST (limit_by_unique_name); ++ ++ return g_test_run(); ++} +diff --git a/gio/tests/meson.build b/gio/tests/meson.build +index fb4ffcf7d..cfa9099c9 100644 +--- a/gio/tests/meson.build ++++ b/gio/tests/meson.build +@@ -188,6 +188,7 @@ if host_machine.system() != 'windows' + ['gdbus-proxy', [], []], + ['gdbus-proxy-threads', [], [dbus1_dep]], + ['gdbus-proxy-well-known-name', [], []], ++ ['gdbus-subscribe', [], []], + ['gdbus-test-codegen', [gdbus_test_codegen_generated], []], + ['gdbus-threading', [], []], + ['gmenumodel', [], []], +-- +2.50.0 + + +From 470bf89e7dd1e1ce9c509a825ddf270d4249cc72 Mon Sep 17 00:00:00 2001 +From: Simon McVittie +Date: Fri, 8 Mar 2024 19:28:15 +0000 +Subject: [PATCH 12/17] tests: Add support for subscribing to signals from a + well-known name + +Signed-off-by: Simon McVittie +--- + gio/tests/gdbus-subscribe.c | 133 ++++++++++++++++++++++++++++++++++-- + 1 file changed, 126 insertions(+), 7 deletions(-) + +diff --git a/gio/tests/gdbus-subscribe.c b/gio/tests/gdbus-subscribe.c +index 3f53e1d7f..3d2a14e03 100644 +--- a/gio/tests/gdbus-subscribe.c ++++ b/gio/tests/gdbus-subscribe.c +@@ -7,6 +7,9 @@ + + #include "gdbus-tests.h" + ++/* From the D-Bus Specification */ ++#define DBUS_REQUEST_NAME_REPLY_PRIMARY_OWNER 1 ++ + #define DBUS_SERVICE_DBUS "org.freedesktop.DBus" + #define DBUS_PATH_DBUS "/org/freedesktop/DBus" + #define DBUS_INTERFACE_DBUS DBUS_SERVICE_DBUS +@@ -22,6 +25,9 @@ + #define EXAMPLE_INTERFACE "org.gtk.GDBus.ExampleInterface" + #define FOO_SIGNAL "Foo" + ++#define ALREADY_OWNED_NAME "org.gtk.Test.AlreadyOwned" ++#define OWNED_LATER_NAME "org.gtk.Test.OwnedLater" ++ + /* Log @s in a debug message. */ + static inline const char * + nonnull (const char *s, +@@ -101,7 +107,8 @@ typedef struct + + typedef struct + { +- TestConn sender; ++ const char *string_sender; ++ TestConn unique_sender; + const char *path; + const char *iface; + const char *member; +@@ -109,11 +116,18 @@ typedef struct + GDBusSignalFlags flags; + } TestSubscribe; + ++typedef struct ++{ ++ const char *name; ++ TestConn owner; ++} TestOwnName; ++ + typedef enum + { + TEST_ACTION_NONE = 0, + TEST_ACTION_SUBSCRIBE, + TEST_ACTION_EMIT_SIGNAL, ++ TEST_ACTION_OWN_NAME, + } TestAction; + + typedef struct +@@ -122,6 +136,7 @@ typedef struct + union { + TestEmitSignal signal; + TestSubscribe subscribe; ++ TestOwnName own_name; + } u; + } TestStep; + +@@ -247,7 +262,7 @@ static const TestPlan plan_match_twice = + { + .action = TEST_ACTION_SUBSCRIBE, + .u.subscribe = { +- .sender = TEST_CONN_SERVICE, ++ .unique_sender = TEST_CONN_SERVICE, + .path = EXAMPLE_PATH, + .iface = EXAMPLE_INTERFACE, + }, +@@ -267,7 +282,7 @@ static const TestPlan plan_match_twice = + { + .action = TEST_ACTION_SUBSCRIBE, + .u.subscribe = { +- .sender = TEST_CONN_SERVICE, ++ .unique_sender = TEST_CONN_SERVICE, + .path = EXAMPLE_PATH, + .iface = EXAMPLE_INTERFACE, + }, +@@ -296,7 +311,7 @@ static const TestPlan plan_limit_by_unique_name = + /* Subscriber wants to receive signals from service */ + .action = TEST_ACTION_SUBSCRIBE, + .u.subscribe = { +- .sender = TEST_CONN_SERVICE, ++ .unique_sender = TEST_CONN_SERVICE, + .path = EXAMPLE_PATH, + .iface = EXAMPLE_INTERFACE, + }, +@@ -343,6 +358,62 @@ static const TestPlan plan_limit_by_unique_name = + }, + }; + ++static const TestPlan plan_limit_by_well_known_name = ++{ ++ .description = "A subscription via a well-known name only accepts messages " ++ "sent by the owner of that well-known name", ++ .steps = { ++ { ++ /* Service already owns one name */ ++ .action = TEST_ACTION_OWN_NAME, ++ .u.own_name = { ++ .name = ALREADY_OWNED_NAME, ++ .owner = TEST_CONN_SERVICE ++ }, ++ }, ++ { ++ /* Subscriber wants to receive signals from service */ ++ .action = TEST_ACTION_SUBSCRIBE, ++ .u.subscribe = { ++ .string_sender = ALREADY_OWNED_NAME, ++ .path = EXAMPLE_PATH, ++ .iface = EXAMPLE_INTERFACE, ++ }, ++ }, ++ { ++ /* Subscriber wants to receive signals from service by another name */ ++ .action = TEST_ACTION_SUBSCRIBE, ++ .u.subscribe = { ++ .string_sender = OWNED_LATER_NAME, ++ .path = EXAMPLE_PATH, ++ .iface = EXAMPLE_INTERFACE, ++ }, ++ }, ++ { ++ /* Service claims another name */ ++ .action = TEST_ACTION_OWN_NAME, ++ .u.own_name = { ++ .name = OWNED_LATER_NAME, ++ .owner = TEST_CONN_SERVICE ++ }, ++ }, ++ { ++ /* Now the subscriber gets this signal twice, once for each ++ * subscription; and similarly each of the two proxies gets this ++ * signal twice */ ++ .action = TEST_ACTION_EMIT_SIGNAL, ++ .u.signal = { ++ .sender = TEST_CONN_SERVICE, ++ .path = EXAMPLE_PATH, ++ .iface = EXAMPLE_INTERFACE, ++ .member = FOO_SIGNAL, ++ .received_by_conn = 2, ++ .received_by_proxy = 2 ++ }, ++ }, ++ }, ++}; ++ + typedef struct + { + const TestPlan *plan; +@@ -540,11 +611,16 @@ fixture_subscribe (Fixture *f, + GDBusConnection *subscriber = f->conns[TEST_CONN_SUBSCRIBER]; + const char *sender; + +- if (subscribe->sender != TEST_CONN_NONE) ++ if (subscribe->string_sender != NULL) ++ { ++ sender = subscribe->string_sender; ++ g_test_message ("\tSender: %s", sender); ++ } ++ else if (subscribe->unique_sender != TEST_CONN_NONE) + { +- sender = f->unique_names[subscribe->sender]; ++ sender = f->unique_names[subscribe->unique_sender]; + g_test_message ("\tSender: %s %s", +- test_conn_descriptions[subscribe->sender], ++ test_conn_descriptions[subscribe->unique_sender], + sender); + } + else +@@ -689,6 +765,43 @@ fixture_emit_signal (Fixture *f, + connection_wait_for_bus (f->conns[signal->sender]); + } + ++static void ++fixture_own_name (Fixture *f, ++ const TestOwnName *own_name) ++{ ++ GVariant *call_result; ++ guint32 flags; ++ guint32 result_code; ++ ++ g_test_message ("\tName: %s", own_name->name); ++ g_test_message ("\tOwner: %s", ++ test_conn_descriptions[own_name->owner]); ++ ++ /* For simplicity, we do this via a direct bus call rather than ++ * using g_bus_own_name_on_connection(). The flags in ++ * GBusNameOwnerFlags are numerically equal to those in the ++ * D-Bus wire protocol. */ ++ flags = G_BUS_NAME_OWNER_FLAGS_DO_NOT_QUEUE; ++ call_result = g_dbus_connection_call_sync (f->conns[own_name->owner], ++ DBUS_SERVICE_DBUS, ++ DBUS_PATH_DBUS, ++ DBUS_INTERFACE_DBUS, ++ "RequestName", ++ g_variant_new ("(su)", ++ own_name->name, ++ flags), ++ G_VARIANT_TYPE ("(u)"), ++ G_DBUS_CALL_FLAGS_NONE, ++ -1, ++ NULL, ++ &f->error); ++ g_assert_no_error (f->error); ++ g_assert_nonnull (call_result); ++ g_variant_get (call_result, "(u)", &result_code); ++ g_assert_cmpuint (result_code, ==, DBUS_REQUEST_NAME_REPLY_PRIMARY_OWNER); ++ g_variant_unref (call_result); ++} ++ + static void + fixture_run_plan (Fixture *f, + const TestPlan *plan, +@@ -721,6 +834,11 @@ fixture_run_plan (Fixture *f, + fixture_emit_signal (f, &step->u.signal, i); + break; + ++ case TEST_ACTION_OWN_NAME: ++ g_test_message ("Step %u: claiming bus name", i); ++ fixture_own_name (f, &step->u.own_name); ++ break; ++ + case TEST_ACTION_NONE: + /* Padding to fill the rest of the array, do nothing */ + break; +@@ -933,6 +1051,7 @@ main (int argc, + ADD_SUBSCRIBE_TEST (broadcast_from_anyone); + ADD_SUBSCRIBE_TEST (match_twice); + ADD_SUBSCRIBE_TEST (limit_by_unique_name); ++ ADD_SUBSCRIBE_TEST (limit_by_well_known_name); + + return g_test_run(); + } +-- +2.50.0 + + +From 6d04728abeded45ba7b90f26636263a1a8b3794d Mon Sep 17 00:00:00 2001 +From: Simon McVittie +Date: Fri, 8 Mar 2024 19:44:03 +0000 +Subject: [PATCH 13/17] tests: Add a test-case for what happens if a unique + name doesn't exist + +On GNOME/glib#3268 there was some concern about whether this would +allow an attacker to send signals and have them be matched to a +GDBusProxy in this situation, but it seems that was a false alarm. + +Signed-off-by: Simon McVittie +--- + gio/tests/gdbus-subscribe.c | 48 +++++++++++++++++++++++++++++++++++++ + 1 file changed, 48 insertions(+) + +diff --git a/gio/tests/gdbus-subscribe.c b/gio/tests/gdbus-subscribe.c +index 3d2a14e03..350ec9f52 100644 +--- a/gio/tests/gdbus-subscribe.c ++++ b/gio/tests/gdbus-subscribe.c +@@ -358,6 +358,53 @@ static const TestPlan plan_limit_by_unique_name = + }, + }; + ++static const TestPlan plan_nonexistent_unique_name = ++{ ++ .description = "A subscription via a unique name that doesn't exist " ++ "accepts no messages", ++ .steps = { ++ { ++ /* Subscriber wants to receive signals from service */ ++ .action = TEST_ACTION_SUBSCRIBE, ++ .u.subscribe = { ++ /* This relies on the implementation detail that the dbus-daemon ++ * (and presumably other bus implementations) never actually generates ++ * a unique name in this format */ ++ .string_sender = ":0.this.had.better.not.exist", ++ .path = EXAMPLE_PATH, ++ .iface = EXAMPLE_INTERFACE, ++ }, ++ }, ++ { ++ /* Attacker wants to trick subscriber into thinking that service ++ * sent a signal */ ++ .action = TEST_ACTION_EMIT_SIGNAL, ++ .u.signal = { ++ .sender = TEST_CONN_ATTACKER, ++ .path = EXAMPLE_PATH, ++ .iface = EXAMPLE_INTERFACE, ++ .member = FOO_SIGNAL, ++ .received_by_conn = 0, ++ .received_by_proxy = 0 ++ }, ++ }, ++ { ++ /* Attacker tries harder, by sending a signal unicast directly to ++ * the subscriber */ ++ .action = TEST_ACTION_EMIT_SIGNAL, ++ .u.signal = { ++ .sender = TEST_CONN_ATTACKER, ++ .unicast_to = TEST_CONN_SUBSCRIBER, ++ .path = EXAMPLE_PATH, ++ .iface = EXAMPLE_INTERFACE, ++ .member = FOO_SIGNAL, ++ .received_by_conn = 0, ++ .received_by_proxy = 0 ++ }, ++ }, ++ }, ++}; ++ + static const TestPlan plan_limit_by_well_known_name = + { + .description = "A subscription via a well-known name only accepts messages " +@@ -1051,6 +1098,7 @@ main (int argc, + ADD_SUBSCRIBE_TEST (broadcast_from_anyone); + ADD_SUBSCRIBE_TEST (match_twice); + ADD_SUBSCRIBE_TEST (limit_by_unique_name); ++ ADD_SUBSCRIBE_TEST (nonexistent_unique_name); + ADD_SUBSCRIBE_TEST (limit_by_well_known_name); + + return g_test_run(); +-- +2.50.0 + + +From e80d5003c028d2263f5ca6a5b67d407a23ba7840 Mon Sep 17 00:00:00 2001 +From: Simon McVittie +Date: Fri, 8 Mar 2024 20:10:29 +0000 +Subject: [PATCH 14/17] tests: Add test coverage for signals that match the + message bus's name + +This is a special case of unique names, even though it's syntactically +a well-known name. + +Signed-off-by: Simon McVittie +--- + gio/tests/gdbus-subscribe.c | 161 ++++++++++++++++++++++++++++++++++-- + 1 file changed, 154 insertions(+), 7 deletions(-) + +diff --git a/gio/tests/gdbus-subscribe.c b/gio/tests/gdbus-subscribe.c +index 350ec9f52..af100de7d 100644 +--- a/gio/tests/gdbus-subscribe.c ++++ b/gio/tests/gdbus-subscribe.c +@@ -13,6 +13,7 @@ + #define DBUS_SERVICE_DBUS "org.freedesktop.DBus" + #define DBUS_PATH_DBUS "/org/freedesktop/DBus" + #define DBUS_INTERFACE_DBUS DBUS_SERVICE_DBUS ++#define NAME_OWNER_CHANGED "NameOwnerChanged" + + /* A signal that each connection emits to indicate that it has finished + * emitting other signals */ +@@ -101,6 +102,7 @@ typedef struct + const char *iface; + const char *member; + const char *arg0; ++ const char *args; + guint received_by_conn; + guint received_by_proxy; + } TestEmitSignal; +@@ -120,6 +122,8 @@ typedef struct + { + const char *name; + TestConn owner; ++ guint received_by_conn; ++ guint received_by_proxy; + } TestOwnName; + + typedef enum +@@ -461,6 +465,63 @@ static const TestPlan plan_limit_by_well_known_name = + }, + }; + ++static const TestPlan plan_limit_to_message_bus = ++{ ++ .description = "A subscription to the message bus only accepts messages " ++ "from the message bus", ++ .steps = { ++ { ++ /* Subscriber wants to receive signals from the message bus itself */ ++ .action = TEST_ACTION_SUBSCRIBE, ++ .u.subscribe = { ++ .string_sender = DBUS_SERVICE_DBUS, ++ .path = DBUS_PATH_DBUS, ++ .iface = DBUS_INTERFACE_DBUS, ++ }, ++ }, ++ { ++ /* Attacker wants to trick subscriber into thinking that the message ++ * bus sent a signal */ ++ .action = TEST_ACTION_EMIT_SIGNAL, ++ .u.signal = { ++ .sender = TEST_CONN_ATTACKER, ++ .path = DBUS_PATH_DBUS, ++ .iface = DBUS_INTERFACE_DBUS, ++ .member = NAME_OWNER_CHANGED, ++ .arg0 = "would I lie to you?", ++ .received_by_conn = 0, ++ .received_by_proxy = 0 ++ }, ++ }, ++ { ++ /* Attacker tries harder, by sending a signal unicast directly to ++ * the subscriber, and using more realistic arguments */ ++ .action = TEST_ACTION_EMIT_SIGNAL, ++ .u.signal = { ++ .unicast_to = TEST_CONN_SUBSCRIBER, ++ .sender = TEST_CONN_ATTACKER, ++ .path = DBUS_PATH_DBUS, ++ .iface = DBUS_INTERFACE_DBUS, ++ .member = NAME_OWNER_CHANGED, ++ .args = "('com.example.Name', '', ':1.12')", ++ .received_by_conn = 0, ++ .received_by_proxy = 0 ++ }, ++ }, ++ { ++ /* When the message bus sends a signal (in this case triggered by ++ * owning a name), it should still get through */ ++ .action = TEST_ACTION_OWN_NAME, ++ .u.own_name = { ++ .name = OWNED_LATER_NAME, ++ .owner = TEST_CONN_SERVICE, ++ .received_by_conn = 1, ++ .received_by_proxy = 1 ++ }, ++ }, ++ }, ++}; ++ + typedef struct + { + const TestPlan *plan; +@@ -591,7 +652,18 @@ fixture_received_signal (Fixture *f, + } + } + +- g_assert_cmpint (received->sender, !=, TEST_CONN_NONE); ++ if (g_str_equal (sender_name, DBUS_SERVICE_DBUS)) ++ { ++ g_test_message ("Signal received from message bus %s", ++ sender_name); ++ } ++ else ++ { ++ g_test_message ("Signal received from %s %s", ++ test_conn_descriptions[received->sender], ++ sender_name); ++ g_assert_cmpint (received->sender, !=, TEST_CONN_NONE); ++ } + + g_test_message ("Signal received from %s %s via %s", + test_conn_descriptions[received->sender], +@@ -607,13 +679,56 @@ fixture_received_signal (Fixture *f, + g_test_message ("\tString argument 0: %s", received->arg0); + g_test_message ("\tSent in step: %u", received->step); + } +- else ++ else if (g_variant_is_of_type (parameters, G_VARIANT_TYPE ("(uu)"))) + { +- g_assert_cmpstr (g_variant_get_type_string (parameters), ==, "(uu)"); + g_variant_get (parameters, "(uu)", NULL, &received->step); + g_test_message ("\tArgument 0: (not a string)"); + g_test_message ("\tSent in step: %u", received->step); + } ++ else if (g_variant_is_of_type (parameters, G_VARIANT_TYPE ("(sss)"))) ++ { ++ const char *name; ++ const char *old_owner; ++ const char *new_owner; ++ ++ /* The only signal of this signature that we legitimately receive ++ * during this test is NameOwnerChanged, so just assert that it ++ * is from the message bus and can be matched to a plausible step. ++ * (This is less thorough than the above, and will not work if we ++ * add a test scenario where a name's ownership is repeatedly ++ * changed while watching NameOwnerChanged - so don't do that.) */ ++ g_assert_cmpstr (sender_name, ==, DBUS_SERVICE_DBUS); ++ g_assert_cmpstr (path, ==, DBUS_PATH_DBUS); ++ g_assert_cmpstr (iface, ==, DBUS_INTERFACE_DBUS); ++ g_assert_cmpstr (member, ==, NAME_OWNER_CHANGED); ++ ++ g_variant_get (parameters, "(&s&s&s)", &name, &old_owner, &new_owner); ++ ++ for (i = 0; i < G_N_ELEMENTS (f->plan->steps); i++) ++ { ++ const TestStep *step = &f->plan->steps[i]; ++ ++ if (step->action == TEST_ACTION_OWN_NAME) ++ { ++ const TestOwnName *own_name = &step->u.own_name; ++ ++ if (g_str_equal (name, own_name->name) ++ && g_str_equal (new_owner, f->unique_names[own_name->owner]) ++ && own_name->received_by_conn > 0) ++ { ++ received->step = i; ++ break; ++ } ++ } ++ ++ if (i >= G_N_ELEMENTS (f->plan->steps)) ++ g_error ("Could not match message to a test step"); ++ } ++ } ++ else ++ { ++ g_error ("Unexpected message received"); ++ } + + g_ptr_array_add (f->received, g_steal_pointer (&received)); + } +@@ -782,10 +897,15 @@ fixture_emit_signal (Fixture *f, + * Otherwise put something that will not match any arg0. + * Either way, put the sequence number in argument 1 so we can + * correlate sent messages with received messages later. */ +- if (signal->arg0 != NULL) ++ if (signal->args != NULL) + { +- g_test_message ("\tString argument 0: %s", signal->arg0); + /* floating */ ++ body = g_variant_new_parsed (signal->args); ++ g_assert_nonnull (body); ++ } ++ else if (signal->arg0 != NULL) ++ { ++ g_test_message ("\tString argument 0: %s", signal->arg0); + body = g_variant_new ("(su)", signal->arg0, (guint32) step_number); + } + else +@@ -933,8 +1053,6 @@ fixture_run_plan (Fixture *f, + + g_assert_cmpuint (received->step, <, G_N_ELEMENTS (f->received_by_conn)); + g_assert_cmpuint (received->step, <, G_N_ELEMENTS (f->received_by_proxy)); +- g_assert_cmpint (plan->steps[received->step].action, +- ==, TEST_ACTION_EMIT_SIGNAL); + + if (received->received_by_proxy != NULL) + f->received_by_proxy[received->step] += 1; +@@ -974,6 +1092,34 @@ fixture_run_plan (Fixture *f, + g_assert_cmpuint (f->received_by_proxy[i], ==, 0); + } + } ++ else if (step->action == TEST_ACTION_OWN_NAME) ++ { ++ const TestOwnName *own_name = &plan->steps[i].u.own_name; ++ ++ if (mode != SUBSCRIPTION_MODE_PROXY) ++ { ++ g_test_message ("NameOwnerChanged from step %u was received %u " ++ "times by GDBusConnection, expected %u", ++ i, f->received_by_conn[i], own_name->received_by_conn); ++ g_assert_cmpuint (f->received_by_conn[i], ==, own_name->received_by_conn); ++ } ++ else ++ { ++ g_assert_cmpuint (f->received_by_conn[i], ==, 0); ++ } ++ ++ if (mode != SUBSCRIPTION_MODE_CONN) ++ { ++ g_test_message ("NameOwnerChanged from step %u was received %u " ++ "times by GDBusProxy, expected %u", ++ i, f->received_by_proxy[i], own_name->received_by_proxy); ++ g_assert_cmpuint (f->received_by_proxy[i], ==, own_name->received_by_proxy); ++ } ++ else ++ { ++ g_assert_cmpuint (f->received_by_proxy[i], ==, 0); ++ } ++ } + } + } + +@@ -1100,6 +1246,7 @@ main (int argc, + ADD_SUBSCRIBE_TEST (limit_by_unique_name); + ADD_SUBSCRIBE_TEST (nonexistent_unique_name); + ADD_SUBSCRIBE_TEST (limit_by_well_known_name); ++ ADD_SUBSCRIBE_TEST (limit_to_message_bus); + + return g_test_run(); + } +-- +2.50.0 + + +From 399343142a761319a4efee177ecac0dc171e3843 Mon Sep 17 00:00:00 2001 +From: Simon McVittie +Date: Fri, 8 Mar 2024 19:51:50 +0000 +Subject: [PATCH 15/17] tests: Add a test for matching by two well-known names + +The expected result is that because TEST_CONN_SERVICE owns +ALREADY_OWNED_NAME but not (yet) OWNED_LATER_NAME, the signal will be +delivered to the subscriber for the former but not the latter. +Before #3268 was fixed, it was incorrectly delivered to both. + +Reproduces: https://gitlab.gnome.org/GNOME/glib/-/issues/3268 (partially) +Signed-off-by: Simon McVittie +--- + gio/tests/gdbus-subscribe.c | 13 +++++++++++++ + 1 file changed, 13 insertions(+) + +diff --git a/gio/tests/gdbus-subscribe.c b/gio/tests/gdbus-subscribe.c +index af100de7d..171d6107d 100644 +--- a/gio/tests/gdbus-subscribe.c ++++ b/gio/tests/gdbus-subscribe.c +@@ -440,6 +440,19 @@ static const TestPlan plan_limit_by_well_known_name = + .iface = EXAMPLE_INTERFACE, + }, + }, ++ { ++ /* When the service sends a signal with the name it already owns, ++ * it should get through */ ++ .action = TEST_ACTION_EMIT_SIGNAL, ++ .u.signal = { ++ .sender = TEST_CONN_SERVICE, ++ .path = EXAMPLE_PATH, ++ .iface = EXAMPLE_INTERFACE, ++ .member = FOO_SIGNAL, ++ .received_by_conn = 1, ++ .received_by_proxy = 1 ++ }, ++ }, + { + /* Service claims another name */ + .action = TEST_ACTION_OWN_NAME, +-- +2.50.0 + + +From 7b8beb61c14211312ade5ec5dfc4c87f0751c0f7 Mon Sep 17 00:00:00 2001 +From: Simon McVittie +Date: Fri, 8 Mar 2024 19:53:22 +0000 +Subject: [PATCH 16/17] tests: Add a test for signal filtering by well-known + name + +The vulnerability reported as GNOME/glib#3268 can be characterized +as: these signals from an attacker should not be delivered to either +the GDBusConnection or the GDBusProxy, but in fact they are (in at +least some scenarios). + +Reproduces: https://gitlab.gnome.org/GNOME/glib/-/issues/3268 +Signed-off-by: Simon McVittie +--- + gio/tests/gdbus-subscribe.c | 27 +++++++++++++++++++++++++++ + 1 file changed, 27 insertions(+) + +diff --git a/gio/tests/gdbus-subscribe.c b/gio/tests/gdbus-subscribe.c +index 171d6107d..5406ba7e2 100644 +--- a/gio/tests/gdbus-subscribe.c ++++ b/gio/tests/gdbus-subscribe.c +@@ -440,6 +440,33 @@ static const TestPlan plan_limit_by_well_known_name = + .iface = EXAMPLE_INTERFACE, + }, + }, ++ { ++ /* Attacker wants to trick subscriber into thinking that service ++ * sent a signal */ ++ .action = TEST_ACTION_EMIT_SIGNAL, ++ .u.signal = { ++ .sender = TEST_CONN_ATTACKER, ++ .path = EXAMPLE_PATH, ++ .iface = EXAMPLE_INTERFACE, ++ .member = FOO_SIGNAL, ++ .received_by_conn = 0, ++ .received_by_proxy = 0 ++ }, ++ }, ++ { ++ /* Attacker tries harder, by sending a signal unicast directly to ++ * the subscriber */ ++ .action = TEST_ACTION_EMIT_SIGNAL, ++ .u.signal = { ++ .sender = TEST_CONN_ATTACKER, ++ .unicast_to = TEST_CONN_SUBSCRIBER, ++ .path = EXAMPLE_PATH, ++ .iface = EXAMPLE_INTERFACE, ++ .member = FOO_SIGNAL, ++ .received_by_conn = 0, ++ .received_by_proxy = 0 ++ }, ++ }, + { + /* When the service sends a signal with the name it already owns, + * it should get through */ +-- +2.50.0 + + +From 28039f209e9ee521f35c33db358483c80c142478 Mon Sep 17 00:00:00 2001 +From: Simon McVittie +Date: Tue, 23 Apr 2024 21:39:43 +0100 +Subject: [PATCH 17/17] tests: Ensure that unsubscribing with GetNameOwner + in-flight doesn't crash + +This was a bug that existed during development of this branch; make sure +it doesn't come back. + +This test fails with a use-after-free and crash if we comment out the +part of name_watcher_unref_watched_name() that removes the name watcher +from `map_method_serial_to_name_watcher`. + +It would also fail with an assertion failure if we asserted in +name_watcher_unref_watched_name() that get_name_owner_serial == 0 +(i.e. that GetNameOwner is not in-flight at destruction). + +Signed-off-by: Simon McVittie +--- + gio/tests/gdbus-subscribe.c | 52 ++++++++++++++++++++++++++++++++++++- + 1 file changed, 51 insertions(+), 1 deletion(-) + +diff --git a/gio/tests/gdbus-subscribe.c b/gio/tests/gdbus-subscribe.c +index 5406ba7e2..4cba4f565 100644 +--- a/gio/tests/gdbus-subscribe.c ++++ b/gio/tests/gdbus-subscribe.c +@@ -116,6 +116,7 @@ typedef struct + const char *member; + const char *arg0; + GDBusSignalFlags flags; ++ gboolean unsubscribe_immediately; + } TestSubscribe; + + typedef struct +@@ -141,6 +142,7 @@ typedef struct + TestEmitSignal signal; + TestSubscribe subscribe; + TestOwnName own_name; ++ guint unsubscribe_undo_step; + } u; + } TestStep; + +@@ -505,6 +507,43 @@ static const TestPlan plan_limit_by_well_known_name = + }, + }; + ++static const TestPlan plan_unsubscribe_immediately = ++{ ++ .description = "Unsubscribing before GetNameOwner can return doesn't result in a crash", ++ .steps = { ++ { ++ /* Service already owns one name */ ++ .action = TEST_ACTION_OWN_NAME, ++ .u.own_name = { ++ .name = ALREADY_OWNED_NAME, ++ .owner = TEST_CONN_SERVICE ++ }, ++ }, ++ { ++ .action = TEST_ACTION_SUBSCRIBE, ++ .u.subscribe = { ++ .string_sender = ALREADY_OWNED_NAME, ++ .path = EXAMPLE_PATH, ++ .iface = EXAMPLE_INTERFACE, ++ .unsubscribe_immediately = TRUE ++ }, ++ }, ++ { ++ .action = TEST_ACTION_EMIT_SIGNAL, ++ .u.signal = { ++ .sender = TEST_CONN_SERVICE, ++ .path = EXAMPLE_PATH, ++ .iface = EXAMPLE_INTERFACE, ++ .member = FOO_SIGNAL, ++ .received_by_conn = 0, ++ /* The proxy can't unsubscribe, except by destroying the proxy ++ * completely, which we don't currently implement in this test */ ++ .received_by_proxy = 1 ++ }, ++ }, ++ }, ++}; ++ + static const TestPlan plan_limit_to_message_bus = + { + .description = "A subscription to the message bus only accepts messages " +@@ -855,8 +894,18 @@ fixture_subscribe (Fixture *f, + subscribe->flags, + subscribed_signal_cb, + f, NULL); ++ + g_assert_cmpuint (id, !=, 0); +- f->subscriptions[step_number] = id; ++ ++ if (subscribe->unsubscribe_immediately) ++ { ++ g_test_message ("\tImmediately unsubscribing"); ++ g_dbus_connection_signal_unsubscribe (subscriber, id); ++ } ++ else ++ { ++ f->subscriptions[step_number] = id; ++ } + } + + if (f->mode != SUBSCRIPTION_MODE_CONN) +@@ -1287,6 +1336,7 @@ main (int argc, + ADD_SUBSCRIBE_TEST (nonexistent_unique_name); + ADD_SUBSCRIBE_TEST (limit_by_well_known_name); + ADD_SUBSCRIBE_TEST (limit_to_message_bus); ++ ADD_SUBSCRIBE_TEST (unsubscribe_immediately); + + return g_test_run(); + } +-- +2.50.0 + diff --git a/SOURCES/CVE-2024-52533.patch b/SOURCES/CVE-2024-52533.patch new file mode 100644 index 0000000..c677cfc --- /dev/null +++ b/SOURCES/CVE-2024-52533.patch @@ -0,0 +1,45 @@ +From 25833cefda24c60af913d6f2d532b5afd608b821 Mon Sep 17 00:00:00 2001 +From: Michael Catanzaro +Date: Thu, 19 Sep 2024 18:35:53 +0100 +Subject: [PATCH] gsocks4aproxy: Fix a single byte buffer overflow in connect + messages + +`SOCKS4_CONN_MSG_LEN` failed to account for the length of the final nul +byte in the connect message, which is an addition in SOCKSv4a vs +SOCKSv4. + +This means that the buffer for building and transmitting the connect +message could be overflowed if the username and hostname are both +`SOCKS4_MAX_LEN` (255) bytes long. + +Proxy configurations are normally statically configured, so the username +is very unlikely to be near its maximum length, and hence this overflow +is unlikely to be triggered in practice. + +(Commit message by Philip Withnall, diagnosis and fix by Michael +Catanzaro.) + +Fixes: #3461 +--- + gio/gsocks4aproxy.c | 4 ++-- + 1 file changed, 2 insertions(+), 2 deletions(-) + +diff --git a/gio/gsocks4aproxy.c b/gio/gsocks4aproxy.c +index 3dad118eb7..b3146d08fd 100644 +--- a/gio/gsocks4aproxy.c ++++ b/gio/gsocks4aproxy.c +@@ -79,9 +79,9 @@ g_socks4a_proxy_init (GSocks4aProxy *proxy) + * +----+----+----+----+----+----+----+----+----+----+....+----+------+....+------+ + * | VN | CD | DSTPORT | DSTIP | USERID |NULL| HOST | | NULL | + * +----+----+----+----+----+----+----+----+----+----+....+----+------+....+------+ +- * 1 1 2 4 variable 1 variable ++ * 1 1 2 4 variable 1 variable 1 + */ +-#define SOCKS4_CONN_MSG_LEN (9 + SOCKS4_MAX_LEN * 2) ++#define SOCKS4_CONN_MSG_LEN (10 + SOCKS4_MAX_LEN * 2) + static gint + set_connect_msg (guint8 *msg, + const gchar *hostname, +-- +GitLab + diff --git a/SOURCES/CVE-2025-4373.patch b/SOURCES/CVE-2025-4373.patch new file mode 100644 index 0000000..eb8acea --- /dev/null +++ b/SOURCES/CVE-2025-4373.patch @@ -0,0 +1,433 @@ +From 6c2178a3bc216a6cc765fc6ba3b0e6d22ce5af7e Mon Sep 17 00:00:00 2001 +From: Emmanuel Fleury +Date: Mon, 4 Feb 2019 13:31:28 +0100 +Subject: [PATCH 1/3] Fixing various warnings in glib/gstring.c +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +In file included from glib/glibconfig.h:9, + from glib/gtypes.h:32, + from glib/gstring.h:32, + from glib/gstring.c:37: +glib/gstring.c: In function ‘g_string_insert_len’: +glib/gstring.c:441:31: error: comparison of integer expressions of different signedness: ‘gssize’ {aka ‘long int’} and ‘gsize’ {aka ‘long unsigned int’} [-Werror=sign-compare] + g_return_val_if_fail (pos <= string->len, string); + ^~ +glib/gmacros.h:455:25: note: in definition of macro ‘G_LIKELY’ + #define G_LIKELY(expr) (expr) + ^~~~ +glib/gstring.c:441:5: note: in expansion of macro ‘g_return_val_if_fail’ + g_return_val_if_fail (pos <= string->len, string); + ^~~~~~~~~~~~~~~~~~~~ +glib/gstring.c:458:15: error: comparison of integer expressions of different signedness: ‘gssize’ {aka ‘long int’} and ‘gsize’ {aka ‘long unsigned int’} [-Werror=sign-compare] + if (pos < string->len) + ^ +glib/gstring.c:462:18: error: comparison of integer expressions of different signedness: ‘gsize’ {aka ‘long unsigned int’} and ‘gssize’ {aka ‘long int’} [-Werror=sign-compare] + if (offset < pos) + ^ +In file included from glib/glibconfig.h:9, + from glib/gtypes.h:32, + from glib/gstring.h:32, + from glib/gstring.c:37: +glib/gmacros.h:351:26: error: comparison of integer expressions of different signedness: ‘gssize’ {aka ‘long int’} and ‘long unsigned int’ [-Werror=sign-compare] + #define MIN(a, b) (((a) < (b)) ? (a) : (b)) + ^ +glib/gstring.c:464:22: note: in expansion of macro ‘MIN’ + precount = MIN (len, pos - offset); + ^~~ +glib/gmacros.h:351:35: error: operand of ?: changes signedness from ‘gssize’ {aka ‘long int’} to ‘long unsigned int’ due to unsignedness of other operand [-Werror=sign-compare] + #define MIN(a, b) (((a) < (b)) ? (a) : (b)) + ^~~ +glib/gstring.c:464:22: note: in expansion of macro ‘MIN’ + precount = MIN (len, pos - offset); + ^~~ +glib/gstring.c:469:15: error: comparison of integer expressions of different signedness: ‘gssize’ {aka ‘long int’} and ‘gsize’ {aka ‘long unsigned int’} [-Werror=sign-compare] + if (len > precount) + ^ +glib/gstring.c:481:15: error: comparison of integer expressions of different signedness: ‘gssize’ {aka ‘long int’} and ‘gsize’ {aka ‘long unsigned int’} [-Werror=sign-compare] + if (pos < string->len) + ^ +In file included from glib/glibconfig.h:9, + from glib/gtypes.h:32, + from glib/gstring.h:32, + from glib/gstring.c:37: +glib/gstring.c: In function ‘g_string_insert_c’: +glib/gstring.c:782:31: error: comparison of integer expressions of different signedness: ‘gssize’ {aka ‘long int’} and ‘gsize’ {aka ‘long unsigned int’} [-Werror=sign-compare] + g_return_val_if_fail (pos <= string->len, string); + ^~ +glib/gmacros.h:455:25: note: in definition of macro ‘G_LIKELY’ + #define G_LIKELY(expr) (expr) + ^~~~ +glib/gstring.c:782:5: note: in expansion of macro ‘g_return_val_if_fail’ + g_return_val_if_fail (pos <= string->len, string); + ^~~~~~~~~~~~~~~~~~~~ +glib/gstring.c:785:11: error: comparison of integer expressions of different signedness: ‘gssize’ {aka ‘long int’} and ‘gsize’ {aka ‘long unsigned int’} [-Werror=sign-compare] + if (pos < string->len) + ^ +In file included from glib/glibconfig.h:9, + from glib/gtypes.h:32, + from glib/gstring.h:32, + from glib/gstring.c:37: +glib/gstring.c: In function ‘g_string_insert_unichar’: +glib/gstring.c:857:31: error: comparison of integer expressions of different signedness: ‘gssize’ {aka ‘long int’} and ‘gsize’ {aka ‘long unsigned int’} [-Werror=sign-compare] + g_return_val_if_fail (pos <= string->len, string); + ^~ +glib/gmacros.h:455:25: note: in definition of macro ‘G_LIKELY’ + #define G_LIKELY(expr) (expr) + ^~~~ +glib/gstring.c:857:5: note: in expansion of macro ‘g_return_val_if_fail’ + g_return_val_if_fail (pos <= string->len, string); + ^~~~~~~~~~~~~~~~~~~~ +glib/gstring.c:860:11: error: comparison of integer expressions of different signedness: ‘gssize’ {aka ‘long int’} and ‘gsize’ {aka ‘long unsigned int’} [-Werror=sign-compare] + if (pos < string->len) + ^ +In file included from glib/glibconfig.h:9, + from glib/gtypes.h:32, + from glib/gstring.h:32, + from glib/gstring.c:37: +glib/gstring.c: In function ‘g_string_erase’: +glib/gstring.c:969:29: error: comparison of integer expressions of different signedness: ‘gssize’ {aka ‘long int’} and ‘gsize’ {aka ‘long unsigned int’} [-Werror=sign-compare] + g_return_val_if_fail (pos <= string->len, string); + ^~ +glib/gmacros.h:455:25: note: in definition of macro ‘G_LIKELY’ + #define G_LIKELY(expr) (expr) + ^~~~ +glib/gstring.c:969:3: note: in expansion of macro ‘g_return_val_if_fail’ + g_return_val_if_fail (pos <= string->len, string); + ^~~~~~~~~~~~~~~~~~~~ +glib/gstring.c:975:39: error: comparison of integer expressions of different signedness: ‘gssize’ {aka ‘long int’} and ‘gsize’ {aka ‘long unsigned int’} [-Werror=sign-compare] + g_return_val_if_fail (pos + len <= string->len, string); + ^~ +glib/gmacros.h:455:25: note: in definition of macro ‘G_LIKELY’ + #define G_LIKELY(expr) (expr) + ^~~~ +glib/gstring.c:975:7: note: in expansion of macro ‘g_return_val_if_fail’ + g_return_val_if_fail (pos + len <= string->len, string); + ^~~~~~~~~~~~~~~~~~~~ +glib/gstring.c:977:21: error: comparison of integer expressions of different signedness: ‘gssize’ {aka ‘long int’} and ‘gsize’ {aka ‘long unsigned int’} [-Werror=sign-compare] + if (pos + len < string->len) + ^ +--- + glib/gstring.c | 82 +++++++++++++++++++++++++++++++------------------- + 1 file changed, 51 insertions(+), 31 deletions(-) + +diff --git a/glib/gstring.c b/glib/gstring.c +index 966502019..f5bfeb0ed 100644 +--- a/glib/gstring.c ++++ b/glib/gstring.c +@@ -426,6 +426,8 @@ g_string_insert_len (GString *string, + const gchar *val, + gssize len) + { ++ gsize len_unsigned, pos_unsigned; ++ + g_return_val_if_fail (string != NULL, NULL); + g_return_val_if_fail (len == 0 || val != NULL, string); + +@@ -434,11 +436,15 @@ g_string_insert_len (GString *string, + + if (len < 0) + len = strlen (val); ++ len_unsigned = len; + + if (pos < 0) +- pos = string->len; ++ pos_unsigned = string->len; + else +- g_return_val_if_fail (pos <= string->len, string); ++ { ++ pos_unsigned = pos; ++ g_return_val_if_fail (pos_unsigned <= string->len, string); ++ } + + /* Check whether val represents a substring of string. + * This test probably violates chapter and verse of the C standards, +@@ -450,45 +456,48 @@ g_string_insert_len (GString *string, + gsize offset = val - string->str; + gsize precount = 0; + +- g_string_maybe_expand (string, len); ++ g_string_maybe_expand (string, len_unsigned); + val = string->str + offset; + /* At this point, val is valid again. */ + + /* Open up space where we are going to insert. */ +- if (pos < string->len) +- memmove (string->str + pos + len, string->str + pos, string->len - pos); ++ if (pos_unsigned < string->len) ++ memmove (string->str + pos_unsigned + len_unsigned, ++ string->str + pos_unsigned, string->len - pos_unsigned); + + /* Move the source part before the gap, if any. */ +- if (offset < pos) ++ if (offset < pos_unsigned) + { +- precount = MIN (len, pos - offset); +- memcpy (string->str + pos, val, precount); ++ precount = MIN (len_unsigned, pos_unsigned - offset); ++ memcpy (string->str + pos_unsigned, val, precount); + } + + /* Move the source part after the gap, if any. */ +- if (len > precount) +- memcpy (string->str + pos + precount, +- val + /* Already moved: */ precount + /* Space opened up: */ len, +- len - precount); ++ if (len_unsigned > precount) ++ memcpy (string->str + pos_unsigned + precount, ++ val + /* Already moved: */ precount + ++ /* Space opened up: */ len_unsigned, ++ len_unsigned - precount); + } + else + { +- g_string_maybe_expand (string, len); ++ g_string_maybe_expand (string, len_unsigned); + + /* If we aren't appending at the end, move a hunk + * of the old string to the end, opening up space + */ +- if (pos < string->len) +- memmove (string->str + pos + len, string->str + pos, string->len - pos); ++ if (pos_unsigned < string->len) ++ memmove (string->str + pos_unsigned + len_unsigned, ++ string->str + pos_unsigned, string->len - pos_unsigned); + + /* insert the new string */ +- if (len == 1) +- string->str[pos] = *val; ++ if (len_unsigned == 1) ++ string->str[pos_unsigned] = *val; + else +- memcpy (string->str + pos, val, len); ++ memcpy (string->str + pos_unsigned, val, len_unsigned); + } + +- string->len += len; ++ string->len += len_unsigned; + + string->str[string->len] = 0; + +@@ -772,6 +781,8 @@ g_string_insert_c (GString *string, + gssize pos, + gchar c) + { ++ gsize pos_unsigned; ++ + g_return_val_if_fail (string != NULL, NULL); + + g_string_maybe_expand (string, 1); +@@ -779,13 +790,15 @@ g_string_insert_c (GString *string, + if (pos < 0) + pos = string->len; + else +- g_return_val_if_fail (pos <= string->len, string); ++ g_return_val_if_fail ((gsize) pos <= string->len, string); ++ pos_unsigned = pos; + + /* If not just an append, move the old stuff */ +- if (pos < string->len) +- memmove (string->str + pos + 1, string->str + pos, string->len - pos); ++ if (pos_unsigned < string->len) ++ memmove (string->str + pos_unsigned + 1, ++ string->str + pos_unsigned, string->len - pos_unsigned); + +- string->str[pos] = c; ++ string->str[pos_unsigned] = c; + + string->len += 1; + +@@ -854,10 +867,10 @@ g_string_insert_unichar (GString *string, + if (pos < 0) + pos = string->len; + else +- g_return_val_if_fail (pos <= string->len, string); ++ g_return_val_if_fail ((gsize) pos <= string->len, string); + + /* If not just an append, move the old stuff */ +- if (pos < string->len) ++ if ((gsize) pos < string->len) + memmove (string->str + pos + charlen, string->str + pos, string->len - pos); + + dest = string->str + pos; +@@ -964,21 +977,28 @@ g_string_erase (GString *string, + gssize pos, + gssize len) + { ++ gsize len_unsigned, pos_unsigned; ++ + g_return_val_if_fail (string != NULL, NULL); + g_return_val_if_fail (pos >= 0, string); +- g_return_val_if_fail (pos <= string->len, string); ++ pos_unsigned = pos; ++ ++ g_return_val_if_fail (pos_unsigned <= string->len, string); + + if (len < 0) +- len = string->len - pos; ++ len_unsigned = string->len - pos_unsigned; + else + { +- g_return_val_if_fail (pos + len <= string->len, string); ++ len_unsigned = len; ++ g_return_val_if_fail (pos_unsigned + len_unsigned <= string->len, string); + +- if (pos + len < string->len) +- memmove (string->str + pos, string->str + pos + len, string->len - (pos + len)); ++ if (pos_unsigned + len_unsigned < string->len) ++ memmove (string->str + pos_unsigned, ++ string->str + pos_unsigned + len_unsigned, ++ string->len - (pos_unsigned + len_unsigned)); + } + +- string->len -= len; ++ string->len -= len_unsigned; + + string->str[string->len] = 0; + +-- +2.50.0 + + +From 101afb01778659cb7051b3cb33d55dc965c8dc7e Mon Sep 17 00:00:00 2001 +From: Michael Catanzaro +Date: Thu, 10 Apr 2025 10:57:20 -0500 +Subject: [PATCH 2/3] gstring: carefully handle gssize parameters + +Wherever we use gssize to allow passing -1, we need to ensure we don't +overflow the value by assigning a gsize to it without checking if the +size exceeds the maximum gssize. The safest way to do this is to just +use normal gsize everywhere instead and use gssize only for the +parameter. + +Our computers don't have enough RAM to write tests for this. I tried +forcing string->len to high values for test purposes, but this isn't +valid and will just cause out of bounds reads/writes due to +string->allocated_len being unexpectedly small, so I don't think we can +test this easily. +--- + glib/gstring.c | 36 +++++++++++++++++++++++------------- + 1 file changed, 23 insertions(+), 13 deletions(-) + +diff --git a/glib/gstring.c b/glib/gstring.c +index f5bfeb0ed..84a98da25 100644 +--- a/glib/gstring.c ++++ b/glib/gstring.c +@@ -435,8 +435,9 @@ g_string_insert_len (GString *string, + return string; + + if (len < 0) +- len = strlen (val); +- len_unsigned = len; ++ len_unsigned = strlen (val); ++ else ++ len_unsigned = len; + + if (pos < 0) + pos_unsigned = string->len; +@@ -788,10 +789,12 @@ g_string_insert_c (GString *string, + g_string_maybe_expand (string, 1); + + if (pos < 0) +- pos = string->len; ++ pos_unsigned = string->len; + else +- g_return_val_if_fail ((gsize) pos <= string->len, string); +- pos_unsigned = pos; ++ { ++ pos_unsigned = pos; ++ g_return_val_if_fail (pos_unsigned <= string->len, string); ++ } + + /* If not just an append, move the old stuff */ + if (pos_unsigned < string->len) +@@ -824,6 +827,7 @@ g_string_insert_unichar (GString *string, + gssize pos, + gunichar wc) + { ++ gsize pos_unsigned; + gint charlen, first, i; + gchar *dest; + +@@ -865,15 +869,18 @@ g_string_insert_unichar (GString *string, + g_string_maybe_expand (string, charlen); + + if (pos < 0) +- pos = string->len; ++ pos_unsigned = string->len; + else +- g_return_val_if_fail ((gsize) pos <= string->len, string); ++ { ++ pos_unsigned = pos; ++ g_return_val_if_fail (pos_unsigned <= string->len, string); ++ } + + /* If not just an append, move the old stuff */ +- if ((gsize) pos < string->len) +- memmove (string->str + pos + charlen, string->str + pos, string->len - pos); ++ if (pos_unsigned < string->len) ++ memmove (string->str + pos_unsigned + charlen, string->str + pos_unsigned, string->len - pos_unsigned); + +- dest = string->str + pos; ++ dest = string->str + pos_unsigned; + /* Code copied from g_unichar_to_utf() */ + for (i = charlen - 1; i > 0; --i) + { +@@ -931,6 +938,7 @@ g_string_overwrite_len (GString *string, + const gchar *val, + gssize len) + { ++ gssize len_unsigned; + gsize end; + + g_return_val_if_fail (string != NULL, NULL); +@@ -942,14 +950,16 @@ g_string_overwrite_len (GString *string, + g_return_val_if_fail (pos <= string->len, string); + + if (len < 0) +- len = strlen (val); ++ len_unsigned = strlen (val); ++ else ++ len_unsigned = len; + +- end = pos + len; ++ end = pos + len_unsigned; + + if (end > string->len) + g_string_maybe_expand (string, end - string->len); + +- memcpy (string->str + pos, val, len); ++ memcpy (string->str + pos, val, len_unsigned); + + if (end > string->len) + { +-- +2.50.0 + + +From 789c240db9738cae37ee77f7e135e5dbc39ab3ca Mon Sep 17 00:00:00 2001 +From: Peter Bloomfield +Date: Fri, 11 Apr 2025 05:52:33 +0000 +Subject: [PATCH 3/3] gstring: Make len_unsigned unsigned + +--- + glib/gstring.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/glib/gstring.c b/glib/gstring.c +index 84a98da25..7379517f5 100644 +--- a/glib/gstring.c ++++ b/glib/gstring.c +@@ -938,7 +938,7 @@ g_string_overwrite_len (GString *string, + const gchar *val, + gssize len) + { +- gssize len_unsigned; ++ gsize len_unsigned; + gsize end; + + g_return_val_if_fail (string != NULL, NULL); +-- +2.50.0 + diff --git a/SOURCES/gdatetime-test.patch b/SOURCES/gdatetime-test.patch new file mode 100644 index 0000000..a3942a7 --- /dev/null +++ b/SOURCES/gdatetime-test.patch @@ -0,0 +1,126 @@ +From a07f759373d888c837a780cb93f14542b029e19c Mon Sep 17 00:00:00 2001 +From: "Rebecca N. Palmer" +Date: Fri, 11 Oct 2024 09:38:52 +0100 +Subject: [PATCH 1/2] gdatetime test: Do not assume PST8PDT was always exactly + -8/-7 + +In newer tzdata, it is an alias for America/Los_Angeles, which has a +slightly different meaning: DST did not exist there before 1883. As a +result, we can no longer hard-code the knowledge that interval 0 is +standard time and interval 1 is summer time, and instead we need to look +up the correct intervals from known timestamps. + +Resolves: https://gitlab.gnome.org/GNOME/glib/-/issues/3502 +Bug-Debian: https://bugs.debian.org/1084190 +[smcv: expand commit message, fix whitespace] +Signed-off-by: Simon McVittie +--- + glib/tests/gdatetime.c | 23 +++++++++++++++++------ + 1 file changed, 17 insertions(+), 6 deletions(-) + +diff --git a/glib/tests/gdatetime.c b/glib/tests/gdatetime.c +index 5a2190d5f..80098bab3 100644 +--- a/glib/tests/gdatetime.c ++++ b/glib/tests/gdatetime.c +@@ -2096,6 +2096,7 @@ test_posix_parse (void) + { + GTimeZone *tz; + GDateTime *gdt1, *gdt2; ++ gint i1, i2; + + tz = g_time_zone_new ("PST"); + g_assert_cmpstr (g_time_zone_get_abbreviation (tz, 0), ==, "UTC"); +@@ -2111,14 +2112,24 @@ test_posix_parse (void) + + /* This fails rules_from_identifier on Unix (though not on Windows) + * but passes anyway because PST8PDT is a zone name. ++ * ++ * Intervals i1 and i2 (rather than 0 and 1) are needed because in ++ * recent tzdata, PST8PDT may be an alias for America/Los_Angeles, ++ * and hence be aware that DST has not always existed. ++ * https://bugs.debian.org/1084190 + */ + tz = g_time_zone_new ("PST8PDT"); +- g_assert_cmpstr (g_time_zone_get_abbreviation (tz, 0), ==, "PST"); +- g_assert_cmpint (g_time_zone_get_offset (tz, 0), ==, - 8 * 3600); +- g_assert (!g_time_zone_is_dst (tz, 0)); +- g_assert_cmpstr (g_time_zone_get_abbreviation (tz, 1), ==, "PDT"); +- g_assert_cmpint (g_time_zone_get_offset (tz, 1), ==,- 7 * 3600); +- g_assert (g_time_zone_is_dst (tz, 1)); ++ g_assert_nonnull (tz); ++ /* a date in winter = non-DST */ ++ i1 = g_time_zone_find_interval (tz, G_TIME_TYPE_STANDARD, 0); ++ /* approximately 6 months in seconds, i.e. a date in summer = DST */ ++ i2 = g_time_zone_find_interval (tz, G_TIME_TYPE_DAYLIGHT, 15000000); ++ g_assert_cmpstr (g_time_zone_get_abbreviation (tz, i1), ==, "PST"); ++ g_assert_cmpint (g_time_zone_get_offset (tz, i1), ==, - 8 * 3600); ++ g_assert (!g_time_zone_is_dst (tz, i1)); ++ g_assert_cmpstr (g_time_zone_get_abbreviation (tz, i2), ==, "PDT"); ++ g_assert_cmpint (g_time_zone_get_offset (tz, i2), ==,- 7 * 3600); ++ g_assert (g_time_zone_is_dst (tz, i2)); + g_time_zone_unref (tz); + + tz = g_time_zone_new ("PST8PDT6:32:15"); +-- +2.50.0 + + +From d1e564d6d4f478c9e0b163763ed2b199dfafd500 Mon Sep 17 00:00:00 2001 +From: Simon McVittie +Date: Fri, 18 Oct 2024 11:03:19 +0100 +Subject: [PATCH 2/2] gdatetime test: Try to make PST8PDT test more obviously + correct + +Instead of using timestamp 0 as a magic number (in this case interpreted +as 1970-01-01T00:00:00-08:00), calculate a timestamp from a recent +year/month/day in winter, in this case 2024-01-01T00:00:00-08:00. + +Similarly, instead of using a timestamp 15 million seconds later +(1970-06-23T15:40:00-07:00), calculate a timestamp from a recent +year/month/day in summer, in this case 2024-07-01T00:00:00-07:00. + +Signed-off-by: Simon McVittie +--- + glib/tests/gdatetime.c | 15 +++++++-------- + 1 file changed, 7 insertions(+), 8 deletions(-) + +diff --git a/glib/tests/gdatetime.c b/glib/tests/gdatetime.c +index 80098bab3..d0755e722 100644 +--- a/glib/tests/gdatetime.c ++++ b/glib/tests/gdatetime.c +@@ -2112,18 +2112,15 @@ test_posix_parse (void) + + /* This fails rules_from_identifier on Unix (though not on Windows) + * but passes anyway because PST8PDT is a zone name. +- * +- * Intervals i1 and i2 (rather than 0 and 1) are needed because in +- * recent tzdata, PST8PDT may be an alias for America/Los_Angeles, +- * and hence be aware that DST has not always existed. +- * https://bugs.debian.org/1084190 + */ + tz = g_time_zone_new ("PST8PDT"); + g_assert_nonnull (tz); + /* a date in winter = non-DST */ +- i1 = g_time_zone_find_interval (tz, G_TIME_TYPE_STANDARD, 0); +- /* approximately 6 months in seconds, i.e. a date in summer = DST */ +- i2 = g_time_zone_find_interval (tz, G_TIME_TYPE_DAYLIGHT, 15000000); ++ gdt1 = g_date_time_new (tz, 2024, 1, 1, 0, 0, 0); ++ i1 = g_time_zone_find_interval (tz, G_TIME_TYPE_STANDARD, g_date_time_to_unix (gdt1)); ++ /* a date in summer = DST */ ++ gdt2 = g_date_time_new (tz, 2024, 7, 1, 0, 0, 0); ++ i2 = g_time_zone_find_interval (tz, G_TIME_TYPE_DAYLIGHT, g_date_time_to_unix (gdt2)); + g_assert_cmpstr (g_time_zone_get_abbreviation (tz, i1), ==, "PST"); + g_assert_cmpint (g_time_zone_get_offset (tz, i1), ==, - 8 * 3600); + g_assert (!g_time_zone_is_dst (tz, i1)); +@@ -2131,6 +2128,8 @@ test_posix_parse (void) + g_assert_cmpint (g_time_zone_get_offset (tz, i2), ==,- 7 * 3600); + g_assert (g_time_zone_is_dst (tz, i2)); + g_time_zone_unref (tz); ++ g_date_time_unref (gdt1); ++ g_date_time_unref (gdt2); + + tz = g_time_zone_new ("PST8PDT6:32:15"); + #ifdef G_OS_WIN32 +-- +2.50.0 + diff --git a/SOURCES/gdbus-conflict-reduction.patch b/SOURCES/gdbus-conflict-reduction.patch new file mode 100644 index 0000000..9329b65 --- /dev/null +++ b/SOURCES/gdbus-conflict-reduction.patch @@ -0,0 +1,401 @@ +From b43d2e06834a9779801f9a68904ae73a26578f01 Mon Sep 17 00:00:00 2001 +From: Philip Withnall +Date: Sat, 21 Sep 2019 10:45:36 +0200 +Subject: [PATCH 1/2] gatomic: Add various casts to use of g_atomic_*()s to fix + warnings +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +When compiling GLib with `-Wsign-conversion`, we get various warnings +about the atomic calls. A lot of these were fixed by +3ad375a629c91a27d0165a31f0ed298fd553de0a, but some remain. Fix them by +adding appropriate casts at the call sites. + +Note that `g_atomic_int_{and,or,xor}()` actually all operate on `guint`s +rather than `gint`s (which is what the rest of the `g_atomic_int_*()` +functions operate on). I can’t find any written reasoning for this, but +assume that it’s because signedness is irrelevant when you’re using an +integer as a bit field. It’s unfortunate that they’re named a +`g_atomic_int_*()` rather than `g_atomic_uint_*()` functions. + +Tested by compiling GLib as: +``` +CFLAGS=-Wsign-conversion jhbuild make -ac |& grep atomic +``` + +I’m not going to add `-Wsign-conversion` to the set of default warnings +for building GLib, because it mostly produces false positives throughout +the rest of GLib. + +Signed-off-by: Philip Withnall + +Fixes: #1565 +--- + gio/gdbusconnection.c | 8 ++++---- + gio/gdbusnamewatching.c | 4 ++-- + gio/tests/gdbus-threading.c | 2 +- + glib/gbitlock.c | 2 +- + glib/gquark.c | 2 +- + glib/gthread-posix.c | 2 +- + glib/gthreadpool.c | 10 +++++----- + glib/tests/atomic.c | 20 ++++++++++---------- + 8 files changed, 25 insertions(+), 25 deletions(-) + +diff --git a/gio/gdbusconnection.c b/gio/gdbusconnection.c +index 117c8df35..1c1f0cf3d 100644 +--- a/gio/gdbusconnection.c ++++ b/gio/gdbusconnection.c +@@ -3127,7 +3127,7 @@ g_dbus_connection_add_filter (GDBusConnection *connection, + + CONNECTION_LOCK (connection); + data = g_new0 (FilterData, 1); +- data->id = g_atomic_int_add (&_global_filter_id, 1); /* TODO: overflow etc. */ ++ data->id = (guint) g_atomic_int_add (&_global_filter_id, 1); /* TODO: overflow etc. */ + data->ref_count = 1; + data->filter_function = filter_function; + data->user_data = user_data; +@@ -3482,7 +3482,7 @@ g_dbus_connection_signal_subscribe (GDBusConnection *connection, + subscriber.callback = callback; + subscriber.user_data = user_data; + subscriber.user_data_free_func = user_data_free_func; +- subscriber.id = g_atomic_int_add (&_global_subscriber_id, 1); /* TODO: overflow etc. */ ++ subscriber.id = (guint) g_atomic_int_add (&_global_subscriber_id, 1); /* TODO: overflow etc. */ + subscriber.context = g_main_context_ref_thread_default (); + + /* see if we've already have this rule */ +@@ -5172,7 +5172,7 @@ g_dbus_connection_register_object (GDBusConnection *connection, + } + + ei = g_new0 (ExportedInterface, 1); +- ei->id = g_atomic_int_add (&_global_registration_id, 1); /* TODO: overflow etc. */ ++ ei->id = (guint) g_atomic_int_add (&_global_registration_id, 1); /* TODO: overflow etc. */ + ei->eo = eo; + ei->user_data = user_data; + ei->user_data_free_func = user_data_free_func; +@@ -6832,7 +6832,7 @@ g_dbus_connection_register_subtree (GDBusConnection *connection, + + es->vtable = _g_dbus_subtree_vtable_copy (vtable); + es->flags = flags; +- es->id = g_atomic_int_add (&_global_subtree_registration_id, 1); /* TODO: overflow etc. */ ++ es->id = (guint) g_atomic_int_add (&_global_subtree_registration_id, 1); /* TODO: overflow etc. */ + es->user_data = user_data; + es->user_data_free_func = user_data_free_func; + es->context = g_main_context_ref_thread_default (); +diff --git a/gio/gdbusnamewatching.c b/gio/gdbusnamewatching.c +index dad8e75ec..01ee6e762 100644 +--- a/gio/gdbusnamewatching.c ++++ b/gio/gdbusnamewatching.c +@@ -603,7 +603,7 @@ g_bus_watch_name (GBusType bus_type, + + client = g_new0 (Client, 1); + client->ref_count = 1; +- client->id = g_atomic_int_add (&next_global_id, 1); /* TODO: uh oh, handle overflow */ ++ client->id = (guint) g_atomic_int_add (&next_global_id, 1); /* TODO: uh oh, handle overflow */ + client->name = g_strdup (name); + client->flags = flags; + client->name_appeared_handler = name_appeared_handler; +@@ -665,7 +665,7 @@ guint g_bus_watch_name_on_connection (GDBusConnection *connection, + + client = g_new0 (Client, 1); + client->ref_count = 1; +- client->id = g_atomic_int_add (&next_global_id, 1); /* TODO: uh oh, handle overflow */ ++ client->id = (guint) g_atomic_int_add (&next_global_id, 1); /* TODO: uh oh, handle overflow */ + client->name = g_strdup (name); + client->flags = flags; + client->name_appeared_handler = name_appeared_handler; +diff --git a/gio/tests/gdbus-threading.c b/gio/tests/gdbus-threading.c +index 3e4dc92e5..13ff15bab 100644 +--- a/gio/tests/gdbus-threading.c ++++ b/gio/tests/gdbus-threading.c +@@ -514,7 +514,7 @@ test_threaded_singleton (void) + /* We want to be the last ref, so let it finish setting up */ + for (j = 0; j < 100; j++) + { +- guint r = g_atomic_int_get (&G_OBJECT (c)->ref_count); ++ guint r = (guint) g_atomic_int_get (&G_OBJECT (c)->ref_count); + + if (r == 1) + break; +diff --git a/glib/gbitlock.c b/glib/gbitlock.c +index 46e5f7d06..23024d08c 100644 +--- a/glib/gbitlock.c ++++ b/glib/gbitlock.c +@@ -224,7 +224,7 @@ g_bit_lock (volatile gint *address, + guint mask = 1u << lock_bit; + guint v; + +- v = g_atomic_int_get (address); ++ v = (guint) g_atomic_int_get (address); + if (v & mask) + { + guint class = ((gsize) address) % G_N_ELEMENTS (g_bit_lock_contended); +diff --git a/glib/gquark.c b/glib/gquark.c +index c4d12b870..df12ff69a 100644 +--- a/glib/gquark.c ++++ b/glib/gquark.c +@@ -262,7 +262,7 @@ g_quark_to_string (GQuark quark) + gchar **strings; + gint seq_id; + +- seq_id = g_atomic_int_get (&quark_seq_id); ++ seq_id = (guint) g_atomic_int_get (&quark_seq_id); + strings = g_atomic_pointer_get (&quarks); + + if (quark < seq_id) +diff --git a/glib/gthread-posix.c b/glib/gthread-posix.c +index 5fff51477..aca9208cc 100644 +--- a/glib/gthread-posix.c ++++ b/glib/gthread-posix.c +@@ -1396,7 +1396,7 @@ void + g_cond_wait (GCond *cond, + GMutex *mutex) + { +- guint sampled = g_atomic_int_get (&cond->i[0]); ++ guint sampled = (guint) g_atomic_int_get (&cond->i[0]); + + g_mutex_unlock (mutex); + syscall (__NR_futex, &cond->i[0], (gsize) FUTEX_WAIT_PRIVATE, (gsize) sampled, NULL); +diff --git a/glib/gthreadpool.c b/glib/gthreadpool.c +index dd7289370..7d75245b0 100644 +--- a/glib/gthreadpool.c ++++ b/glib/gthreadpool.c +@@ -145,7 +145,7 @@ g_thread_pool_wait_for_new_pool (void) + gint last_wakeup_thread_serial; + gboolean have_relayed_thread_marker = FALSE; + +- local_max_unused_threads = g_atomic_int_get (&max_unused_threads); ++ local_max_unused_threads = (guint) g_atomic_int_get (&max_unused_threads); + local_max_idle_time = g_atomic_int_get (&max_idle_time); + last_wakeup_thread_serial = g_atomic_int_get (&wakeup_thread_serial); + +@@ -209,7 +209,7 @@ g_thread_pool_wait_for_new_pool (void) + DEBUG_MSG (("thread %p updating to new limits.", + g_thread_self ())); + +- local_max_unused_threads = g_atomic_int_get (&max_unused_threads); ++ local_max_unused_threads = (guint) g_atomic_int_get (&max_unused_threads); + local_max_idle_time = g_atomic_int_get (&max_idle_time); + last_wakeup_thread_serial = local_wakeup_thread_serial; + +@@ -893,7 +893,7 @@ g_thread_pool_get_max_unused_threads (void) + guint + g_thread_pool_get_num_unused_threads (void) + { +- return g_atomic_int_get (&unused_threads); ++ return (guint) g_atomic_int_get (&unused_threads); + } + + /** +@@ -1016,7 +1016,7 @@ g_thread_pool_set_max_idle_time (guint interval) + + g_atomic_int_set (&max_idle_time, interval); + +- i = g_atomic_int_get (&unused_threads); ++ i = (guint) g_atomic_int_get (&unused_threads); + if (i > 0) + { + g_atomic_int_inc (&wakeup_thread_serial); +@@ -1052,5 +1052,5 @@ g_thread_pool_set_max_idle_time (guint interval) + guint + g_thread_pool_get_max_idle_time (void) + { +- return g_atomic_int_get (&max_idle_time); ++ return (guint) g_atomic_int_get (&max_idle_time); + } +diff --git a/glib/tests/atomic.c b/glib/tests/atomic.c +index 35fa705a4..856df12d2 100644 +--- a/glib/tests/atomic.c ++++ b/glib/tests/atomic.c +@@ -27,7 +27,7 @@ test_types (void) + cspp = &csp; + + g_atomic_int_set (&u, 5); +- u2 = g_atomic_int_get (&u); ++ u2 = (guint) g_atomic_int_get (&u); + g_assert_cmpint (u2, ==, 5); + res = g_atomic_int_compare_and_exchange (&u, 6, 7); + g_assert (!res); +@@ -62,13 +62,13 @@ test_types (void) + res = g_atomic_int_dec_and_test (&s); + g_assert (!res); + g_assert_cmpint (s, ==, 6); +- s2 = g_atomic_int_and (&s, 5); ++ s2 = (gint) g_atomic_int_and (&s, 5); + g_assert_cmpint (s2, ==, 6); + g_assert_cmpint (s, ==, 4); +- s2 = g_atomic_int_or (&s, 8); ++ s2 = (gint) g_atomic_int_or (&s, 8); + g_assert_cmpint (s2, ==, 4); + g_assert_cmpint (s, ==, 12); +- s2 = g_atomic_int_xor (&s, 4); ++ s2 = (gint) g_atomic_int_xor (&s, 4); + g_assert_cmpint (s2, ==, 12); + g_assert_cmpint (s, ==, 8); + +@@ -92,7 +92,7 @@ test_types (void) + res = g_atomic_pointer_compare_and_exchange (&gs, 0, 0); + g_assert (res); + g_assert (gs == 0); +- gs2 = g_atomic_pointer_add (&gs, 5); ++ gs2 = (gsize) g_atomic_pointer_add (&gs, 5); + g_assert (gs2 == 0); + g_assert (gs == 5); + gs2 = g_atomic_pointer_and (&gs, 6); +@@ -127,7 +127,7 @@ test_types (void) + #undef g_atomic_pointer_xor + + g_atomic_int_set ((gint*)&u, 5); +- u2 = g_atomic_int_get ((gint*)&u); ++ u2 = (guint) g_atomic_int_get ((gint*)&u); + g_assert_cmpint (u2, ==, 5); + res = g_atomic_int_compare_and_exchange ((gint*)&u, 6, 7); + g_assert (!res); +@@ -161,13 +161,13 @@ test_types (void) + res = g_atomic_int_dec_and_test (&s); + g_assert (!res); + g_assert_cmpint (s, ==, 6); +- s2 = g_atomic_int_and ((guint*)&s, 5); ++ s2 = (gint) g_atomic_int_and ((guint*)&s, 5); + g_assert_cmpint (s2, ==, 6); + g_assert_cmpint (s, ==, 4); +- s2 = g_atomic_int_or ((guint*)&s, 8); ++ s2 = (gint) g_atomic_int_or ((guint*)&s, 8); + g_assert_cmpint (s2, ==, 4); + g_assert_cmpint (s, ==, 12); +- s2 = g_atomic_int_xor ((guint*)&s, 4); ++ s2 = (gint) g_atomic_int_xor ((guint*)&s, 4); + g_assert_cmpint (s2, ==, 12); + g_assert_cmpint (s, ==, 8); + G_GNUC_BEGIN_IGNORE_DEPRECATIONS +@@ -196,7 +196,7 @@ G_GNUC_END_IGNORE_DEPRECATIONS + res = g_atomic_pointer_compare_and_exchange (&gs, 0, 0); + g_assert (res); + g_assert (gs == 0); +- gs2 = g_atomic_pointer_add (&gs, 5); ++ gs2 = (gsize) g_atomic_pointer_add (&gs, 5); + g_assert (gs2 == 0); + g_assert (gs == 5); + gs2 = g_atomic_pointer_and (&gs, 6); +-- +2.50.0 + + +From 5d6ced6a7fc6dbbc891992a7d16cd465f2ff1290 Mon Sep 17 00:00:00 2001 +From: Philip Withnall +Date: Wed, 22 Feb 2023 12:19:16 +0000 +Subject: [PATCH 2/2] gdbusconnection: Rearrange refcount handling of + map_method_serial_to_task +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +It already implicitly held a strong ref on its `GTask` values, but +didn’t have a free function set so that they would be automatically +unreffed on removal from the map. + +This meant that the functions handling removals from the map, +`on_worker_closed()` (via `cancel_method_on_close()`) and +`send_message_with_reply_cleanup()` had to call unref once more than +they would otherwise. + +In `send_message_with_reply_cleanup()`, this behaviour depended on +whether it was called with `remove == TRUE`. If not, it was `(transfer +none)` not `(transfer full)`. This led to bugs in its callers. + +For example, this led to a direct leak in `cancel_method_on_close()`, as +it needed to remove tasks from `map_method_serial_to_task`, but called +`send_message_with_reply_cleanup(remove = FALSE)` and erroneously didn’t +call unref an additional time. + +Try and simplify it all by setting a `GDestroyNotify` on +`map_method_serial_to_task`’s values, and making the refcount handling +of `send_message_with_reply_cleanup()` not be conditional on its +arguments. + +Signed-off-by: Philip Withnall + +Helps: #1264 +--- + gio/gdbusconnection.c | 18 ++++++++++-------- + 1 file changed, 10 insertions(+), 8 deletions(-) + +diff --git a/gio/gdbusconnection.c b/gio/gdbusconnection.c +index 1c1f0cf3d..63f37ca4b 100644 +--- a/gio/gdbusconnection.c ++++ b/gio/gdbusconnection.c +@@ -433,7 +433,7 @@ struct _GDBusConnection + GDBusConnectionFlags flags; + + /* Map used for managing method replies, protected by @lock */ +- GHashTable *map_method_serial_to_task; /* guint32 -> GTask* */ ++ GHashTable *map_method_serial_to_task; /* guint32 -> owned GTask* */ + + /* Maps used for managing signal subscription, protected by @lock */ + GHashTable *map_rule_to_signal_data; /* match rule (gchar*) -> SignalData */ +@@ -1067,7 +1067,7 @@ g_dbus_connection_init (GDBusConnection *connection) + g_mutex_init (&connection->lock); + g_mutex_init (&connection->init_lock); + +- connection->map_method_serial_to_task = g_hash_table_new (g_direct_hash, g_direct_equal); ++ connection->map_method_serial_to_task = g_hash_table_new_full (g_direct_hash, g_direct_equal, NULL, g_object_unref); + + connection->map_rule_to_signal_data = g_hash_table_new (g_str_hash, + g_str_equal); +@@ -1759,7 +1759,7 @@ send_message_data_free (SendMessageData *data) + + /* ---------------------------------------------------------------------------------------------------- */ + +-/* can be called from any thread with lock held; @task is (transfer full) */ ++/* can be called from any thread with lock held; @task is (transfer none) */ + static void + send_message_with_reply_cleanup (GTask *task, gboolean remove) + { +@@ -1789,13 +1789,11 @@ send_message_with_reply_cleanup (GTask *task, gboolean remove) + GUINT_TO_POINTER (data->serial)); + g_warn_if_fail (removed); + } +- +- g_object_unref (task); + } + + /* ---------------------------------------------------------------------------------------------------- */ + +-/* Called from GDBus worker thread with lock held; @task is (transfer full). */ ++/* Called from GDBus worker thread with lock held; @task is (transfer none). */ + static void + send_message_data_deliver_reply_unlocked (GTask *task, + GDBusMessage *reply) +@@ -1813,7 +1811,7 @@ send_message_data_deliver_reply_unlocked (GTask *task, + ; + } + +-/* Called from a user thread, lock is not held */ ++/* Called from a user thread, lock is not held; @task is (transfer none) */ + static void + send_message_data_deliver_error (GTask *task, + GQuark domain, +@@ -1830,7 +1828,10 @@ send_message_data_deliver_error (GTask *task, + return; + } + ++ /* Hold a ref on @task as send_message_with_reply_cleanup() will remove it ++ * from the task map and could end up dropping the last reference */ + g_object_ref (task); ++ + send_message_with_reply_cleanup (task, TRUE); + CONNECTION_UNLOCK (connection); + +@@ -2363,7 +2364,8 @@ on_worker_message_about_to_be_sent (GDBusWorker *worker, + return message; + } + +-/* called with connection lock held, in GDBusWorker thread */ ++/* called with connection lock held, in GDBusWorker thread ++ * @key, @value and @user_data are (transfer none) */ + static gboolean + cancel_method_on_close (gpointer key, gpointer value, gpointer user_data) + { +-- +2.50.0 + diff --git a/SOURCES/gdbus-signal-race.patch b/SOURCES/gdbus-signal-race.patch new file mode 100644 index 0000000..43b20b2 --- /dev/null +++ b/SOURCES/gdbus-signal-race.patch @@ -0,0 +1,780 @@ +From c53784092c2d69c4a1d916bb22235ffb8f0b5bad Mon Sep 17 00:00:00 2001 +From: Philip Withnall +Date: Fri, 17 Jan 2020 16:11:06 +0000 +Subject: [PATCH 1/5] gdbusconnection: Allocate SignalSubscriber structs + individually + +The `SignalSubscriber` structs contain the callback and `user_data` of each +subscriber to a signal, along with the `guint id` token held by that +subscriber to identify their subscription. There are one or more +`SignalSubscriber` structs for a given signal match rule, which is +represented as a `SignalData` struct. + +Previously, the `SignalSubscriber` structs were stored in a `GArray` in +the `SignalData` struct, to reduce the number of allocations needed +when subscribing to a signal. + +However, this means that a `SignalSubscriber` struct cannot have a +lifetime which exceeds the `SignalData` which contains it. In order to +fix the race in #978, one thread needs to be able to unsubscribe from a +signal (destroying the `SignalData` struct) while zero or more other +threads are in the process of calling the callbacks from a previous +emission of that signal (using the callback and `user_data` from zero or +more `SignalSubscriber` structs). Multiple threads could be calling +callbacks because callbacks are invoked in the `GMainContext` which +originally made a subscription, and GDBus supports subscribing to a +signal from multiple threads. In that case, the callbacks are dispatched +to multiple threads. + +In order to allow the `SignalSubscriber` structs to outlive the +`SignalData` which contained their old match rule, store them in a +`GPtrArray` in the `SignalData` struct, and refcount them individually. + +This commit in itself should make no functional changes to how GDBus +works, but will allow following commits to do so. + +Signed-off-by: Philip Withnall + +Helps: #978 +--- + gio/gdbusconnection.c | 105 +++++++++++++++++++++++++----------------- + 1 file changed, 63 insertions(+), 42 deletions(-) + +diff --git a/gio/gdbusconnection.c b/gio/gdbusconnection.c +index 63f37ca4b..170057682 100644 +--- a/gio/gdbusconnection.c ++++ b/gio/gdbusconnection.c +@@ -3229,18 +3229,9 @@ typedef struct + gchar *object_path; + gchar *arg0; + GDBusSignalFlags flags; +- GArray *subscribers; ++ GPtrArray *subscribers; /* (owned) (element-type SignalSubscriber) */ + } SignalData; + +-typedef struct +-{ +- GDBusSignalCallback callback; +- gpointer user_data; +- GDestroyNotify user_data_free_func; +- guint id; +- GMainContext *context; +-} SignalSubscriber; +- + static void + signal_data_free (SignalData *signal_data) + { +@@ -3251,10 +3242,37 @@ signal_data_free (SignalData *signal_data) + g_free (signal_data->member); + g_free (signal_data->object_path); + g_free (signal_data->arg0); +- g_array_free (signal_data->subscribers, TRUE); ++ g_ptr_array_unref (signal_data->subscribers); + g_free (signal_data); + } + ++typedef struct ++{ ++ gatomicrefcount ref_count; ++ GDBusSignalCallback callback; ++ gpointer user_data; ++ GDestroyNotify user_data_free_func; ++ guint id; ++ GMainContext *context; ++} SignalSubscriber; ++ ++static SignalSubscriber * ++signal_subscriber_ref (SignalSubscriber *subscriber) ++{ ++ g_atomic_ref_count_inc (&subscriber->ref_count); ++ return subscriber; ++} ++ ++static void ++signal_subscriber_unref (SignalSubscriber *subscriber) ++{ ++ if (g_atomic_ref_count_dec (&subscriber->ref_count)) ++ { ++ g_main_context_unref (subscriber->context); ++ g_free (subscriber); ++ } ++} ++ + static gchar * + args_to_rule (const gchar *sender, + const gchar *interface_name, +@@ -3440,7 +3458,7 @@ g_dbus_connection_signal_subscribe (GDBusConnection *connection, + { + gchar *rule; + SignalData *signal_data; +- SignalSubscriber subscriber; ++ SignalSubscriber *subscriber; + GPtrArray *signal_data_array; + const gchar *sender_unique_name; + +@@ -3481,17 +3499,19 @@ g_dbus_connection_signal_subscribe (GDBusConnection *connection, + else + sender_unique_name = ""; + +- subscriber.callback = callback; +- subscriber.user_data = user_data; +- subscriber.user_data_free_func = user_data_free_func; +- subscriber.id = (guint) g_atomic_int_add (&_global_subscriber_id, 1); /* TODO: overflow etc. */ +- subscriber.context = g_main_context_ref_thread_default (); ++ subscriber = g_new0 (SignalSubscriber, 1); ++ subscriber->ref_count = 1; ++ subscriber->callback = callback; ++ subscriber->user_data = user_data; ++ subscriber->user_data_free_func = user_data_free_func; ++ subscriber->id = (guint) g_atomic_int_add (&_global_subscriber_id, 1); /* TODO: overflow etc. */ ++ subscriber->context = g_main_context_ref_thread_default (); + + /* see if we've already have this rule */ + signal_data = g_hash_table_lookup (connection->map_rule_to_signal_data, rule); + if (signal_data != NULL) + { +- g_array_append_val (signal_data->subscribers, subscriber); ++ g_ptr_array_add (signal_data->subscribers, subscriber); + g_free (rule); + goto out; + } +@@ -3505,8 +3525,8 @@ g_dbus_connection_signal_subscribe (GDBusConnection *connection, + signal_data->object_path = g_strdup (object_path); + signal_data->arg0 = g_strdup (arg0); + signal_data->flags = flags; +- signal_data->subscribers = g_array_new (FALSE, FALSE, sizeof (SignalSubscriber)); +- g_array_append_val (signal_data->subscribers, subscriber); ++ signal_data->subscribers = g_ptr_array_new_with_free_func ((GDestroyNotify) signal_subscriber_unref); ++ g_ptr_array_add (signal_data->subscribers, subscriber); + + g_hash_table_insert (connection->map_rule_to_signal_data, + signal_data->rule, +@@ -3536,12 +3556,12 @@ g_dbus_connection_signal_subscribe (GDBusConnection *connection, + + out: + g_hash_table_insert (connection->map_id_to_signal_data, +- GUINT_TO_POINTER (subscriber.id), ++ GUINT_TO_POINTER (subscriber->id), + signal_data); + + CONNECTION_UNLOCK (connection); + +- return subscriber.id; ++ return subscriber->id; + } + + /* ---------------------------------------------------------------------------------------------------- */ +@@ -3551,7 +3571,7 @@ g_dbus_connection_signal_subscribe (GDBusConnection *connection, + static void + unsubscribe_id_internal (GDBusConnection *connection, + guint subscription_id, +- GArray *out_removed_subscribers) ++ GPtrArray *out_removed_subscribers) + { + SignalData *signal_data; + GPtrArray *signal_data_array; +@@ -3567,16 +3587,19 @@ unsubscribe_id_internal (GDBusConnection *connection, + + for (n = 0; n < signal_data->subscribers->len; n++) + { +- SignalSubscriber *subscriber; ++ SignalSubscriber *subscriber = signal_data->subscribers->pdata[n]; + +- subscriber = &(g_array_index (signal_data->subscribers, SignalSubscriber, n)); + if (subscriber->id != subscription_id) + continue; + ++ /* It’s OK to rearrange the array order using the ‘fast’ #GPtrArray ++ * removal functions, since we’re going to exit the loop below anyway — we ++ * never move on to the next element. Secondly, subscription IDs are ++ * guaranteed to be unique. */ + g_warn_if_fail (g_hash_table_remove (connection->map_id_to_signal_data, + GUINT_TO_POINTER (subscription_id))); +- g_array_append_val (out_removed_subscribers, *subscriber); +- g_array_remove_index (signal_data->subscribers, n); ++ g_ptr_array_add (out_removed_subscribers, signal_subscriber_ref (subscriber)); ++ g_ptr_array_remove_index_fast (signal_data->subscribers, n); + + if (signal_data->subscribers->len == 0) + { +@@ -3634,13 +3657,13 @@ void + g_dbus_connection_signal_unsubscribe (GDBusConnection *connection, + guint subscription_id) + { +- GArray *subscribers; ++ GPtrArray *subscribers; + guint n; + + g_return_if_fail (G_IS_DBUS_CONNECTION (connection)); + g_return_if_fail (check_initialized (connection)); + +- subscribers = g_array_new (FALSE, FALSE, sizeof (SignalSubscriber)); ++ subscribers = g_ptr_array_new_with_free_func ((GDestroyNotify) signal_subscriber_unref); + + CONNECTION_LOCK (connection); + unsubscribe_id_internal (connection, +@@ -3654,15 +3677,15 @@ g_dbus_connection_signal_unsubscribe (GDBusConnection *connection, + /* call GDestroyNotify without lock held */ + for (n = 0; n < subscribers->len; n++) + { +- SignalSubscriber *subscriber; +- subscriber = &(g_array_index (subscribers, SignalSubscriber, n)); ++ SignalSubscriber *subscriber = subscribers->pdata[n]; + call_destroy_notify (subscriber->context, + subscriber->user_data_free_func, + subscriber->user_data); +- g_main_context_unref (subscriber->context); ++ subscriber->user_data_free_func = NULL; ++ subscriber->user_data = NULL; + } + +- g_array_free (subscribers, TRUE); ++ g_ptr_array_unref (subscribers); + } + + /* ---------------------------------------------------------------------------------------------------- */ +@@ -3852,12 +3875,10 @@ schedule_callbacks (GDBusConnection *connection, + + for (m = 0; m < signal_data->subscribers->len; m++) + { +- SignalSubscriber *subscriber; ++ SignalSubscriber *subscriber = signal_data->subscribers->pdata[m]; + GSource *idle_source; + SignalInstance *signal_instance; + +- subscriber = &(g_array_index (signal_data->subscribers, SignalSubscriber, m)); +- + signal_instance = g_new0 (SignalInstance, 1); + signal_instance->subscription_id = subscriber->id; + signal_instance->callback = subscriber->callback; +@@ -3930,7 +3951,7 @@ purge_all_signal_subscriptions (GDBusConnection *connection) + GHashTableIter iter; + gpointer key; + GArray *ids; +- GArray *subscribers; ++ GPtrArray *subscribers; + guint n; + + ids = g_array_new (FALSE, FALSE, sizeof (guint)); +@@ -3941,7 +3962,7 @@ purge_all_signal_subscriptions (GDBusConnection *connection) + g_array_append_val (ids, subscription_id); + } + +- subscribers = g_array_new (FALSE, FALSE, sizeof (SignalSubscriber)); ++ subscribers = g_ptr_array_new_with_free_func ((GDestroyNotify) signal_subscriber_unref); + for (n = 0; n < ids->len; n++) + { + guint subscription_id = g_array_index (ids, guint, n); +@@ -3954,15 +3975,15 @@ purge_all_signal_subscriptions (GDBusConnection *connection) + /* call GDestroyNotify without lock held */ + for (n = 0; n < subscribers->len; n++) + { +- SignalSubscriber *subscriber; +- subscriber = &(g_array_index (subscribers, SignalSubscriber, n)); ++ SignalSubscriber *subscriber = subscribers->pdata[n]; + call_destroy_notify (subscriber->context, + subscriber->user_data_free_func, + subscriber->user_data); +- g_main_context_unref (subscriber->context); ++ subscriber->user_data_free_func = NULL; ++ subscriber->user_data = NULL; + } + +- g_array_free (subscribers, TRUE); ++ g_ptr_array_unref (subscribers); + } + + /* ---------------------------------------------------------------------------------------------------- */ +-- +2.50.0 + + +From d21d9fb02496f21864e4175d08b5f55416e832e6 Mon Sep 17 00:00:00 2001 +From: Philip Withnall +Date: Fri, 17 Jan 2020 19:52:46 +0000 +Subject: [PATCH 2/5] gdbusconnection: Tidy up destroy notification for signal + subscriptions +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Tie the destruction of the `user_data` to the destruction of the +`SignalSubscriber` struct. This is tidier, and ensures that the fields +in `SignalSubscriber` are all immutable after being set, so the +structure can safely be used across threads without locking. + +It doesn’t matter which thread we call `call_destroy_notify()` in, since +it always defers calling `user_data_free_func` to the user-provided +`GMainContext`. + +Signed-off-by: Philip Withnall + +Helps: #978 +--- + gio/gdbusconnection.c | 32 +++++++++----------------------- + 1 file changed, 9 insertions(+), 23 deletions(-) + +diff --git a/gio/gdbusconnection.c b/gio/gdbusconnection.c +index 170057682..2dba09fcc 100644 +--- a/gio/gdbusconnection.c ++++ b/gio/gdbusconnection.c +@@ -3248,6 +3248,7 @@ signal_data_free (SignalData *signal_data) + + typedef struct + { ++ /* All fields are immutable after construction. */ + gatomicrefcount ref_count; + GDBusSignalCallback callback; + gpointer user_data; +@@ -3268,6 +3269,14 @@ signal_subscriber_unref (SignalSubscriber *subscriber) + { + if (g_atomic_ref_count_dec (&subscriber->ref_count)) + { ++ /* Destroy the user data. It doesn’t matter which thread ++ * signal_subscriber_unref() is called in (or whether it’s called with a ++ * lock held), as call_destroy_notify() always defers to the next ++ * #GMainContext iteration. */ ++ call_destroy_notify (subscriber->context, ++ subscriber->user_data_free_func, ++ subscriber->user_data); ++ + g_main_context_unref (subscriber->context); + g_free (subscriber); + } +@@ -3658,7 +3667,6 @@ g_dbus_connection_signal_unsubscribe (GDBusConnection *connection, + guint subscription_id) + { + GPtrArray *subscribers; +- guint n; + + g_return_if_fail (G_IS_DBUS_CONNECTION (connection)); + g_return_if_fail (check_initialized (connection)); +@@ -3674,17 +3682,6 @@ g_dbus_connection_signal_unsubscribe (GDBusConnection *connection, + /* invariant */ + g_assert (subscribers->len == 0 || subscribers->len == 1); + +- /* call GDestroyNotify without lock held */ +- for (n = 0; n < subscribers->len; n++) +- { +- SignalSubscriber *subscriber = subscribers->pdata[n]; +- call_destroy_notify (subscriber->context, +- subscriber->user_data_free_func, +- subscriber->user_data); +- subscriber->user_data_free_func = NULL; +- subscriber->user_data = NULL; +- } +- + g_ptr_array_unref (subscribers); + } + +@@ -3972,17 +3969,6 @@ purge_all_signal_subscriptions (GDBusConnection *connection) + } + g_array_free (ids, TRUE); + +- /* call GDestroyNotify without lock held */ +- for (n = 0; n < subscribers->len; n++) +- { +- SignalSubscriber *subscriber = subscribers->pdata[n]; +- call_destroy_notify (subscriber->context, +- subscriber->user_data_free_func, +- subscriber->user_data); +- subscriber->user_data_free_func = NULL; +- subscriber->user_data = NULL; +- } +- + g_ptr_array_unref (subscribers); + } + +-- +2.50.0 + + +From 5b7c9398a67ad274cb8faef8bf45461540a7198e Mon Sep 17 00:00:00 2001 +From: Philip Withnall +Date: Fri, 17 Jan 2020 19:38:55 +0000 +Subject: [PATCH 3/5] gdbusconnection: Fix race when emitting D-Bus signal + callbacks +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Instead of storing a copy of the `callback` and `user_data` from a +`SignalSubscriber` in a `SignalInstance` struct (which is the closure +for signal callback data as it’s sent from the D-Bus worker thread to +the thread which originally subscribed to a signal), store a strong +reference to the `SignalSubscriber` struct itself. + +This keeps the `SignalSubscriber` alive until the emission is +complete, which ensures that the `user_data` is not freed prematurely. +It also slightly reduces the allocation size of `SignalInstance` (not +that it matters). + +This is threadsafe because the fields in `SignalSubscriber` are all +immutable after construction. + +Signed-off-by: Philip Withnall + +Helps: #978 +--- + gio/gdbusconnection.c | 27 ++++++++++++--------------- + 1 file changed, 12 insertions(+), 15 deletions(-) + +diff --git a/gio/gdbusconnection.c b/gio/gdbusconnection.c +index 2dba09fcc..4cae53b29 100644 +--- a/gio/gdbusconnection.c ++++ b/gio/gdbusconnection.c +@@ -3689,9 +3689,7 @@ g_dbus_connection_signal_unsubscribe (GDBusConnection *connection, + + typedef struct + { +- guint subscription_id; +- GDBusSignalCallback callback; +- gpointer user_data; ++ SignalSubscriber *subscriber; /* (owned) */ + GDBusMessage *message; + GDBusConnection *connection; + const gchar *sender; +@@ -3723,7 +3721,7 @@ emit_signal_instance_in_idle_cb (gpointer data) + + #if 0 + g_print ("in emit_signal_instance_in_idle_cb (id=%d sender=%s path=%s interface=%s member=%s params=%s)\n", +- signal_instance->subscription_id, ++ signal_instance->subscriber->id, + signal_instance->sender, + signal_instance->path, + signal_instance->interface, +@@ -3735,18 +3733,18 @@ emit_signal_instance_in_idle_cb (gpointer data) + CONNECTION_LOCK (signal_instance->connection); + has_subscription = FALSE; + if (g_hash_table_lookup (signal_instance->connection->map_id_to_signal_data, +- GUINT_TO_POINTER (signal_instance->subscription_id)) != NULL) ++ GUINT_TO_POINTER (signal_instance->subscriber->id)) != NULL) + has_subscription = TRUE; + CONNECTION_UNLOCK (signal_instance->connection); + + if (has_subscription) +- signal_instance->callback (signal_instance->connection, +- signal_instance->sender, +- signal_instance->path, +- signal_instance->interface, +- signal_instance->member, +- parameters, +- signal_instance->user_data); ++ signal_instance->subscriber->callback (signal_instance->connection, ++ signal_instance->sender, ++ signal_instance->path, ++ signal_instance->interface, ++ signal_instance->member, ++ parameters, ++ signal_instance->subscriber->user_data); + + g_variant_unref (parameters); + +@@ -3758,6 +3756,7 @@ signal_instance_free (SignalInstance *signal_instance) + { + g_object_unref (signal_instance->message); + g_object_unref (signal_instance->connection); ++ signal_subscriber_unref (signal_instance->subscriber); + g_free (signal_instance); + } + +@@ -3877,9 +3876,7 @@ schedule_callbacks (GDBusConnection *connection, + SignalInstance *signal_instance; + + signal_instance = g_new0 (SignalInstance, 1); +- signal_instance->subscription_id = subscriber->id; +- signal_instance->callback = subscriber->callback; +- signal_instance->user_data = subscriber->user_data; ++ signal_instance->subscriber = signal_subscriber_ref (subscriber); + signal_instance->message = g_object_ref (message); + signal_instance->connection = g_object_ref (connection); + signal_instance->sender = sender; +-- +2.50.0 + + +From a13bbdc1dc0cd4ec16c3e75d0cc1c07eb791b3bc Mon Sep 17 00:00:00 2001 +From: Philip Withnall +Date: Fri, 17 Jan 2020 19:56:58 +0000 +Subject: [PATCH 4/5] gdbusconnection: Tidy up unsubscription code + +This just removes a now-redundant intermediate array. This means that +the `SignalSubscriber` instances are now potentially freed a little +sooner, inside the locked segment, but they are already careful to only +call their `user_data_free_func` in the right thread. So that should not +deadlock. + +Signed-off-by: Philip Withnall + +Helps: #978 +--- + gio/gdbusconnection.c | 33 +++++++++++---------------------- + 1 file changed, 11 insertions(+), 22 deletions(-) + +diff --git a/gio/gdbusconnection.c b/gio/gdbusconnection.c +index 4cae53b29..4d99f8570 100644 +--- a/gio/gdbusconnection.c ++++ b/gio/gdbusconnection.c +@@ -3576,15 +3576,16 @@ g_dbus_connection_signal_subscribe (GDBusConnection *connection, + /* ---------------------------------------------------------------------------------------------------- */ + + /* called in any thread */ +-/* must hold lock when calling this (except if connection->finalizing is TRUE) */ +-static void ++/* must hold lock when calling this (except if connection->finalizing is TRUE) ++ * returns the number of removed subscribers */ ++static guint + unsubscribe_id_internal (GDBusConnection *connection, +- guint subscription_id, +- GPtrArray *out_removed_subscribers) ++ guint subscription_id) + { + SignalData *signal_data; + GPtrArray *signal_data_array; + guint n; ++ guint n_removed = 0; + + signal_data = g_hash_table_lookup (connection->map_id_to_signal_data, + GUINT_TO_POINTER (subscription_id)); +@@ -3607,7 +3608,7 @@ unsubscribe_id_internal (GDBusConnection *connection, + * guaranteed to be unique. */ + g_warn_if_fail (g_hash_table_remove (connection->map_id_to_signal_data, + GUINT_TO_POINTER (subscription_id))); +- g_ptr_array_add (out_removed_subscribers, signal_subscriber_ref (subscriber)); ++ n_removed++; + g_ptr_array_remove_index_fast (signal_data->subscribers, n); + + if (signal_data->subscribers->len == 0) +@@ -3649,7 +3650,7 @@ unsubscribe_id_internal (GDBusConnection *connection, + g_assert_not_reached (); + + out: +- ; ++ return n_removed; + } + + /** +@@ -3666,23 +3667,17 @@ void + g_dbus_connection_signal_unsubscribe (GDBusConnection *connection, + guint subscription_id) + { +- GPtrArray *subscribers; ++ guint n_subscribers_removed G_GNUC_UNUSED /* when compiling with G_DISABLE_ASSERT */; + + g_return_if_fail (G_IS_DBUS_CONNECTION (connection)); + g_return_if_fail (check_initialized (connection)); + +- subscribers = g_ptr_array_new_with_free_func ((GDestroyNotify) signal_subscriber_unref); +- + CONNECTION_LOCK (connection); +- unsubscribe_id_internal (connection, +- subscription_id, +- subscribers); ++ n_subscribers_removed = unsubscribe_id_internal (connection, subscription_id); + CONNECTION_UNLOCK (connection); + + /* invariant */ +- g_assert (subscribers->len == 0 || subscribers->len == 1); +- +- g_ptr_array_unref (subscribers); ++ g_assert (n_subscribers_removed == 0 || n_subscribers_removed == 1); + } + + /* ---------------------------------------------------------------------------------------------------- */ +@@ -3945,7 +3940,6 @@ purge_all_signal_subscriptions (GDBusConnection *connection) + GHashTableIter iter; + gpointer key; + GArray *ids; +- GPtrArray *subscribers; + guint n; + + ids = g_array_new (FALSE, FALSE, sizeof (guint)); +@@ -3956,17 +3950,12 @@ purge_all_signal_subscriptions (GDBusConnection *connection) + g_array_append_val (ids, subscription_id); + } + +- subscribers = g_ptr_array_new_with_free_func ((GDestroyNotify) signal_subscriber_unref); + for (n = 0; n < ids->len; n++) + { + guint subscription_id = g_array_index (ids, guint, n); +- unsubscribe_id_internal (connection, +- subscription_id, +- subscribers); ++ unsubscribe_id_internal (connection, subscription_id); + } + g_array_free (ids, TRUE); +- +- g_ptr_array_unref (subscribers); + } + + /* ---------------------------------------------------------------------------------------------------- */ +-- +2.50.0 + + +From 0ec439aab1da5df011bed6a1a317439efacb6c25 Mon Sep 17 00:00:00 2001 +From: Philip Withnall +Date: Fri, 17 Jan 2020 20:00:22 +0000 +Subject: [PATCH 5/5] gdbusnameowning: Fix race between connection shutdown and + NameLost +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +As with all D-Bus signal subscriptions, it’s possible for a signal +callback to be invoked in one thread (T1) while another thread (T2) is +unsubscribing from that signal. In this case, T1 is the main thread, and +T2 is the D-Bus connection worker thread which is unsubscribing all +signals as it’s in the process of closing. + +Due to this possibility, all `user_data` for signal callbacks needs to +be referenced outside the lifecycle of the code which +subscribes/unsubscribes the signal. In other words, it’s not safe to +subscribe to a signal, store the subscription ID in a struct, +unsubscribe from the signal when freeing the struct, and dereference the +struct in the signal callback. The data passed to the signal callback +has to have its own strong reference. + +Instead, it’s safe to subscribe to a signal and add a strong reference +to the struct, store the subscription ID in that struct, and unsubscribe +from the signal when the last external reference to your struct is +dropped. That unsubscription should break the refcount cycle between the +signal connection and the struct, and allow the struct to be completely +freed. Only with that approach is it safe to dereference the struct in +the signal callback, if there’s any possibility that the signal might be +unsubscribed from a separate thread. + +The tests need specific additional main loop cycles to completely emit +the NameLost signal callback. Ideally they need refactoring, but this +will do (1000 test cycles passed). + +Signed-off-by: Philip Withnall + +Fixes: #978 +--- + gio/gdbusnameowning.c | 15 ++++++++++----- + gio/tests/gdbus-names.c | 11 ++++++++++- + 2 files changed, 20 insertions(+), 6 deletions(-) + +diff --git a/gio/gdbusnameowning.c b/gio/gdbusnameowning.c +index 2c2714db2..00554c16a 100644 +--- a/gio/gdbusnameowning.c ++++ b/gio/gdbusnameowning.c +@@ -366,7 +366,12 @@ request_name_cb (GObject *source_object, + connection = g_object_ref (client->connection); + G_UNLOCK (lock); + +- /* start listening to NameLost and NameAcquired messages */ ++ /* Start listening to NameLost and NameAcquired messages. We hold ++ * references to the Client in the signal closures, since it’s possible ++ * for a signal to be in-flight after unsubscribing the signal handler. ++ * This creates a reference count cycle, but that’s explicitly broken by ++ * disconnecting the signal handlers before calling client_unref() in ++ * g_bus_unown_name(). */ + if (connection != NULL) + { + client->name_lost_subscription_id = +@@ -378,8 +383,8 @@ request_name_cb (GObject *source_object, + client->name, + G_DBUS_SIGNAL_FLAGS_NONE, + on_name_lost_or_acquired, +- client, +- NULL); ++ client_ref (client), ++ (GDestroyNotify) client_unref); + client->name_acquired_subscription_id = + g_dbus_connection_signal_subscribe (connection, + "org.freedesktop.DBus", +@@ -389,8 +394,8 @@ request_name_cb (GObject *source_object, + client->name, + G_DBUS_SIGNAL_FLAGS_NONE, + on_name_lost_or_acquired, +- client, +- NULL); ++ client_ref (client), ++ (GDestroyNotify) client_unref); + g_object_unref (connection); + } + } +diff --git a/gio/tests/gdbus-names.c b/gio/tests/gdbus-names.c +index 648b54774..b42ba2141 100644 +--- a/gio/tests/gdbus-names.c ++++ b/gio/tests/gdbus-names.c +@@ -189,6 +189,7 @@ test_bus_own_name (void) + * Stop owning the name - this should invoke our free func + */ + g_bus_unown_name (id); ++ g_main_loop_run (loop); + g_assert_cmpint (data.num_free_func, ==, 2); + + /* +@@ -330,6 +331,7 @@ test_bus_own_name (void) + g_assert_cmpint (data2.num_acquired, ==, 0); + g_assert_cmpint (data2.num_lost, ==, 1); + g_bus_unown_name (id2); ++ g_main_loop_run (loop); + g_assert_cmpint (data2.num_bus_acquired, ==, 0); + g_assert_cmpint (data2.num_acquired, ==, 0); + g_assert_cmpint (data2.num_lost, ==, 1); +@@ -355,6 +357,7 @@ test_bus_own_name (void) + g_assert_cmpint (data2.num_acquired, ==, 0); + g_assert_cmpint (data2.num_lost, ==, 1); + g_bus_unown_name (id2); ++ g_main_loop_run (loop); + g_assert_cmpint (data2.num_bus_acquired, ==, 0); + g_assert_cmpint (data2.num_acquired, ==, 0); + g_assert_cmpint (data2.num_lost, ==, 1); +@@ -365,6 +368,7 @@ test_bus_own_name (void) + */ + data.expect_null_connection = FALSE; + g_bus_unown_name (id); ++ g_main_loop_run (loop); + g_assert_cmpint (data.num_bus_acquired, ==, 1); + g_assert_cmpint (data.num_acquired, ==, 1); + g_assert_cmpint (data.num_free_func, ==, 4); +@@ -418,6 +422,7 @@ test_bus_own_name (void) + g_assert_cmpint (data2.num_acquired, ==, 0); + g_assert_cmpint (data2.num_lost, ==, 1); + g_bus_unown_name (id2); ++ g_main_loop_run (loop); + g_assert_cmpint (data2.num_bus_acquired, ==, 0); + g_assert_cmpint (data2.num_acquired, ==, 0); + g_assert_cmpint (data2.num_lost, ==, 1); +@@ -450,8 +455,9 @@ test_bus_own_name (void) + g_assert_cmpint (data2.num_bus_acquired, ==, 0); + /* ok, make owner2 release the name - then wait for owner to automagically reacquire it */ + g_bus_unown_name (id2); +- g_assert_cmpint (data2.num_free_func, ==, 1); + g_main_loop_run (loop); ++ g_main_loop_run (loop); ++ g_assert_cmpint (data2.num_free_func, ==, 1); + g_assert_cmpint (data.num_acquired, ==, 2); + g_assert_cmpint (data.num_lost, ==, 1); + +@@ -466,6 +472,7 @@ test_bus_own_name (void) + g_assert_cmpint (data.num_acquired, ==, 2); + g_assert_cmpint (data.num_lost, ==, 2); + g_bus_unown_name (id); ++ g_main_loop_run (loop); + g_assert_cmpint (data.num_free_func, ==, 5); + + g_object_unref (c); +@@ -648,6 +655,7 @@ test_bus_watch_name (void) + + /* unown the name */ + g_bus_unown_name (owner_id); ++ g_main_loop_run (loop); + g_assert_cmpint (data.num_acquired, ==, 1); + g_assert_cmpint (data.num_free_func, ==, 2); + +@@ -707,6 +715,7 @@ test_bus_watch_name (void) + g_assert_cmpint (data.num_free_func, ==, 1); + + g_bus_unown_name (owner_id); ++ g_main_loop_run (loop); + g_assert_cmpint (data.num_free_func, ==, 2); + + session_bus_down (); +-- +2.50.0 + diff --git a/SPECS/glib2.spec b/SPECS/glib2.spec index bff6022..206c0c8 100644 --- a/SPECS/glib2.spec +++ b/SPECS/glib2.spec @@ -5,7 +5,7 @@ Name: glib2 Version: 2.56.4 -Release: 165%{?dist} +Release: 166%{?dist} Summary: A library of handy utility functions License: LGPLv2+ @@ -128,6 +128,25 @@ Patch23: 1549.patch # https://gitlab.gnome.org/GNOME/glib/-/merge_requests/4155 Patch24: 4155.patch +# https://gitlab.gnome.org/GNOME/glib/-/merge_requests/4281 +Patch25: CVE-2024-52533.patch + +# https://gitlab.gnome.org/GNOME/glib/-/merge_requests/4588 +# https://gitlab.gnome.org/GNOME/glib/-/merge_requests/4592 +Patch26: CVE-2025-4373.patch + +# Contains commits from: +# https://gitlab.gnome.org/GNOME/glib/-/merge_requests/1121 +# https://gitlab.gnome.org/GNOME/glib/-/merge_requests/3291 +Patch27: gdbus-conflict-reduction.patch +# https://gitlab.gnome.org/GNOME/glib/-/merge_requests/1332 +Patch28: gdbus-signal-race.patch +# https://gitlab.gnome.org/GNOME/glib/-/merge_requests/4038 +Patch29: CVE-2024-34397.patch + +# https://gitlab.gnome.org/GNOME/glib/-/merge_requests/4356 +Patch30: gdatetime-test.patch + %description GLib is the low-level core library that forms the basis for projects such as GTK+ and GNOME. It provides data structure handling for C, @@ -325,6 +344,13 @@ glib-compile-schemas %{_datadir}/glib-2.0/schemas &> /dev/null || : %{_datadir}/installed-tests %changelog +* Fri Jul 11 2025 Michael Catanzaro - 2.56.4-166 +- Add patches for CVE-2024-34397, CVE-2024-52533, CVE-2025-4373 +- Update GDateTime test for new tzdata +- Resolves: RHEL-67084 +- Resolves: RHEL-94286 +- Resolves: RHEL-94848 + * Thu Sep 26 2024 Ondrej Holy - 2.56.4-165 - Add support for x-gvfs-trash mount option - Resolves: RHEL-46828