[gnome-settings-daemon] rfkill: Fix crash due to racy GTask management

[Bastien Nocera <hadess src gnome org>]

commit aa92455b1c7b2b17a4d4617fab944d46748c7c2f
Author: Philip Withnall <withnall endlessm com>
Date:   Thu Feb 9 14:20:36 2017 +0000

    rfkill: Fix crash due to racy GTask management
    
    The change_all_timeout_id and task members of CcRfkillGlibPrivate are
    not necessarily both set and unset at the same time: it’s normal for
    (task != NULL) when (change_all_timeout_id == 0), when the code is doing
    its initial write before write_change_all_done_cb(); and when the code
    is doing its second write attempt before
    write_change_all_again_done_cb().
    
    However, the code was checking (change_all_timeout_id != 0) to see if a
    task was pending when cc_rfkill_glib_send_change_all_event(). If a
    previous task was still in the middle of a write, this would result in
    its GTask being overwritten with the new GTask. When the first task’s
    write completed, it would then clear the second GTask. When the second
    task’s write completed, it would try to access the second GTask, and
    find it was NULL, causing a crash.
    
    This can happen on systems where the rfkill subsystem is slow (and hence
    blocks on writes for tens or hundreds of milliseconds); for example, if
    rfkill passes through a GPIO.
    
    Fix this by:
     • checking whether (task != NULL) to see if a task is in flight; and
     • only clearing priv->task if it matches the callback’s task.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=778383

 plugins/rfkill/rfkill-glib.c |   37 ++++++++++++++++++++++++++-----------
 1 files changed, 26 insertions(+), 11 deletions(-)
---
diff --git a/plugins/rfkill/rfkill-glib.c b/plugins/rfkill/rfkill-glib.c
index 97f1935..0ef265f 100644
--- a/plugins/rfkill/rfkill-glib.c
+++ b/plugins/rfkill/rfkill-glib.c
@@ -52,7 +52,9 @@ struct _CcRfkillGlib {
        GIOChannel *channel;
        guint watch_id;
 
-       /* Pending Bluetooth enablement */
+       /* Pending Bluetooth enablement.
+        * If (@change_all_timeout_id != 0), then (task != NULL). The converse
+        * does not necessarily hold. */
        guint change_all_timeout_id;
        GTask *task;
 };
@@ -63,6 +65,20 @@ G_DEFINE_TYPE (CcRfkillGlib, cc_rfkill_glib, G_TYPE_OBJECT)
 
 static const char *type_to_string (unsigned int type);
 
+static void
+cancel_current_task (CcRfkillGlib *rfkill)
+{
+       if (rfkill->task != NULL) {
+               g_cancellable_cancel (g_task_get_cancellable (rfkill->task));
+               g_clear_object (&rfkill->task);
+       }
+
+       if (rfkill->change_all_timeout_id != 0) {
+               g_source_remove (rfkill->change_all_timeout_id);
+               rfkill->change_all_timeout_id = 0;
+       }
+}
+
 /* Note that this can return %FALSE without setting @error. */
 gboolean
 cc_rfkill_glib_send_change_all_event_finish (CcRfkillGlib        *rfkill,
@@ -94,7 +110,9 @@ write_change_all_again_done_cb (GObject      *source_object,
        else
                g_task_return_boolean (task, ret >= 0);
 
-       g_clear_object (&rfkill->task);
+       /* If this @task has been cancelled, it may have been superceded. */
+       if (rfkill->task == task)
+               g_clear_object (&rfkill->task);
 }
 
 static gboolean
@@ -142,6 +160,7 @@ write_change_all_done_cb (GObject      *source_object,
                goto bail;
        }
 
+       g_assert (rfkill->change_all_timeout_id == 0);
        rfkill->change_all_timeout_id = g_timeout_add (CHANGE_ALL_TIMEOUT,
                                                       (GSourceFunc) write_change_all_timeout_cb,
                                                       rfkill);
@@ -149,7 +168,9 @@ write_change_all_done_cb (GObject      *source_object,
        return;
 
 bail:
-       g_clear_object (&rfkill->task);
+       /* If this @task has been cancelled, it may have been superceded. */
+       if (rfkill->task == task)
+               g_clear_object (&rfkill->task);
 }
 
 void
@@ -170,12 +191,7 @@ cc_rfkill_glib_send_change_all_event (CcRfkillGlib        *rfkill,
        g_task_set_source_tag (task, cc_rfkill_glib_send_change_all_event);
 
        /* Clear any previous task. */
-       if (rfkill->change_all_timeout_id > 0) {
-               g_source_remove (rfkill->change_all_timeout_id);
-               rfkill->change_all_timeout_id = 0;
-               write_change_all_timeout_cb (rfkill);
-       }
-
+       cancel_current_task (rfkill);
        g_assert (rfkill->task == NULL);
 
        /* Start writing out a new event. */
@@ -422,8 +438,7 @@ cc_rfkill_glib_finalize (GObject *object)
 {
        CcRfkillGlib *rfkill = CC_RFKILL_GLIB (object);
 
-       if (rfkill->change_all_timeout_id > 0)
-               write_change_all_timeout_cb (rfkill);
+       cancel_current_task (rfkill);
 
        /* cleanup monitoring */
        if (rfkill->watch_id > 0) {