Blob Blame Raw
From 018a0f889d18c41e314f0b1297d1dc559603142b Mon Sep 17 00:00:00 2001
From: fujiwarat <takao.fujiwara1@gmail.com>
Date: Tue, 5 Feb 2019 18:36:04 +0900
Subject: [PATCH] Fix SEGV in bus_panel_proxy_focus_in()

rhbz#1349148, rhbz#1385349
SEGV in BUS_IS_PANEL_PROXY() in bus_panel_proxy_focus_in()
Check if GDBusConnect is closed before bus_panel_proxy_new() is called.

rhbz#1350291 SEGV in BUS_IS_CONNECTION(skip_connection) in
bus_dbus_impl_dispatch_message_by_rule()
check if dbus_connection is closed in bus_dbus_impl_connection_filter_cb().

rhbz#1406699 SEGV in new_owner!=NULL in bus_dbus_impl_name_owner_changed()
which is called by bus_name_service_remove_owner()
If bus_connection_get_unique_name()==NULL, set new_owner="" in
bus_name_service_remove_owner()

rhbz#1432252 SEGV in old_owner!=NULL in bus_dbus_impl_name_owner_changed()
which is called by bus_name_service_set_primary_owner()
If bus_connection_get_unique_name()==NULL, set old_owner="" in
bus_name_service_set_primary_owner()

rhbz#1601577 SEGV in ibus_engine_desc_get_layout() in
bus_engine_proxy_new_internal()
WIP: Added a GError to get the error message to check why the SEGV happened.

rhbz#1663528 SEGV in g_mutex_clear() in bus_dbus_impl_destroy()
If the mutex is not unlocked, g_mutex_clear() causes assert.

BUG=rhbz#1349148
BUG=rhbz#1385349
BUG=rhbz#1350291
BUG=rhbz#1406699
BUG=rhbz#1432252
BUG=rhbz#1601577
BUG=rhbz#1663528
---
 bus/dbusimpl.c    | 70 +++++++++++++++++++++++++++++++++++++++++------
 bus/engineproxy.c |  9 +++++-
 bus/ibusimpl.c    | 21 ++++++++++++--
 3 files changed, 88 insertions(+), 12 deletions(-)

diff --git a/bus/dbusimpl.c b/bus/dbusimpl.c
index b54ef817..fb38faf0 100644
--- a/bus/dbusimpl.c
+++ b/bus/dbusimpl.c
@@ -2,7 +2,8 @@
 /* vim:set et sts=4: */
 /* ibus - The Input Bus
  * Copyright (C) 2008-2013 Peng Huang <shawn.p.huang@gmail.com>
- * Copyright (C) 2008-2013 Red Hat, Inc.
+ * Copyright (C) 2015-2019 Takao Fujiwara <takao.fujiwara1@gmail.com>
+ * Copyright (C) 2008-2019 Red Hat, Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -344,6 +345,8 @@ bus_name_service_set_primary_owner (BusNameService     *service,
                                     BusConnectionOwner *owner,
                                     BusDBusImpl        *dbus)
 {
+    gboolean has_old_owner = FALSE;
+
     g_assert (service != NULL);
     g_assert (owner != NULL);
     g_assert (dbus != NULL);
@@ -351,6 +354,13 @@ bus_name_service_set_primary_owner (BusNameService     *service,
     BusConnectionOwner *old = service->owners != NULL ?
             (BusConnectionOwner *)service->owners->data : NULL;
 
+    /* rhbz#1432252 If bus_connection_get_unique_name() == NULL,
+     * "Hello" method is not received yet.
+     */
+    if (old != NULL && bus_connection_get_unique_name (old->conn) != NULL) {
+        has_old_owner = TRUE;
+    }
+
     if (old != NULL) {
         g_signal_emit (dbus,
                        dbus_signals[NAME_LOST],
@@ -370,7 +380,8 @@ bus_name_service_set_primary_owner (BusNameService     *service,
                    0,
                    owner->conn,
                    service->name,
-                   old != NULL ? bus_connection_get_unique_name (old->conn) : "",
+                   has_old_owner ? bus_connection_get_unique_name (old->conn) :
+                           "",
                    bus_connection_get_unique_name (owner->conn));
 
     if (old != NULL && old->do_not_queue != 0) {
@@ -427,6 +438,7 @@ bus_name_service_remove_owner (BusNameService     *service,
                                BusDBusImpl        *dbus)
 {
     GSList *owners;
+    gboolean has_new_owner = FALSE;
 
     g_assert (service != NULL);
     g_assert (owner != NULL);
@@ -439,6 +451,13 @@ bus_name_service_remove_owner (BusNameService     *service,
         BusConnectionOwner *_new = NULL;
         if (owners->next != NULL) {
             _new = (BusConnectionOwner *)owners->next->data;
+            /* rhbz#1406699 If bus_connection_get_unique_name() == NULL,
+             * "Hello" method is not received yet.
+             */
+            if (_new != NULL &&
+                bus_connection_get_unique_name (_new->conn) != NULL) {
+                has_new_owner = TRUE;
+            }
         }
 
         if (dbus != NULL) {
@@ -447,7 +466,7 @@ bus_name_service_remove_owner (BusNameService     *service,
                            0,
                            owner->conn,
                            service->name);
-            if (_new != NULL) {
+            if (has_new_owner) {
                 g_signal_emit (dbus,
                                dbus_signals[NAME_ACQUIRED],
                                0,
@@ -460,7 +479,7 @@ bus_name_service_remove_owner (BusNameService     *service,
                     _new != NULL ? _new->conn : NULL,
                     service->name,
                     bus_connection_get_unique_name (owner->conn),
-                    _new != NULL ? bus_connection_get_unique_name (_new->conn) : "");
+                    has_new_owner ? bus_connection_get_unique_name (_new->conn) : "");
 
         }
     }
@@ -591,6 +610,7 @@ static void
 bus_dbus_impl_destroy (BusDBusImpl *dbus)
 {
     GList *p;
+    int i;
 
     for (p = dbus->objects; p != NULL; p = p->next) {
         IBusService *object = (IBusService *) p->data;
@@ -628,12 +648,39 @@ bus_dbus_impl_destroy (BusDBusImpl *dbus)
     dbus->unique_names = NULL;
     dbus->names = NULL;
 
+    for (i = 0; g_idle_remove_by_data (dbus); i++) {
+        if (i > 1000) {
+            g_warning ("Too many idle threads were generated by " \
+                       "bus_dbus_impl_forward_message_idle_cb and " \
+                       "bus_dbus_impl_dispatch_message_by_rule_idle_cb");
+            break;
+        }
+    }
     g_list_free_full (dbus->start_service_calls,
                       (GDestroyNotify) bus_method_call_free);
     dbus->start_service_calls = NULL;
 
-    g_mutex_clear (&dbus->dispatch_lock);
-    g_mutex_clear (&dbus->forward_lock);
+   /* rhbz#1663528 Call g_mutex_trylock() before g_mutex_clear()
+    * because if the mutex is not unlocked, g_mutex_clear() causes assert.
+    */
+#define BUS_DBUS_MUTEX_SAFE_CLEAR(mtex) {                               \
+    int count = 0;                                                      \
+    while (!g_mutex_trylock ((mtex))) {                                 \
+        g_usleep (1);                                                   \
+        if (count > 60) {                                               \
+            g_warning (#mtex " is dead lock");                          \
+            break;                                                      \
+        }                                                               \
+        ++count;                                                        \
+    }                                                                   \
+    g_mutex_unlock ((mtex));                                            \
+    g_mutex_clear ((mtex));                                             \
+}
+
+    BUS_DBUS_MUTEX_SAFE_CLEAR (&dbus->dispatch_lock);
+    BUS_DBUS_MUTEX_SAFE_CLEAR (&dbus->forward_lock);
+
+#undef BUS_DBUS_MUTEX_SAFE_CLEAR
 
     /* FIXME destruct _lock and _queue members. */
     IBUS_OBJECT_CLASS(bus_dbus_impl_parent_class)->destroy ((IBusObject *) dbus);
@@ -1464,13 +1511,20 @@ bus_dbus_impl_connection_filter_cb (GDBusConnection *dbus_connection,
                                     gboolean         incoming,
                                     gpointer         user_data)
 {
+    BusDBusImpl *dbus;
+    BusConnection *connection;
+
     g_assert (G_IS_DBUS_CONNECTION (dbus_connection));
     g_assert (G_IS_DBUS_MESSAGE (message));
     g_assert (BUS_IS_DBUS_IMPL (user_data));
 
-    BusDBusImpl *dbus = (BusDBusImpl *) user_data;
-    BusConnection *connection = bus_connection_lookup (dbus_connection);
+    if (g_dbus_connection_is_closed (dbus_connection))
+        return NULL;
+
+    dbus = (BusDBusImpl *) user_data;
+    connection = bus_connection_lookup (dbus_connection);
     g_assert (connection != NULL);
+    g_assert (BUS_IS_CONNECTION (connection));
 
     if (incoming) {
         /* is incoming message */
diff --git a/bus/engineproxy.c b/bus/engineproxy.c
index 2d98995c..2176e0c9 100644
--- a/bus/engineproxy.c
+++ b/bus/engineproxy.c
@@ -665,6 +665,7 @@ bus_engine_proxy_new_internal (const gchar     *path,
                                IBusEngineDesc  *desc,
                                GDBusConnection *connection)
 {
+    GError *error = NULL;
     g_assert (path);
     g_assert (IBUS_IS_ENGINE_DESC (desc));
     g_assert (G_IS_DBUS_CONNECTION (connection));
@@ -673,7 +674,7 @@ bus_engine_proxy_new_internal (const gchar     *path,
     BusEngineProxy *engine =
         (BusEngineProxy *) g_initable_new (BUS_TYPE_ENGINE_PROXY,
                                            NULL,
-                                           NULL,
+                                           &error,
                                            "desc",              desc,
                                            "g-connection",      connection,
                                            "g-interface-name",  IBUS_INTERFACE_ENGINE,
@@ -681,6 +682,12 @@ bus_engine_proxy_new_internal (const gchar     *path,
                                            "g-default-timeout", g_gdbus_timeout,
                                            "g-flags",           flags,
                                            NULL);
+    /* FIXME: rhbz#1601577 */
+    if (error) {
+        /* show abrt local variable */
+        gchar *message = g_strdup (error->message);
+        g_error ("%s", message);
+    }
     const gchar *layout = ibus_engine_desc_get_layout (desc);
     if (layout != NULL && layout[0] != '\0') {
         engine->keymap = ibus_keymap_get (layout);
diff --git a/bus/ibusimpl.c b/bus/ibusimpl.c
index bbbb5770..77fcf42f 100644
--- a/bus/ibusimpl.c
+++ b/bus/ibusimpl.c
@@ -464,13 +464,16 @@ _dbus_name_owner_changed_cb (BusDBusImpl   *dbus,
     else if (!g_strcmp0 (name, IBUS_SERVICE_PANEL_EXTENSION_EMOJI))
         panel_type = PANEL_TYPE_EXTENSION_EMOJI;
 
-    if (panel_type != PANEL_TYPE_NONE) {
+    do {
+        if (panel_type == PANEL_TYPE_NONE)
+            break;
         if (g_strcmp0 (new_name, "") != 0) {
             /* a Panel process is started. */
             BusConnection *connection;
             BusInputContext *context = NULL;
             BusPanelProxy   **panel = (panel_type == PANEL_TYPE_PANEL) ?
                                       &ibus->panel : &ibus->emoji_extension;
+            GDBusConnection *dbus_connection = NULL;
 
             if (*panel != NULL) {
                 ibus_proxy_destroy ((IBusProxy *)(*panel));
@@ -479,9 +482,21 @@ _dbus_name_owner_changed_cb (BusDBusImpl   *dbus,
                 g_assert (*panel == NULL);
             }
 
-            connection = bus_dbus_impl_get_connection_by_name (BUS_DEFAULT_DBUS, new_name);
+            connection = bus_dbus_impl_get_connection_by_name (BUS_DEFAULT_DBUS,
+                                                               new_name);
             g_return_if_fail (connection != NULL);
 
+            dbus_connection = bus_connection_get_dbus_connection (connection);
+            /* rhbz#1349148 rhbz#1385349
+             * Avoid SEGV of BUS_IS_PANEL_PROXY (ibus->panel)
+             * This function is called during destroying the connection
+             * in this case? */
+            if (dbus_connection == NULL ||
+                g_dbus_connection_is_closed (dbus_connection)) {
+                new_name = "";
+                break;
+            }
+
             *panel = bus_panel_proxy_new (connection, panel_type);
             if (panel_type == PANEL_TYPE_EXTENSION_EMOJI)
                 ibus->enable_emoji_extension = FALSE;
@@ -535,7 +550,7 @@ _dbus_name_owner_changed_cb (BusDBusImpl   *dbus,
                 }
             }
         }
-    }
+    } while (0);
 
     bus_ibus_impl_component_name_owner_changed (ibus, name, old_name, new_name);
 }
-- 
2.20.1