Attachment 51526 Details for Bug 41126 – Fix two crashes when dbus_g_proxy_new_for_peer is used on a bus

[patch] Fix two crashes when dbus_g_proxy_new_for_peer is used on a bus

0001-Fix-two-crashes-when-dbus_g_proxy_new_for_peer-is-us.patch (text/plain), 8.25 KB, created by Simon McVittie on 2011-09-22 11:16:26 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Simon McVittie

Created: 2011-09-22 11:16:26 UTC

Size: 8.25 KB

patch

obsolete

>From 6486495254ed3a02410cfde28954317350a43a92 Mon Sep 17 00:00:00 2001
>From: Simon McVittie <simon.mcvittie@collabora.co.uk>
>Date: Thu, 22 Sep 2011 19:15:48 +0100
>Subject: [PATCH] Fix two crashes when dbus_g_proxy_new_for_peer is used on a
> bus
>
>The first part of the bug is that when NameOwnerChanged is received with
>a dbus_g_proxy_new_for_peer (which has no name) alive, checking
>whether it was affected by the NameOwnerChanged caused a NULL
>dereference and segfault.
>
>The second part of the bug is that if the last proxy in existence is
>for a peer, when it was unregistered there would be no owner_match_rules,
>causing a crash.
>
>Both are exercised in the new test added here.
>
>Bug: https://bugs.freedesktop.org/show_bug.cgi?id=41126
>---
> .gitignore              |    1 +
> dbus/dbus-gproxy.c      |    5 +-
> test/core/Makefile.am   |    3 +
> test/core/peer-on-bus.c |  148 +++++++++++++++++++++++++++++++++++++++++++++++
> test/core/run-test.sh   |    1 +
> 5 files changed, 156 insertions(+), 2 deletions(-)
> create mode 100644 test/core/peer-on-bus.c
>
>diff --git a/.gitignore b/.gitignore
>index c568858..bfc16df 100644
>--- a/.gitignore
>+++ b/.gitignore
>@@ -198,6 +198,7 @@ test/core/run-with-tmp-session-bus.conf
> test/core/peer-client
> test/core/peer-server
> test/core/test-dbus-glib
>+/test/core/test-peer-on-bus
> test/core/test-profile
> test/core/test-proxy-peer
> test/core/test-service-glib
>diff --git a/dbus/dbus-gproxy.c b/dbus/dbus-gproxy.c
>index ee5d2bd..23ef040 100644
>--- a/dbus/dbus-gproxy.c
>+++ b/dbus/dbus-gproxy.c
>@@ -676,7 +676,7 @@ unassociate_proxies (gpointer key, gpointer val, gpointer user_data)
> 
>       manager = priv->manager;
> 
>-      if (!strcmp (priv->name, name))
>+      if (priv->name != NULL && !strcmp (priv->name, name))
> 	{
> 	  if (!priv->for_owner)
> 	    {
>@@ -1139,7 +1139,8 @@ dbus_g_proxy_manager_unregister (DBusGProxyManager *manager,
>       manager->proxy_lists = NULL;
>     }
> 
>-  if (g_hash_table_size (manager->owner_match_rules) == 0)
>+  if (manager->owner_match_rules != NULL &&
>+      g_hash_table_size (manager->owner_match_rules) == 0)
>     {
>       g_hash_table_destroy (manager->owner_match_rules);
>       manager->owner_match_rules = NULL;
>diff --git a/test/core/Makefile.am b/test/core/Makefile.am
>index ef6cb26..65d7731 100644
>--- a/test/core/Makefile.am
>+++ b/test/core/Makefile.am
>@@ -54,6 +54,7 @@ noinst_PROGRAMS = \
> 	peer-client \
> 	test-types \
> 	test-30574 \
>+	test-peer-on-bus \
> 	test-proxy-peer \
> 	test-registrations \
> 	test-variant-recursion \
>@@ -133,6 +134,8 @@ test_types_SOURCES = \
> test_gvariant_SOURCES = \
> 	test-gvariant.c
> 
>+test_peer_on_bus_SOURCES = peer-on-bus.c
>+
> CLEANFILES = \
> 	$(BUILT_SOURCES) \
> 	run-with-tmp-session-bus.conf
>diff --git a/test/core/peer-on-bus.c b/test/core/peer-on-bus.c
>new file mode 100644
>index 0000000..7c5147d
>--- /dev/null
>+++ b/test/core/peer-on-bus.c
>@@ -0,0 +1,148 @@
>+/* Regression test for object registration and unregistration
>+ *
>+ * Copyright Â© 2009 Collabora Ltd. <http://www.collabora.co.uk/>
>+ * Copyright Â© 2009-2011 Nokia Corporation
>+ *
>+ * In preparation for dbus-glib relicensing (if it ever happens), this file is
>+ * licensed under (at your option) either the AFL v2.1, the GPL v2 or later,
>+ * or an MIT/X11-style license:
>+ *
>+ * Licensed under the Academic Free License version 2.1
>+ *
>+ * This program is free software; you can redistribute it and/or modify
>+ * it under the terms of the GNU General Public License as published by
>+ * the Free Software Foundation; either version 2 of the License, or
>+ * (at your option) any later version.
>+ *
>+ * Permission is hereby granted, free of charge, to any person
>+ * obtaining a copy of this software and associated documentation
>+ * files (the "Software"), to deal in the Software without
>+ * restriction, including without limitation the rights to use, copy,
>+ * modify, merge, publish, distribute, sublicense, and/or sell copies
>+ * of the Software, and to permit persons to whom the Software is
>+ * furnished to do so, subject to the following conditions:
>+ *
>+ * The above copyright notice and this permission notice shall be
>+ * included in all copies or substantial portions of the Software.
>+ *
>+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
>+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
>+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
>+ * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
>+ * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
>+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
>+ * DEALINGS IN THE SOFTWARE.
>+ */
>+
>+#include <config.h>
>+
>+#include <dbus/dbus-glib.h>
>+#include <dbus/dbus-glib-lowlevel.h>
>+
>+#include "test/lib/util.h"
>+
>+GMainLoop *loop = NULL;
>+
>+typedef struct {
>+    DBusGConnection *bus;
>+    DBusGConnection *bus2;
>+} Fixture;
>+
>+static void
>+setup (Fixture *f,
>+    gconstpointer path_to_use)
>+{
>+  f->bus = dbus_g_bus_get_private (DBUS_BUS_SESSION, NULL, NULL);
>+  g_assert (f->bus != NULL);
>+
>+  f->bus2 = dbus_g_bus_get_private (DBUS_BUS_SESSION, NULL, NULL);
>+  g_assert (f->bus2 != NULL);
>+}
>+
>+static void
>+teardown (Fixture *f,
>+    gconstpointer test_data G_GNUC_UNUSED)
>+{
>+  if (f->bus != NULL)
>+    {
>+      test_run_until_disconnected (f->bus, NULL);
>+      dbus_g_connection_unref (f->bus);
>+    }
>+
>+  if (f->bus2 != NULL)
>+    {
>+      test_run_until_disconnected (f->bus2, NULL);
>+      dbus_g_connection_unref (f->bus2);
>+    }
>+
>+  dbus_shutdown ();
>+}
>+
>+static void
>+destroy_cb (DBusGProxy *proxy G_GNUC_UNUSED,
>+            gpointer user_data)
>+{
>+  gboolean *destroyed = user_data;
>+
>+  *destroyed = TRUE;
>+}
>+
>+static void
>+test_name_owner_changed (Fixture *f,
>+    gconstpointer test_data G_GNUC_UNUSED)
>+{
>+  DBusGProxy *peer;
>+  DBusGProxy *named;
>+  gboolean destroyed = FALSE;
>+
>+  g_test_bug ("41126");
>+
>+  /* bus has a proxy for bus2... */
>+  named = dbus_g_proxy_new_for_name (f->bus,
>+      dbus_bus_get_unique_name (dbus_g_connection_get_connection (f->bus2)),
>+      "/", "org.freedesktop.DBus.Peer");
>+  /* ... and also a proxy for the peer (i.e. the dbus-daemon) */
>+  peer = dbus_g_proxy_new_for_peer (f->bus, "/", "org.freedesktop.DBus.Peer");
>+
>+  g_signal_connect (G_OBJECT (named), "destroy", G_CALLBACK (destroy_cb),
>+                    &destroyed);
>+
>+  /* Disconnect bus2, to provoke a NameOwnerChanged signal on bus */
>+  test_run_until_disconnected (f->bus2, NULL);
>+  dbus_g_connection_unref (f->bus2);
>+  f->bus2 = NULL;
>+
>+  /* Wait for that NameOwnerChanged to be processed */
>+  while (!destroyed)
>+    g_main_context_iteration (NULL, TRUE);
>+
>+  g_signal_handlers_disconnect_by_func (named, destroy_cb, &destroyed);
>+
>+  /* The first part of the bug was that we'd never get here, because checking
>+   * whether 'peer' was affected by the NameOwnerChanged caused a NULL
>+   * dereference and segfault. If we get here, all is OK.
>+   *
>+   * The second part of the bug was that if the last proxy in existence was
>+   * for a peer, when it was unregistered there would be no owner_match_rules,
>+   * causing a crash. Unref named before peer, to exercise that. */
>+
>+  g_object_unref (named);
>+  g_object_unref (peer);
>+}
>+
>+int
>+main (int argc, char **argv)
>+{
>+  g_setenv ("DBUS_FATAL_WARNINGS", "1", TRUE);
>+  g_type_init ();
>+  g_log_set_always_fatal (G_LOG_LEVEL_WARNING | G_LOG_LEVEL_CRITICAL);
>+  dbus_g_type_specialized_init ();
>+  g_test_bug_base ("https://bugs.freedesktop.org/show_bug.cgi?id=");
>+  g_test_init (&argc, &argv, NULL);
>+
>+  g_test_add ("/peer-on-bus/name-owner-changed", Fixture, NULL,
>+      setup, test_name_owner_changed, teardown);
>+
>+  return g_test_run ();
>+}
>diff --git a/test/core/run-test.sh b/test/core/run-test.sh
>index 09e28b1..a61372b 100755
>--- a/test/core/run-test.sh
>+++ b/test/core/run-test.sh
>@@ -49,4 +49,5 @@ else
>   ${DBUS_TOP_BUILDDIR}/libtool --mode=execute $DEBUG $DBUS_TOP_BUILDDIR/test/core/test-variant-recursion || die "test-variant-recursion failed"
>   ${DBUS_TOP_BUILDDIR}/libtool --mode=execute $DEBUG $DBUS_TOP_BUILDDIR/test/core/test-gvariant || die "test-gvariant failed"
>   ${DBUS_TOP_BUILDDIR}/libtool --mode=execute $DEBUG $DBUS_TOP_BUILDDIR/test/core/test-30574 || die "test-30574 failed"
>+  ${DBUS_TOP_BUILDDIR}/libtool --mode=execute $DEBUG $DBUS_TOP_BUILDDIR/test/core/test-peer-on-bus || die "test-peer-on-bus failed"
> fi
>-- 
>1.7.6.3
>

Actions: View | Splinter Review

Attachments on bug 41126: 51526 | 51527 | 60057 | 60058

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.