Skip to content

Commit

Permalink
gfdonotificationbackend: Fix possible invalid pointer in dbus callback
Browse files Browse the repository at this point in the history
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:
<linuxmint/cinnamon#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>
  • Loading branch information
elboulangero authored and pwithnall committed Jun 13, 2018
1 parent 4a27a88 commit d26b66e
Showing 1 changed file with 34 additions and 21 deletions.
55 changes: 34 additions & 21 deletions gio/gfdonotificationbackend.c
Expand Up @@ -62,7 +62,6 @@ typedef struct
GVariant *default_action_target;
} FreedesktopNotification;


static void
freedesktop_notification_free (gpointer data)
{
Expand All @@ -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)
Expand Down Expand Up @@ -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
{
Expand All @@ -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);
}
}
Expand Down Expand Up @@ -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)
{
Expand All @@ -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);
}
Expand Down

0 comments on commit d26b66e

Please sign in to comment.