rpms
/
glib2
20
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity
You can not select more than 25 topics Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
Branches Tags
${ item.name }
Create tag ${ searchTerm }
Create branch ${ searchTerm }
from '2212700d7a'
${ noResults }
glib2/SOURCES/4038.patch

3360 lines
115 KiB
Raw Blame History Escape

This file contains ambiguous Unicode characters!

This file contains ambiguous Unicode characters that may be confused with others in your current locale. If your use case is intentional and legitimate, you can safely ignore this warning. Use the Escape button to highlight these characters.

From 82f62517eb327aa3ea255294a90911091f39d7ae Mon Sep 17 00:00:00 2001
From: Philip Withnall <pwithnall@gnome.org>
Date: Tue, 28 Nov 2023 12:58:20 +0000
Subject: [PATCH 01/19] gdbusmessage: Cache the arg0 value
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Technically we can’t rely on it being kept alive by the `message->body`
pointer, unless we can guarantee that the `GVariant` is always
serialised. That’s not necessarily the case, so keep a separate ref on
the arg0 value at all times.
This avoids a potential use-after-free.
Spotted by Thomas Haller in
https://gitlab.gnome.org/GNOME/glib/-/merge_requests/3720#note_1924707.
[This is a prerequisite for having tests pass after fixing the
vulnerability described in glib#3268, because after fixing that
vulnerability, the use-after-free genuinely does happen during
regression testing. -smcv]
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3183, #3268
(cherry picked from commit 10e9a917be7fb92b6b27837ef7a7f1d0be6095d5)
---
gio/gdbusmessage.c | 35 ++++++++++++++++++++++-------------
1 file changed, 22 insertions(+), 13 deletions(-)
diff --git a/gio/gdbusmessage.c b/gio/gdbusmessage.c
index bc9386ee7..40a77dd92 100644
--- a/gio/gdbusmessage.c
+++ b/gio/gdbusmessage.c
@@ -471,6 +471,7 @@ struct _GDBusMessage
guint32 serial;
GHashTable *headers;
GVariant *body;
+ GVariant *arg0_cache; /* (nullable) (owned) */
#ifdef G_OS_UNIX
GUnixFDList *fd_list;
#endif
@@ -493,6 +494,7 @@ g_dbus_message_finalize (GObject *object)
g_hash_table_unref (message->headers);
if (message->body != NULL)
g_variant_unref (message->body);
+ g_clear_pointer (&message->arg0_cache, g_variant_unref);
#ifdef G_OS_UNIX
if (message->fd_list != NULL)
g_object_unref (message->fd_list);
@@ -1128,6 +1130,7 @@ g_dbus_message_set_body (GDBusMessage *message,
if (body == NULL)
{
message->body = NULL;
+ message->arg0_cache = NULL;
g_dbus_message_set_signature (message, NULL);
}
else
@@ -1138,6 +1141,12 @@ g_dbus_message_set_body (GDBusMessage *message,
message->body = g_variant_ref_sink (body);
+ if (g_variant_is_of_type (message->body, G_VARIANT_TYPE_TUPLE) &&
+ g_variant_n_children (message->body) > 0)
+ message->arg0_cache = g_variant_get_child_value (message->body, 0);
+ else
+ message->arg0_cache = NULL;
+
type_string = g_variant_get_type_string (body);
type_string_len = strlen (type_string);
g_assert (type_string_len >= 2);
@@ -2230,6 +2239,14 @@ g_dbus_message_new_from_blob (guchar *blob,
2,
error);
g_variant_type_free (variant_type);
+
+ if (message->body != NULL &&
+ g_variant_is_of_type (message->body, G_VARIANT_TYPE_TUPLE) &&
+ g_variant_n_children (message->body) > 0)
+ message->arg0_cache = g_variant_get_child_value (message->body, 0);
+ else
+ message->arg0_cache = NULL;
+
if (message->body == NULL)
goto out;
}
@@ -3265,22 +3282,13 @@ g_dbus_message_set_signature (GDBusMessage *message,
const gchar *
g_dbus_message_get_arg0 (GDBusMessage *message)
{
- const gchar *ret;
-
g_return_val_if_fail (G_IS_DBUS_MESSAGE (message), NULL);
- ret = NULL;
+ if (message->arg0_cache != NULL &&
+ g_variant_is_of_type (message->arg0_cache, G_VARIANT_TYPE_STRING))
+ return g_variant_get_string (message->arg0_cache, NULL);
- if (message->body != NULL && g_variant_is_of_type (message->body, G_VARIANT_TYPE_TUPLE))
- {
- GVariant *item;
- item = g_variant_get_child_value (message->body, 0);
- if (g_variant_is_of_type (item, G_VARIANT_TYPE_STRING))
- ret = g_variant_get_string (item, NULL);
- g_variant_unref (item);
- }
-
- return ret;
+ return NULL;
}
/* ---------------------------------------------------------------------------------------------------- */
@@ -3723,6 +3731,7 @@ g_dbus_message_copy (GDBusMessage *message,
* to just ref (as opposed to deep-copying) the GVariant instances
*/
ret->body = message->body != NULL ? g_variant_ref (message->body) : NULL;
+ ret->arg0_cache = message->arg0_cache != NULL ? g_variant_ref (message->arg0_cache) : NULL;
g_hash_table_iter_init (&iter, message->headers);
while (g_hash_table_iter_next (&iter, &header_key, (gpointer) &header_value))
g_hash_table_insert (ret->headers, header_key, g_variant_ref (header_value));
--
2.45.0
From 3649ce253433149dea16d158542e7285655eb182 Mon Sep 17 00:00:00 2001
From: Simon McVittie <smcv@collabora.com>
Date: Wed, 1 May 2024 15:51:42 +0100
Subject: [PATCH 02/19] gdbusconnection: Make a backport of g_set_str()
available
A subsequent commit will need this. Copying all of g_set_str() into a
private header seems cleaner than replacing the call to it.
Helps: GNOME/glib#3268
Signed-off-by: Simon McVittie <smcv@collabora.com>
---
gio/gdbusconnection.c | 1 +
glib/glib-private.h | 18 ++++++++++++++++++
2 files changed, 19 insertions(+)
diff --git a/gio/gdbusconnection.c b/gio/gdbusconnection.c
index a37611275..2808b1e46 100644
--- a/gio/gdbusconnection.c
+++ b/gio/gdbusconnection.c
@@ -95,6 +95,7 @@
#include <stdlib.h>
#include <string.h>
+#include "glib-private.h"
#include "gdbusauth.h"
#include "gdbusutils.h"
#include "gdbusaddress.h"
diff --git a/glib/glib-private.h b/glib/glib-private.h
index 8de380d12..acdfa4911 100644
--- a/glib/glib-private.h
+++ b/glib/glib-private.h
@@ -161,4 +161,22 @@ GLibPrivateVTable *glib__private__ (void);
# define GLIB_DEFAULT_LOCALE ""
#endif
+/* Backported from GLib 2.78.x, where it is public API in gstrfuncs.h */
+static inline 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;
+}
+
#endif /* __GLIB_PRIVATE_H__ */
--
2.45.0
From cef2cd7a033d3df50a8b5b3db28df93feb1d14a9 Mon Sep 17 00:00:00 2001
From: Simon McVittie <smcv@collabora.com>
Date: Fri, 8 Mar 2024 14:19:46 +0000
Subject: [PATCH 03/19] 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 <smcv@collabora.com>
---
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 <gio/gio.h>
+
+#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 e1f583c70..1cd55e4ad 100644
--- a/gio/tests/meson.build
+++ b/gio/tests/meson.build
@@ -314,6 +314,7 @@ if host_machine.system() != 'windows'
},
'gdbus-proxy-unique-name' : {'extra_sources' : extra_sources},
'gdbus-proxy-well-known-name' : {'extra_sources' : extra_sources},
+ 'gdbus-subscribe' : {'extra_sources' : extra_sources},
'gdbus-test-codegen' : {
'extra_sources' : [extra_sources, gdbus_test_codegen_generated, gdbus_test_codegen_generated_interface_info],
'c_args' : ['-DGLIB_VERSION_MIN_REQUIRED=GLIB_VERSION_2_32'],
--
2.45.0
From 38ee87713f8cc99c67ef00a4960027676cc16b25 Mon Sep 17 00:00:00 2001
From: Simon McVittie <smcv@collabora.com>
Date: Fri, 8 Mar 2024 19:28:15 +0000
Subject: [PATCH 04/19] tests: Add support for subscribing to signals from a
well-known name
Signed-off-by: Simon McVittie <smcv@collabora.com>
---
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.45.0
From 59ff44e332059175a47af3ed02da28714b70c51b Mon Sep 17 00:00:00 2001
From: Simon McVittie <smcv@collabora.com>
Date: Fri, 8 Mar 2024 19:44:03 +0000
Subject: [PATCH 05/19] 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 <smcv@collabora.com>
---
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.45.0
From 600d631e0978fd529877f481c10a7b3e971337cf Mon Sep 17 00:00:00 2001
From: Simon McVittie <smcv@collabora.com>
Date: Fri, 8 Mar 2024 20:10:29 +0000
Subject: [PATCH 06/19] 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 <smcv@collabora.com>
---
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.45.0
From d338bad6baa8a198953b109c00a96b02a18a8103 Mon Sep 17 00:00:00 2001
From: Simon McVittie <smcv@collabora.com>
Date: Thu, 14 Mar 2024 19:18:15 +0000
Subject: [PATCH 07/19] 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 <smcv@collabora.com>
---
gio/gdbusprivate.h | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/gio/gdbusprivate.h b/gio/gdbusprivate.h
index 8f8a93ba7..e9bca11ca 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.45.0
From a207a6f5adc9e6cea929c07a9d7533cc90043d01 Mon Sep 17 00:00:00 2001
From: Simon McVittie <smcv@collabora.com>
Date: Wed, 1 May 2024 19:43:24 +0100
Subject: [PATCH 08/19] 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.
[Backport to 2.66.x: fix minor conflicts]
Signed-off-by: Simon McVittie <smcv@collabora.com>
---
gio/gdbusconnection.c | 128 +++++++++++++++++++++---------------------
1 file changed, 65 insertions(+), 63 deletions(-)
diff --git a/gio/gdbusconnection.c b/gio/gdbusconnection.c
index 2808b1e46..8d841d78b 100644
--- a/gio/gdbusconnection.c
+++ b/gio/gdbusconnection.c
@@ -299,6 +299,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
@@ -3253,69 +3318,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.45.0
From 998cee11197dda4f48334921900bca0e6a8cd62b Mon Sep 17 00:00:00 2001
From: Simon McVittie <smcv@collabora.com>
Date: Thu, 14 Mar 2024 19:30:12 +0000
Subject: [PATCH 09/19] 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 <smcv@collabora.com>
---
gio/gdbusconnection.c | 42 ++++++++++++++++++++++++++++++++----------
1 file changed, 32 insertions(+), 10 deletions(-)
diff --git a/gio/gdbusconnection.c b/gio/gdbusconnection.c
index 8d841d78b..67091574a 100644
--- a/gio/gdbusconnection.c
+++ b/gio/gdbusconnection.c
@@ -348,6 +348,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)
{
@@ -3584,16 +3608,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.45.0
From 6df7b138b1f5b4c53ad124182b80e05cdba16c3c Mon Sep 17 00:00:00 2001
From: Simon McVittie <smcv@collabora.com>
Date: Tue, 23 Apr 2024 20:31:57 +0100
Subject: [PATCH 10/19] gdbusconnection: Factor out add_signal_data()
No functional changes.
Signed-off-by: Simon McVittie <smcv@collabora.com>
---
gio/gdbusconnection.c | 64 +++++++++++++++++++++++++------------------
1 file changed, 37 insertions(+), 27 deletions(-)
diff --git a/gio/gdbusconnection.c b/gio/gdbusconnection.c
index 67091574a..a570e5856 100644
--- a/gio/gdbusconnection.c
+++ b/gio/gdbusconnection.c
@@ -3462,6 +3462,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
@@ -3551,7 +3587,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
@@ -3617,32 +3652,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.45.0
From dd4a94708ef1bd63763bedcb93161746cbc9c7e6 Mon Sep 17 00:00:00 2001
From: Simon McVittie <smcv@collabora.com>
Date: Thu, 14 Mar 2024 19:51:59 +0000
Subject: [PATCH 11/19] 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 <smcv@collabora.com>
---
gio/gdbusconnection.c | 83 +++++++++++++++++++++++++------------------
1 file changed, 48 insertions(+), 35 deletions(-)
diff --git a/gio/gdbusconnection.c b/gio/gdbusconnection.c
index a570e5856..b2268e637 100644
--- a/gio/gdbusconnection.c
+++ b/gio/gdbusconnection.c
@@ -3666,6 +3666,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 */
@@ -3674,7 +3720,6 @@ unsubscribe_id_internal (GDBusConnection *connection,
guint subscription_id)
{
SignalData *signal_data;
- GPtrArray *signal_data_array;
guint n;
guint n_removed = 0;
@@ -3701,40 +3746,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.45.0
From 64758288c6e71dd92270771b6b81891e4be87fa9 Mon Sep 17 00:00:00 2001
From: Simon McVittie <smcv@collabora.com>
Date: Tue, 23 Apr 2024 20:39:05 +0100
Subject: [PATCH 12/19] 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 <smcv@collabora.com>
---
gio/gdbusconnection.c | 36 ++++++++++++++++++++++++------------
1 file changed, 24 insertions(+), 12 deletions(-)
diff --git a/gio/gdbusconnection.c b/gio/gdbusconnection.c
index b2268e637..e17703d0f 100644
--- a/gio/gdbusconnection.c
+++ b/gio/gdbusconnection.c
@@ -339,19 +339,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,
@@ -362,7 +362,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;
@@ -377,7 +377,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);
@@ -3453,7 +3452,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 ||
@@ -3465,7 +3464,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;
@@ -3485,12 +3485,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);
@@ -3587,6 +3587,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
@@ -3622,6 +3623,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 = "";
@@ -3645,14 +3651,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,
@@ -3676,22 +3682,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.45.0
From 3dd78b5a67c0dea7c453d29345cf42f6c30a600b Mon Sep 17 00:00:00 2001
From: Simon McVittie <smcv@collabora.com>
Date: Wed, 1 May 2024 15:43:09 +0100
Subject: [PATCH 13/19] 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.
[Backported to glib-2-74, resolving minor conflicts]
Signed-off-by: Simon McVittie <smcv@collabora.com>
---
gio/gdbusconnection.c | 350 +++++++++++++++++++++++++++++++++++++++++-
1 file changed, 343 insertions(+), 7 deletions(-)
diff --git a/gio/gdbusconnection.c b/gio/gdbusconnection.c
index e17703d0f..6d18b2457 100644
--- a/gio/gdbusconnection.c
+++ b/gio/gdbusconnection.c
@@ -336,6 +336,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;
@@ -345,13 +370,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,
@@ -362,7 +410,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;
@@ -375,6 +422,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);
@@ -382,6 +440,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);
}
@@ -513,6 +572,7 @@ struct _GDBusConnection
/* Map used for managing method replies, protected by @lock */
GHashTable *map_method_serial_to_task; /* guint32 -> 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 */
@@ -760,6 +820,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);
@@ -1155,6 +1216,7 @@ g_dbus_connection_init (GDBusConnection *connection)
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_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);
@@ -2281,6 +2343,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;
@@ -2392,6 +2639,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);
@@ -2407,6 +2655,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)
@@ -3586,6 +3847,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;
@@ -3651,13 +3913,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:
@@ -3685,10 +3993,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 = "";
@@ -3721,6 +4037,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);
}
@@ -3990,6 +4315,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];
@@ -4051,7 +4387,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.45.0
From ccaea1caa6ebafd931b6221f241dc90d5fd96e62 Mon Sep 17 00:00:00 2001
From: Simon McVittie <smcv@collabora.com>
Date: Thu, 14 Mar 2024 20:42:41 +0000
Subject: [PATCH 14/19] 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 <smcv@collabora.com>
---
gio/gdbusconnection.c | 40 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 40 insertions(+)
diff --git a/gio/gdbusconnection.c b/gio/gdbusconnection.c
index 6d18b2457..85f99c5c6 100644
--- a/gio/gdbusconnection.c
+++ b/gio/gdbusconnection.c
@@ -4296,6 +4296,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.45.0
From 9fe1b11d43ca8f8d77ecf5e3f0976306d6d099ec Mon Sep 17 00:00:00 2001
From: Simon McVittie <smcv@collabora.com>
Date: Fri, 8 Mar 2024 19:51:50 +0000
Subject: [PATCH 15/19] 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 <smcv@collabora.com>
---
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.45.0
From d9c20bc802c6f2fc8e36c51acad7a751de5f1b3f Mon Sep 17 00:00:00 2001
From: Simon McVittie <smcv@collabora.com>
Date: Fri, 8 Mar 2024 19:53:22 +0000
Subject: [PATCH 16/19] 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 <smcv@collabora.com>
---
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.45.0
From 420402151349d1e34ea13b7585c76d3a51ba48f7 Mon Sep 17 00:00:00 2001
From: Simon McVittie <smcv@collabora.com>
Date: Tue, 23 Apr 2024 21:39:43 +0100
Subject: [PATCH 17/19] 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 <smcv@collabora.com>
---
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.45.0
From 6762e278c870dfaad4b26329da5e24b2e9bdceda Mon Sep 17 00:00:00 2001
From: Simon McVittie <smcv@collabora.com>
Date: Wed, 8 May 2024 14:46:08 +0000
Subject: [PATCH 18/19] 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 <smcv@debian.org>
---
gio/gdbusconnection.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/gio/gdbusconnection.c b/gio/gdbusconnection.c
index 85f99c5c6..348b5b985 100644
--- a/gio/gdbusconnection.c
+++ b/gio/gdbusconnection.c
@@ -2406,7 +2406,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\"",
@@ -2458,7 +2461,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.45.0
From 9ad45e2d6699157d8b233bc67881e55f9b71f613 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= <mail@3v1n0.net>
Date: Wed, 8 May 2024 22:53:51 +0200
Subject: [PATCH 19/19] gdbusmessage: Clean the cached arg0 when setting the
message body
We're now caching arg0 but such value is not cleared when a new body is
set as it's in the connection filter test cases where we've a leak as
highlighted by both valgrind and leak sanitizer
---
gio/gdbusmessage.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/gio/gdbusmessage.c b/gio/gdbusmessage.c
index 40a77dd92..7489c0ac4 100644
--- a/gio/gdbusmessage.c
+++ b/gio/gdbusmessage.c
@@ -1127,10 +1127,12 @@ g_dbus_message_set_body (GDBusMessage *message,
if (message->body != NULL)
g_variant_unref (message->body);
+
+ g_clear_pointer (&message->arg0_cache, g_variant_unref);
+
if (body == NULL)
{
message->body = NULL;
- message->arg0_cache = NULL;
g_dbus_message_set_signature (message, NULL);
}
else
@@ -1144,8 +1146,6 @@ g_dbus_message_set_body (GDBusMessage *message,
if (g_variant_is_of_type (message->body, G_VARIANT_TYPE_TUPLE) &&
g_variant_n_children (message->body) > 0)
message->arg0_cache = g_variant_get_child_value (message->body, 0);
- else
- message->arg0_cache = NULL;
type_string = g_variant_get_type_string (body);
type_string_len = strlen (type_string);
--
2.45.0
Reference in new issue View Git Blame Copy Permalink