From 023d50db40912e4f7ee333543e05995a9c730bae Mon Sep 17 00:00:00 2001

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

Date: Fri, 15 May 2020 21:44:16 +0900

Subject: [PATCH] Fix SEGV in bus_panel_proxy_focus_in()

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#1767976 SEGV in assert(connection != NULL) in

bus_dbus_impl_connection_filter_cb()

call bus_connection_set_filter() in bus_dbus_impl_destroy().

rhbz#1601577 rhbz#1797726 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.

rhbz#1771238 SEGV in assert(m_loop == null) in switcher.vala.

Grabbing keyboard could be failed and switcher received the keyboard

events and m_loop was not released.

rhbz#1797120 SEGV in assert(bus.is_connected()) in panel_binding_construct()

Check m_ibus in extension.vala:bus_name_acquired_cb()

BUG=rhbz#1350291

BUG=rhbz#1601577

BUG=rhbz#1663528

BUG=rhbz#1767691

BUG=rhbz#1795499

BUG=rhbz#1771238

BUG=rhbz#1767976

BUG=rhbz#1797120

---

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

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

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

 src/ibusbus.c          |  5 +++

 ui/gtk3/extension.vala |  4 +++

 ui/gtk3/switcher.vala  | 73 +++++++++++++++++++++++++-----------------

 6 files changed, 141 insertions(+), 47 deletions(-)

diff --git a/bus/dbusimpl.c b/bus/dbusimpl.c

index 59787a80..af2fbde2 100644

--- a/bus/dbusimpl.c

+++ b/bus/dbusimpl.c

@@ -610,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;

@@ -633,6 +634,10 @@ bus_dbus_impl_destroy (BusDBusImpl *dbus)

     for (p = dbus->connections; p != NULL; p = p->next) {

         BusConnection *connection = BUS_CONNECTION (p->data);

+        /* rhbz#1767976 Fix connection == NULL in

+         * bus_dbus_impl_connection_filter_cb()

+         */

+        bus_connection_set_filter (connection, NULL, NULL, NULL);

         g_signal_handlers_disconnect_by_func (connection,

                 bus_dbus_impl_connection_destroy_cb, dbus);

         ibus_object_destroy (IBUS_OBJECT (connection));

@@ -647,12 +652,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);

@@ -1483,13 +1515,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..bbbe5532 100644

--- a/bus/engineproxy.c

+++ b/bus/engineproxy.c

@@ -660,20 +660,33 @@ bus_engine_proxy_g_signal (GDBusProxy  *proxy,

     g_return_if_reached ();

 }

+#pragma GCC optimize ("O0")

 static BusEngineProxy *

 bus_engine_proxy_new_internal (const gchar     *path,

                                IBusEngineDesc  *desc,

-                               GDBusConnection *connection)

+                               GDBusConnection *connection,

+                               GError         **error)

 {

+    GDBusProxyFlags flags;

+    BusEngineProxy *engine;

+

     g_assert (path);

     g_assert (IBUS_IS_ENGINE_DESC (desc));

     g_assert (G_IS_DBUS_CONNECTION (connection));

+    g_assert (error && *error == NULL);

-    GDBusProxyFlags flags = G_DBUS_PROXY_FLAGS_DO_NOT_AUTO_START;

-    BusEngineProxy *engine =

+    /* rhbz#1601577 engine == NULL if connection is closed. */

+    if (g_dbus_connection_is_closed (connection)) {

+        *error = g_error_new (G_DBUS_ERROR,

+                              G_DBUS_ERROR_FAILED,

+                              "Connection is closed.");

+        return NULL;

+    }

+    flags = G_DBUS_PROXY_FLAGS_DO_NOT_AUTO_START;

+    engine =

         (BusEngineProxy *) g_initable_new (BUS_TYPE_ENGINE_PROXY,

                                            NULL,

-                                           NULL,

+                                           error,

                                            "desc",              desc,

                                            "g-connection",      connection,

                                            "g-interface-name",  IBUS_INTERFACE_ENGINE,

@@ -681,12 +694,19 @@ bus_engine_proxy_new_internal (const gchar     *path,

                                            "g-default-timeout", g_gdbus_timeout,

                                            "g-flags",           flags,

                                            NULL);

+    /* FIXME: rhbz#1601577 */

+    if (!engine) {

+        /* 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);

     }

     return engine;

 }

+#pragma GCC reset_options

 typedef struct {

     GTask           *task;

@@ -748,23 +768,30 @@ create_engine_ready_cb (BusFactoryProxy    *factory,

                         GAsyncResult       *res,

                         EngineProxyNewData *data)

 {

+    GError *error = NULL;

+    gchar *path;

+    BusEngineProxy *engine;

+

     g_return_if_fail (data->task != NULL);

-    GError *error = NULL;

-    gchar *path = bus_factory_proxy_create_engine_finish (factory,

-                                                          res,

-                                                          &error);

+    path = bus_factory_proxy_create_engine_finish (factory, res, &error);

     if (path == NULL) {

         g_task_return_error (data->task, error);

         engine_proxy_new_data_free (data);

         return;

     }

-    BusEngineProxy *engine =

-            bus_engine_proxy_new_internal (path,

-                                           data->desc,

-                                           g_dbus_proxy_get_connection ((GDBusProxy *)data->factory));

+    engine = bus_engine_proxy_new_internal (

+            path,

+            data->desc,

+            g_dbus_proxy_get_connection ((GDBusProxy *)data->factory),

+            &error);

     g_free (path);

+    if (!engine) {

+        g_task_return_error (data->task, error);

+        engine_proxy_new_data_free (data);

+        return;

+    }

     /* FIXME: set destroy callback ? */

     g_task_return_pointer (data->task, engine, NULL);

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;

     }

diff --git a/ui/gtk3/extension.vala b/ui/gtk3/extension.vala

index ea3cd464..57ed1357 100644

--- a/ui/gtk3/extension.vala

+++ b/ui/gtk3/extension.vala

@@ -73,6 +73,10 @@ class ExtensionGtk : Gtk.Application {

                                       string signal_name,

                                       Variant parameters) {

         debug("signal_name = %s", signal_name);

+        /* rhbz#1797120 Fix assert(bus.is_connected()) in

+         * panel_binding_construct()

+         */

+        return_if_fail(m_bus.is_connected());

         m_panel = new PanelBinding(m_bus, this);

         m_panel.load_settings();

     }

diff --git a/ui/gtk3/switcher.vala b/ui/gtk3/switcher.vala

index a4529c88..29a70dd5 100644

--- a/ui/gtk3/switcher.vala

+++ b/ui/gtk3/switcher.vala

@@ -140,8 +140,8 @@ class Switcher : Gtk.Window {

                    IBus.EngineDesc[] engines,

                    int               index,

                    string            input_context_path) {

-        assert (m_loop == null);

-        assert (index < engines.length);

+        assert(m_loop == null);

+        assert(index < engines.length);

         m_is_running = true;

         m_keyval = keyval;

@@ -198,16 +198,18 @@ class Switcher : Gtk.Window {

                            null,

                            event,

                            null);

-        if (status != Gdk.GrabStatus.SUCCESS)

+        if (status != Gdk.GrabStatus.SUCCESS) {

             warning("Grab keyboard failed! status = %d", status);

-        status = seat.grab(get_window(),

-                           Gdk.SeatCapabilities.POINTER,

-                           true,

-                           null,

-                           event,

-                           null);

-        if (status != Gdk.GrabStatus.SUCCESS)

-            warning("Grab pointer failed! status = %d", status);

+        } else {

+            status = seat.grab(get_window(),

+                               Gdk.SeatCapabilities.POINTER,

+                               true,

+                               null,

+                               event,

+                               null);

+            if (status != Gdk.GrabStatus.SUCCESS)

+                warning("Grab pointer failed! status = %d", status);

+        }

 #else

         Gdk.Device device = event.get_device();

         if (device == null) {

@@ -243,30 +245,41 @@ class Switcher : Gtk.Window {

                                Gdk.EventMask.KEY_RELEASE_MASK,

                                null,

                                Gdk.CURRENT_TIME);

-        if (status != Gdk.GrabStatus.SUCCESS)

+        if (status != Gdk.GrabStatus.SUCCESS) {

             warning("Grab keyboard failed! status = %d", status);

-        // Grab all pointer events

-        status = pointer.grab(get_window(),

-                              Gdk.GrabOwnership.NONE,

-                              true,

-                              Gdk.EventMask.BUTTON_PRESS_MASK |

-                              Gdk.EventMask.BUTTON_RELEASE_MASK,

-                              null,

-                              Gdk.CURRENT_TIME);

-        if (status != Gdk.GrabStatus.SUCCESS)

-            warning("Grab pointer failed! status = %d", status);

+        } else {

+            // Grab all pointer events

+            status = pointer.grab(get_window(),

+                                  Gdk.GrabOwnership.NONE,

+                                  true,

+                                  Gdk.EventMask.BUTTON_PRESS_MASK |

+                                  Gdk.EventMask.BUTTON_RELEASE_MASK,

+                                  null,

+                                  Gdk.CURRENT_TIME);

+            if (status != Gdk.GrabStatus.SUCCESS)

+                warning("Grab pointer failed! status = %d", status);

+        }

 #endif

-        // Probably we can delete m_popup_delay_time in 1.6

-        pointer.get_position_double(null,

-                                    out m_mouse_init_x,

-                                    out m_mouse_init_y);

-        m_mouse_moved = false;

+        /* Fix RHBZ #1771238 assert(m_loop == null)

+         * Grabbing keyboard can be failed when the second Super-e is typed

+         * before Switcher dialog is focused. And m_loop could not be released

+         * if the failed Super-e would call m_loop.run() below and could not

+         * call key_release_event(). And m_loop == null would be false in the

+         * third Super-e.

+         */

+        if (status == Gdk.GrabStatus.SUCCESS) {

+            // Probably we can delete m_popup_delay_time in 1.6

+            pointer.get_position_double(null,

+                                        out m_mouse_init_x,

+                                        out m_mouse_init_y);

+            m_mouse_moved = false;

-        m_loop = new GLib.MainLoop();

-        m_loop.run();

-        m_loop = null;

+            m_loop = new GLib.MainLoop();

+            m_loop.run();

+            m_loop = null;

+        }

 #if VALA_0_34

         seat.ungrab();

-- 

2.24.1