183 lines
6.3 KiB
Diff
183 lines
6.3 KiB
Diff
|
From 1f86923766a3d1d319fe54ad24fcf6e2d75aca0d Mon Sep 17 00:00:00 2001
|
|||
|
|
From: Philip Withnall <pwithnall@endlessos.org>
|
|||
|
|
Date: Wed, 22 Feb 2023 12:40:49 +0000
|
|||
|
|
Subject: [PATCH 1/3] gdbusinterfaceskeleton: Remove an unnecessary helper
|
|||
|
|
struct member
|
|||
|
|
MIME-Version: 1.0
|
|||
|
|
Content-Type: text/plain; charset=UTF-8
|
|||
|
|
Content-Transfer-Encoding: 8bit
|
|||
|
|
|
|||
|
|
The `GDBusInterfaceSkeleton` is already stored as the source object of
|
|||
|
|
the `GTask` here, with a strong reference.
|
|||
|
|
|
|||
|
|
Storing it again in the task’s data struct is redundant, and makes it
|
|||
|
|
look like the `GDBusInterfaceSkeleton` is being used without holding a
|
|||
|
|
strong reference. (There’s not actually a bug there though: the strong
|
|||
|
|
reference from the `GTask` outlives the data struct, so is sufficient.)
|
|||
|
|
|
|||
|
|
Remove the unnecessary helper struct member to clarify the code a bit.
|
|||
|
|
|
|||
|
|
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
|
|||
|
|
|
|||
|
|
Helps: #2924
|
|||
|
|
---
|
|||
|
|
gio/gdbusinterfaceskeleton.c | 15 +++++++--------
|
|||
|
|
1 file changed, 7 insertions(+), 8 deletions(-)
|
|||
|
|
|
|||
|
|
diff --git a/gio/gdbusinterfaceskeleton.c b/gio/gdbusinterfaceskeleton.c
|
|||
|
|
index 3f07d4d0b2..d28282fea3 100644
|
|||
|
|
--- a/gio/gdbusinterfaceskeleton.c
|
|||
|
|
+++ b/gio/gdbusinterfaceskeleton.c
|
|||
|
|
@@ -461,7 +461,6 @@ dbus_interface_interface_init (GDBusInterfaceIface *iface)
|
|||
|
|
typedef struct
|
|||
|
|
{
|
|||
|
|
gint ref_count; /* (atomic) */
|
|||
|
|
- GDBusInterfaceSkeleton *interface;
|
|||
|
|
GDBusInterfaceMethodCallFunc method_call_func;
|
|||
|
|
GDBusMethodInvocation *invocation;
|
|||
|
|
} DispatchData;
|
|||
|
|
@@ -502,16 +501,17 @@ dispatch_in_thread_func (GTask *task,
|
|||
|
|
GCancellable *cancellable)
|
|||
|
|
{
|
|||
|
|
DispatchData *data = task_data;
|
|||
|
|
+ GDBusInterfaceSkeleton *interface = g_task_get_source_object (task);
|
|||
|
|
GDBusInterfaceSkeletonFlags flags;
|
|||
|
|
GDBusObject *object;
|
|||
|
|
gboolean authorized;
|
|||
|
|
|
|||
|
|
- g_mutex_lock (&data->interface->priv->lock);
|
|||
|
|
- flags = data->interface->priv->flags;
|
|||
|
|
- object = data->interface->priv->object;
|
|||
|
|
+ g_mutex_lock (&interface->priv->lock);
|
|||
|
|
+ flags = interface->priv->flags;
|
|||
|
|
+ object = interface->priv->object;
|
|||
|
|
if (object != NULL)
|
|||
|
|
g_object_ref (object);
|
|||
|
|
- g_mutex_unlock (&data->interface->priv->lock);
|
|||
|
|
+ g_mutex_unlock (&interface->priv->lock);
|
|||
|
|
|
|||
|
|
/* first check on the enclosing object (if any), then the interface */
|
|||
|
|
authorized = TRUE;
|
|||
|
|
@@ -519,13 +519,13 @@ dispatch_in_thread_func (GTask *task,
|
|||
|
|
{
|
|||
|
|
g_signal_emit_by_name (object,
|
|||
|
|
"authorize-method",
|
|||
|
|
- data->interface,
|
|||
|
|
+ interface,
|
|||
|
|
data->invocation,
|
|||
|
|
&authorized);
|
|||
|
|
}
|
|||
|
|
if (authorized)
|
|||
|
|
{
|
|||
|
|
- g_signal_emit (data->interface,
|
|||
|
|
+ g_signal_emit (interface,
|
|||
|
|
signals[G_AUTHORIZE_METHOD_SIGNAL],
|
|||
|
|
0,
|
|||
|
|
data->invocation,
|
|||
|
|
@@ -627,7 +627,6 @@ g_dbus_interface_method_dispatch_helper (GDBusInterfaceSkeleton *interface
|
|||
|
|
DispatchData *data;
|
|||
|
|
|
|||
|
|
data = g_slice_new0 (DispatchData);
|
|||
|
|
- data->interface = interface;
|
|||
|
|
data->method_call_func = method_call_func;
|
|||
|
|
data->invocation = invocation;
|
|||
|
|
data->ref_count = 1;
|
|||
|
|
--
|
|||
|
|
GitLab
|
|||
|
|
|
|||
|
|
|
|||
|
|
From d5710deb9d621bcf0cec0ff2db0708f361490752 Mon Sep 17 00:00:00 2001
|
|||
|
|
From: Philip Withnall <pwithnall@endlessos.org>
|
|||
|
|
Date: Wed, 22 Feb 2023 12:47:36 +0000
|
|||
|
|
Subject: [PATCH 2/3] gdbusinterfaceskeleton: Fix a use-after-free of a
|
|||
|
|
GDBusMethodInvocation
|
|||
|
|
MIME-Version: 1.0
|
|||
|
|
Content-Type: text/plain; charset=UTF-8
|
|||
|
|
Content-Transfer-Encoding: 8bit
|
|||
|
|
|
|||
|
|
This `GDBusMethodInvocation` may be shared across threads, with no
|
|||
|
|
guarantee on the strong ref in one thread outlasting any refs in other
|
|||
|
|
threads — so it needs a ref in this helper struct.
|
|||
|
|
|
|||
|
|
This should fix a use-after-free where the `GDBusMethodInvocation` is
|
|||
|
|
freed from `g_value_unset()` after `g_signal_emit()` returns in
|
|||
|
|
`dispatch_in_thread_func()` in one thread; but then dereferenced again
|
|||
|
|
in `g_source_destroy_internal()` from another thread.
|
|||
|
|
|
|||
|
|
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
|
|||
|
|
|
|||
|
|
Fixes: #2924
|
|||
|
|
---
|
|||
|
|
gio/gdbusinterfaceskeleton.c | 9 ++++++---
|
|||
|
|
1 file changed, 6 insertions(+), 3 deletions(-)
|
|||
|
|
|
|||
|
|
diff --git a/gio/gdbusinterfaceskeleton.c b/gio/gdbusinterfaceskeleton.c
|
|||
|
|
index d28282fea3..a2a79fe3d8 100644
|
|||
|
|
--- a/gio/gdbusinterfaceskeleton.c
|
|||
|
|
+++ b/gio/gdbusinterfaceskeleton.c
|
|||
|
|
@@ -462,14 +462,17 @@ typedef struct
|
|||
|
|
{
|
|||
|
|
gint ref_count; /* (atomic) */
|
|||
|
|
GDBusInterfaceMethodCallFunc method_call_func;
|
|||
|
|
- GDBusMethodInvocation *invocation;
|
|||
|
|
+ GDBusMethodInvocation *invocation; /* (owned) */
|
|||
|
|
} DispatchData;
|
|||
|
|
|
|||
|
|
static void
|
|||
|
|
dispatch_data_unref (DispatchData *data)
|
|||
|
|
{
|
|||
|
|
if (g_atomic_int_dec_and_test (&data->ref_count))
|
|||
|
|
- g_slice_free (DispatchData, data);
|
|||
|
|
+ {
|
|||
|
|
+ g_clear_object (&data->invocation);
|
|||
|
|
+ g_slice_free (DispatchData, data);
|
|||
|
|
+ }
|
|||
|
|
}
|
|||
|
|
|
|||
|
|
static DispatchData *
|
|||
|
|
@@ -628,7 +631,7 @@ g_dbus_interface_method_dispatch_helper (GDBusInterfaceSkeleton *interface
|
|||
|
|
|
|||
|
|
data = g_slice_new0 (DispatchData);
|
|||
|
|
data->method_call_func = method_call_func;
|
|||
|
|
- data->invocation = invocation;
|
|||
|
|
+ data->invocation = g_object_ref (invocation);
|
|||
|
|
data->ref_count = 1;
|
|||
|
|
|
|||
|
|
task = g_task_new (interface, NULL, NULL, NULL);
|
|||
|
|
--
|
|||
|
|
GitLab
|
|||
|
|
|
|||
|
|
|
|||
|
|
From 7b101588e924f3783a0f5075f06b3e1d698be936 Mon Sep 17 00:00:00 2001
|
|||
|
|
From: Philip Withnall <pwithnall@endlessos.org>
|
|||
|
|
Date: Wed, 22 Feb 2023 12:50:10 +0000
|
|||
|
|
Subject: [PATCH 3/3] gdbusconnection: Make GDBusMethodInvocation transfer a
|
|||
|
|
bit clearer
|
|||
|
|
|
|||
|
|
Add a missing steal call in `schedule_method_call()`. This introduces no
|
|||
|
|
functional changes, but documents the ownership transfer more clearly.
|
|||
|
|
|
|||
|
|
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
|
|||
|
|
|
|||
|
|
Helps: #2924
|
|||
|
|
---
|
|||
|
|
gio/gdbusconnection.c | 2 +-
|
|||
|
|
1 file changed, 1 insertion(+), 1 deletion(-)
|
|||
|
|
|
|||
|
|
diff --git a/gio/gdbusconnection.c b/gio/gdbusconnection.c
|
|||
|
|
index d938f71b99..da6b66f2ec 100644
|
|||
|
|
--- a/gio/gdbusconnection.c
|
|||
|
|
+++ b/gio/gdbusconnection.c
|
|||
|
|
@@ -5043,7 +5043,7 @@ schedule_method_call (GDBusConnection *connection,
|
|||
|
|
g_source_set_priority (idle_source, G_PRIORITY_DEFAULT);
|
|||
|
|
g_source_set_callback (idle_source,
|
|||
|
|
call_in_idle_cb,
|
|||
|
|
- invocation,
|
|||
|
|
+ g_steal_pointer (&invocation),
|
|||
|
|
g_object_unref);
|
|||
|
|
g_source_set_static_name (idle_source, "[gio, " __FILE__ "] call_in_idle_cb");
|
|||
|
|
g_source_attach (idle_source, main_context);
|
|||
|
|
--
|
|||
|
|
GitLab
|
|||
|
|
|