From fd19aaaa097e71e7589bdb3627971dc10bf3873e Mon Sep 17 00:00:00 2001

From: fujiwarat <takao.fujiwara1@gmail.com>

Date: Thu, 13 Feb 2020 19:33:32 +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.

rhbz#1767691 SEGV in client/x11/main.c:_sighandler().

Do not call atexit functions in _sighandler().

rhbz#1795499 SEGV in ibus_bus_get_bus_address() because of no _bus->priv.

_changed_cb() should not be called after ibus_bus_destroy() is called.

BUG=rhbz#1349148

BUG=rhbz#1385349

BUG=rhbz#1350291

BUG=rhbz#1406699

BUG=rhbz#1432252

BUG=rhbz#1601577

BUG=rhbz#1663528

BUG=rhbz#1767691

BUG=rhbz#1795499

---

 bus/dbusimpl.c    | 70 +++++++++++++++++++++++++++++++++++++++++------

 bus/engineproxy.c |  9 +++++-

 bus/ibusimpl.c    | 21 ++++++++++++--

 client/x11/main.c |  8 +++++-

 src/ibusbus.c     |  5 ++++

 5 files changed, 100 insertions(+), 13 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 85761d30..f0dbccd1 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);

 }

diff --git a/client/x11/main.c b/client/x11/main.c

index c9ee174d..768b91f0 100644

--- a/client/x11/main.c

+++ b/client/x11/main.c

@@ -40,6 +40,7 @@

 #include <iconv.h>

 #include <signal.h>

 #include <stdlib.h>

+#include <unistd.h>

 #include <getopt.h>

@@ -1104,7 +1105,12 @@ _atexit_cb ()

 static void

 _sighandler (int sig)

 {

-    exit(EXIT_FAILURE);

+    /* rhbz#1767691 _sighandler() is called with SIGTERM

+     * and exit() causes SEGV during calling atexit functions.

+     * _atexit_cb() might be broken. _exit() does not call

+     * atexit functions.

+     */

+    _exit(EXIT_FAILURE);

 }

 static void

diff --git a/src/ibusbus.c b/src/ibusbus.c

index b7ffbb47..668c8a26 100644

--- a/src/ibusbus.c

+++ b/src/ibusbus.c

@@ -689,6 +689,11 @@ ibus_bus_destroy (IBusObject *object)

     _bus = NULL;

     if (bus->priv->monitor) {

+        /* rhbz#1795499 _changed_cb() causes SEGV because of no bus->priv

+         * after ibus_bus_destroy() is called.

+         */

+        g_signal_handlers_disconnect_by_func (bus->priv->monitor,

+                                              (GCallback) _changed_cb, bus);

         g_object_unref (bus->priv->monitor);

         bus->priv->monitor = NULL;

     }

-- 

2.24.1