slackware-current/source/l/glib2/4073.patch

From df2c5d925ac4b8f1708bafa5ac1d35acada05d55 Mon Sep 17 00:00:00 2001
From: Philip Withnall <pwithnall@gnome.org>
Date: Wed, 15 May 2024 12:26:36 +0100
Subject: [PATCH 1/2] gmenuexporter: Fix a NULL pointer dereference on an error
 handling path
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This latent bug wasn’t triggered until commit 3f30ec86c (or its
cherry-pick onto `glib-2-80`, 747e3af99, which was first released in
2.80.1).

That change means that `g_menu_exporter_free()` is now called on the
registration failure path by `g_dbus_connection_register_object()`
before it returns. The caller then tries to call `g_slice_free()` on the
exporter again. The call to `g_menu_exporter_free()` tries to
dereference/free members of the exporter which it expects to be
initialised — but because this is happening in an error handling path,
they are not initialised.

If it were to get any further, the `g_slice_free()` would then be a
double-free on the exporter allocation.

Fix that by making `g_menu_exporter_free()` robust to some of the
exporter members being `NULL`, and moving some of the initialisation
code higher in `g_dbus_connection_export_menu_model()`, and removing the
duplicate free code on the error handling path.

This includes a unit test.

Signed-off-by: Philip Withnall <pwithnall@gnome.org>

Fixes: #3366
---
 gio/gmenuexporter.c    | 23 ++++++++---------------
 gio/tests/gmenumodel.c | 37 +++++++++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+), 15 deletions(-)

diff --git a/gio/gmenuexporter.c b/gio/gmenuexporter.c
index 909780cb2c..1d4db13523 100644
--- a/gio/gmenuexporter.c
+++ b/gio/gmenuexporter.c
@@ -707,11 +707,9 @@ g_menu_exporter_create_group (GMenuExporter *exporter)
 }
 
 static void
-g_menu_exporter_free (gpointer user_data)
+g_menu_exporter_free (GMenuExporter *exporter)
 {
-  GMenuExporter *exporter = user_data;
-
-  g_menu_exporter_menu_free (exporter->root);
+  g_clear_pointer (&exporter->root, g_menu_exporter_menu_free);
   g_clear_pointer (&exporter->peer_remote, g_menu_exporter_remote_free);
   g_hash_table_unref (exporter->remotes);
   g_hash_table_unref (exporter->groups);
@@ -794,21 +792,16 @@ g_dbus_connection_export_menu_model (GDBusConnection  *connection,
   guint id;
 
   exporter = g_slice_new0 (GMenuExporter);
-
-  id = g_dbus_connection_register_object (connection, object_path, org_gtk_Menus_get_interface (),
-                                          &vtable, exporter, g_menu_exporter_free, error);
-
-  if (id == 0)
-    {
-      g_slice_free (GMenuExporter, exporter);
-      return 0;
-    }
-
   exporter->connection = g_object_ref (connection);
   exporter->object_path = g_strdup (object_path);
   exporter->groups = g_hash_table_new (NULL, NULL);
   exporter->remotes = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, g_menu_exporter_remote_free);
-  exporter->root = g_menu_exporter_group_add_menu (g_menu_exporter_create_group (exporter), menu);
+
+  id = g_dbus_connection_register_object (connection, object_path, org_gtk_Menus_get_interface (),
+                                          &vtable, exporter, (GDestroyNotify) g_menu_exporter_free, error);
+
+  if (id != 0)
+    exporter->root = g_menu_exporter_group_add_menu (g_menu_exporter_create_group (exporter), menu);
 
   return id;
 }
diff --git a/gio/tests/gmenumodel.c b/gio/tests/gmenumodel.c
index d75f36a297..22d1b4d61e 100644
--- a/gio/tests/gmenumodel.c
+++ b/gio/tests/gmenumodel.c
@@ -1159,6 +1159,42 @@ test_dbus_peer_subscriptions (void)
 #endif
 }
 
+static void
+test_dbus_export_error_handling (void)
+{
+  GRand *rand = NULL;
+  RandomMenu *menu = NULL;
+  GDBusConnection *bus;
+  GError *local_error = NULL;
+  guint id1, id2;
+
+  g_test_summary ("Test that error handling of menu model export failure works");
+  g_test_bug ("https://gitlab.gnome.org/GNOME/glib/-/issues/3366");
+
+  bus = g_bus_get_sync (G_BUS_TYPE_SESSION, NULL, NULL);
+
+  rand = g_rand_new_with_seed (g_test_rand_int ());
+  menu = random_menu_new (rand, 2);
+
+  id1 = g_dbus_connection_export_menu_model (bus, "/", G_MENU_MODEL (menu), &local_error);
+  g_assert_no_error (local_error);
+  g_assert_cmpuint (id1, !=, 0);
+
+  /* Trigger a failure by trying to export on a path which is already in use */
+  id2 = g_dbus_connection_export_menu_model (bus, "/", G_MENU_MODEL (menu), &local_error);
+  g_assert_error (local_error, G_IO_ERROR, G_IO_ERROR_EXISTS);
+  g_assert_cmpuint (id2, ==, 0);
+  g_clear_error (&local_error);
+
+  g_dbus_connection_unexport_menu_model (bus, id1);
+
+  while (g_main_context_iteration (NULL, FALSE));
+
+  g_clear_object (&menu);
+  g_rand_free (rand);
+  g_clear_object (&bus);
+}
+
 static gpointer
 do_modify (gpointer data)
 {
@@ -1658,6 +1694,7 @@ main (int argc, char **argv)
   g_test_add_func ("/gmenu/dbus/threaded", test_dbus_threaded);
   g_test_add_func ("/gmenu/dbus/peer/roundtrip", test_dbus_peer_roundtrip);
   g_test_add_func ("/gmenu/dbus/peer/subscriptions", test_dbus_peer_subscriptions);
+  g_test_add_func ("/gmenu/dbus/export/error-handling", test_dbus_export_error_handling);
   g_test_add_func ("/gmenu/attributes", test_attributes);
   g_test_add_func ("/gmenu/attributes/iterate", test_attribute_iter);
   g_test_add_func ("/gmenu/links", test_links);
-- 
GitLab


From 7a7137838e79e5a98e6f4eab6898e2a0dc6392cd Mon Sep 17 00:00:00 2001
From: Philip Withnall <pwithnall@gnome.org>
Date: Wed, 15 May 2024 14:00:09 +0100
Subject: [PATCH 2/2] gactiongroupexporter: Fix memory problems on an error
 handling path

Almost identically to the previous commit, fix a similar latent bug in
`g_dbus_connection_export_action_group()`, which was not ready to handle
the fledgling `GActionGroupExporter` being freed early on an error
handling path.

See the previous commit message for details of the approach.

This includes a unit test.

Signed-off-by: Philip Withnall <pwithnall@gnome.org>

Fixes: #3366
---
 gio/gactiongroupexporter.c | 35 ++++++++++++++------------------
 gio/tests/actions.c        | 41 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 56 insertions(+), 20 deletions(-)

diff --git a/gio/gactiongroupexporter.c b/gio/gactiongroupexporter.c
index 3ec1db224e..1e253ec88b 100644
--- a/gio/gactiongroupexporter.c
+++ b/gio/gactiongroupexporter.c
@@ -531,10 +531,8 @@ org_gtk_Actions_method_call (GDBusConnection       *connection,
 }
 
 static void
-g_action_group_exporter_free (gpointer user_data)
+g_action_group_exporter_free (GActionGroupExporter *exporter)
 {
-  GActionGroupExporter *exporter = user_data;
-
   g_signal_handlers_disconnect_by_func (exporter->action_group,
                                         g_action_group_exporter_action_added, exporter);
   g_signal_handlers_disconnect_by_func (exporter->action_group,
@@ -616,15 +614,6 @@ g_dbus_connection_export_action_group (GDBusConnection  *connection,
     }
 
   exporter = g_slice_new (GActionGroupExporter);
-  id = g_dbus_connection_register_object (connection, object_path, org_gtk_Actions, &vtable,
-                                          exporter, g_action_group_exporter_free, error);
-
-  if (id == 0)
-    {
-      g_slice_free (GActionGroupExporter, exporter);
-      return 0;
-    }
-
   exporter->context = g_main_context_ref_thread_default ();
   exporter->pending_changes = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, NULL);
   exporter->pending_source = NULL;
@@ -632,14 +621,20 @@ g_dbus_connection_export_action_group (GDBusConnection  *connection,
   exporter->connection = g_object_ref (connection);
   exporter->object_path = g_strdup (object_path);
 
-  g_signal_connect (action_group, "action-added",
-                    G_CALLBACK (g_action_group_exporter_action_added), exporter);
-  g_signal_connect (action_group, "action-removed",
-                    G_CALLBACK (g_action_group_exporter_action_removed), exporter);
-  g_signal_connect (action_group, "action-state-changed",
-                    G_CALLBACK (g_action_group_exporter_action_state_changed), exporter);
-  g_signal_connect (action_group, "action-enabled-changed",
-                    G_CALLBACK (g_action_group_exporter_action_enabled_changed), exporter);
+  id = g_dbus_connection_register_object (connection, object_path, org_gtk_Actions, &vtable,
+                                          exporter, (GDestroyNotify) g_action_group_exporter_free, error);
+
+  if (id != 0)
+    {
+      g_signal_connect (action_group, "action-added",
+                        G_CALLBACK (g_action_group_exporter_action_added), exporter);
+      g_signal_connect (action_group, "action-removed",
+                        G_CALLBACK (g_action_group_exporter_action_removed), exporter);
+      g_signal_connect (action_group, "action-state-changed",
+                        G_CALLBACK (g_action_group_exporter_action_state_changed), exporter);
+      g_signal_connect (action_group, "action-enabled-changed",
+                        G_CALLBACK (g_action_group_exporter_action_enabled_changed), exporter);
+    }
 
   return id;
 }
diff --git a/gio/tests/actions.c b/gio/tests/actions.c
index a24c52c5e4..2b7a100fcf 100644
--- a/gio/tests/actions.c
+++ b/gio/tests/actions.c
@@ -1125,6 +1125,46 @@ test_dbus_export (void)
   session_bus_down ();
 }
 
+static void
+test_dbus_export_error_handling (void)
+{
+  GDBusConnection *bus = NULL;
+  GSimpleActionGroup *group = NULL;
+  GError *local_error = NULL;
+  guint id1, id2;
+
+  g_test_summary ("Test that error handling of action group export failure works");
+  g_test_bug ("https://gitlab.gnome.org/GNOME/glib/-/issues/3366");
+
+  session_bus_up ();
+  bus = g_bus_get_sync (G_BUS_TYPE_SESSION, NULL, NULL);
+
+  group = g_simple_action_group_new ();
+  g_simple_action_group_add_entries (group,
+                                     exported_entries,
+                                     G_N_ELEMENTS (exported_entries),
+                                     NULL);
+
+  id1 = g_dbus_connection_export_action_group (bus, "/", G_ACTION_GROUP (group), &local_error);
+  g_assert_no_error (local_error);
+  g_assert_cmpuint (id1, !=, 0);
+
+  /* Trigger a failure by trying to export on a path which is already in use */
+  id2 = g_dbus_connection_export_action_group (bus, "/", G_ACTION_GROUP (group), &local_error);
+  g_assert_error (local_error, G_IO_ERROR, G_IO_ERROR_EXISTS);
+  g_assert_cmpuint (id2, ==, 0);
+  g_clear_error (&local_error);
+
+  g_dbus_connection_unexport_action_group (bus, id1);
+
+  while (g_main_context_iteration (NULL, FALSE));
+
+  g_object_unref (group);
+  g_object_unref (bus);
+
+  session_bus_down ();
+}
+
 static gpointer
 do_export (gpointer data)
 {
@@ -1448,6 +1488,7 @@ main (int argc, char **argv)
   g_test_add_func ("/actions/entries", test_entries);
   g_test_add_func ("/actions/parse-detailed", test_parse_detailed);
   g_test_add_func ("/actions/dbus/export", test_dbus_export);
+  g_test_add_func ("/actions/dbus/export/error-handling", test_dbus_export_error_handling);
   g_test_add_func ("/actions/dbus/threaded", test_dbus_threaded);
   g_test_add_func ("/actions/dbus/bug679509", test_bug679509);
   g_test_add_func ("/actions/property", test_property_actions);
-- 
GitLab