[glib: 1/2] gfdonotificationbackend: Fix possible invalid pointer in dbus callback



commit 57e070f874bc8947fce060d294646c19ac26560a
Author: Arnaud Rebillout <elboulangero gmail com>
Date:   Sun Jun 10 20:56:12 2018 +0700

    gfdonotificationbackend: Fix possible invalid pointer in dbus callback
    
    The way things were before: a FreedesktopNotification struct is
    allocated before the dbus call, and this same struct is possibly re-used
    for other dbus calls. If the server becomes unavailable, the callback
    will be invoked after the call times out, which leaves a long time where
    other dbus calls can happen, re-using the same FreedesktopNotification
    as user data. When the first call times out, the callback is invoked,
    and the user data is freed. Subsequent calls that used the same user
    data will time out later on, and try to free a pointer that was already
    freed, hence segfaults.
    
    This bug can be reproduced in Cinnamon 3.6.7, as mentioned in:
    <https://github.com/linuxmint/Cinnamon/issues/7491>
    
    This commit fixes that by always allocating a new
    FreedesktopNotification before invoking dbus_call(), ensuring that the
    callback always have a valid user data.
    
    Signed-off-by: Arnaud Rebillout <elboulangero gmail com>

 gio/gfdonotificationbackend.c | 55 ++++++++++++++++++++++++++-----------------
 1 file changed, 34 insertions(+), 21 deletions(-)
---
diff --git a/gio/gfdonotificationbackend.c b/gio/gfdonotificationbackend.c
index a0d481433..ab5329497 100644
--- a/gio/gfdonotificationbackend.c
+++ b/gio/gfdonotificationbackend.c
@@ -62,7 +62,6 @@ typedef struct
   GVariant *default_action_target;
 } FreedesktopNotification;
 
-
 static void
 freedesktop_notification_free (gpointer data)
 {
@@ -76,6 +75,24 @@ freedesktop_notification_free (gpointer data)
   g_slice_free (FreedesktopNotification, n);
 }
 
+static FreedesktopNotification *
+freedesktop_notification_new (GFdoNotificationBackend *backend,
+                              const gchar             *id,
+                              GNotification           *notification)
+{
+  FreedesktopNotification *n;
+
+  n = g_slice_new0 (FreedesktopNotification);
+  n->backend = backend;
+  n->id = g_strdup (id);
+  n->notify_id = 0;
+  g_notification_get_default_action (notification,
+                                     &n->default_action,
+                                     &n->default_action_target);
+
+  return n;
+}
+
 static FreedesktopNotification *
 g_fdo_notification_backend_find_notification (GFdoNotificationBackend *backend,
                                               const gchar             *id)
@@ -319,8 +336,19 @@ notification_sent (GObject      *source_object,
   val = g_dbus_connection_call_finish (G_DBUS_CONNECTION (source_object), result, &error);
   if (val)
     {
+      GFdoNotificationBackend *backend = n->backend;
+      FreedesktopNotification *match;
+
       g_variant_get (val, "(u)", &n->notify_id);
       g_variant_unref (val);
+
+      match = g_fdo_notification_backend_find_notification_by_notify_id (backend, n->notify_id);
+      if (match != NULL)
+        {
+          backend->notifications = g_slist_remove (backend->notifications, match);
+          freedesktop_notification_free (match);
+        }
+      backend->notifications = g_slist_prepend (backend->notifications, n);
     }
   else
     {
@@ -331,9 +359,7 @@ notification_sent (GObject      *source_object,
           warning_printed = TRUE;
         }
 
-      n->backend->notifications = g_slist_remove (n->backend->notifications, n);
       freedesktop_notification_free (n);
-
       g_error_free (error);
     }
 }
@@ -378,7 +404,7 @@ g_fdo_notification_backend_send_notification (GNotificationBackend *backend,
                                               GNotification        *notification)
 {
   GFdoNotificationBackend *self = G_FDO_NOTIFICATION_BACKEND (backend);
-  FreedesktopNotification *n;
+  FreedesktopNotification *n, *tmp;
 
   if (self->notify_subscription == 0)
     {
@@ -391,24 +417,11 @@ g_fdo_notification_backend_send_notification (GNotificationBackend *backend,
                                             notify_signal, backend, NULL);
     }
 
-  n = g_fdo_notification_backend_find_notification (self, id);
-  if (n == NULL)
-    {
-      n = g_slice_new0 (FreedesktopNotification);
-      n->backend = self;
-      n->id = g_strdup (id);
-      n->notify_id = 0;
-
-      n->backend->notifications = g_slist_prepend (n->backend->notifications, n);
-    }
-  else
-    {
-      /* Only clear default action. All other fields are still valid */
-      g_clear_pointer (&n->default_action, g_free);
-      g_clear_pointer (&n->default_action_target, g_variant_unref);
-    }
+  n = freedesktop_notification_new (self, id, notification);
 
-  g_notification_get_default_action (notification, &n->default_action, &n->default_action_target);
+  tmp = g_fdo_notification_backend_find_notification (self, id);
+  if (tmp)
+    n->notify_id = tmp->notify_id;
 
   call_notify (backend->dbus_connection, backend->application, n->notify_id, notification, 
notification_sent, n);
 }


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]