udisks2: Prevent race between unmount reply and retry timer

Currently it is possible for the unmount op reply and the retry unmount
timer to race. A udisks2 unmount operation (or umount spawned command)
is started via the timer. In the meantime, a "cancel" or "force unmount"
reply is received which completes the gvfs unmount operation and frees
the private data. When the udisks2 unmount operation started by the
timer completes, it tries to use the freed data and segfaults.

To fix this, prevent starting an unmount operation when another is
already in progress.  If a timer callback is received while an unmount
operation is in progress, simply ignore it.  If an unmount op reply is
received while an unmount operation is in progress, store the result of
the reply and handle it once the unmount operation has completed.

https://bugzilla.gnome.org/show_bug.cgi?id=678555

1 files changed, 50 insertions, 16 deletions

diff --git a/monitor/udisks2/gvfsudisks2mount.c b/monitor/udisks2/gvfsudisks2mount.c
index dd00db48..8607490d 100644
--- a/monitor/udisks2/gvfsudisks2mount.c
+++ b/monitor/udisks2/gvfsudisks2mount.c
@@ -527,6 +527,7 @@ typedef struct
 {
   volatile gint ref_count;
   GSimpleAsyncResult *simple;
+  gboolean in_progress;
   gboolean completed;
 
   GVfsUDisks2Mount *mount;
@@ -541,6 +542,10 @@ typedef struct
 
   gulong mount_op_reply_handler_id;
   guint retry_unmount_timer_id;
+
+  GMountOperationResult reply_result;
+  gint reply_choice;
+  gboolean reply_set;
 } UnmountData;
 
 static UnmountData *
@@ -603,6 +608,7 @@ unmount_data_complete (UnmountData *data,
   else
     g_simple_async_result_complete (data->simple);
 
+  data->in_progress = FALSE;
   data->completed = TRUE;
   unmount_data_unref (data);
 }
@@ -620,6 +626,9 @@ on_retry_timer_cb (gpointer user_data)
   /* we're removing the timeout */
   data->retry_unmount_timer_id = 0;
 
+  if (data->completed || data->in_progress)
+    goto out;
+
   /* timeout expired => try again */
   unmount_do (data, FALSE);
 
@@ -628,23 +637,13 @@ on_retry_timer_cb (gpointer user_data)
 }
 
 static void
-on_mount_op_reply (GMountOperation       *mount_operation,
-                   GMountOperationResult result,
-                   gpointer              user_data)
+mount_op_reply_handle (UnmountData *data)
 {
-  UnmountData *data = user_data;
-  gint choice;
-
-  /* disconnect the signal handler */
-  g_warn_if_fail (data->mount_op_reply_handler_id != 0);
-  g_signal_handler_disconnect (data->mount_operation,
-                               data->mount_op_reply_handler_id);
-  data->mount_op_reply_handler_id = 0;
-
-  choice = g_mount_operation_get_choice (mount_operation);
+  data->reply_set = FALSE;
 
-  if (result == G_MOUNT_OPERATION_ABORTED ||
-      (result == G_MOUNT_OPERATION_HANDLED && choice == 1))
+  if (data->reply_result == G_MOUNT_OPERATION_ABORTED ||
+      (data->reply_result == G_MOUNT_OPERATION_HANDLED &&
+       data->reply_choice == 1))
     {
       /* don't show an error dialog here */
       g_simple_async_result_set_error (data->simple,
@@ -654,7 +653,7 @@ on_mount_op_reply (GMountOperation       *mount_operation,
                                        "error since it is G_IO_ERROR_FAILED_HANDLED)");
       unmount_data_complete (data, TRUE);
     }
-  else if (result == G_MOUNT_OPERATION_HANDLED)
+  else if (data->reply_result == G_MOUNT_OPERATION_HANDLED)
     {
       /* user chose force unmount => try again with force_unmount==TRUE */
       unmount_do (data, TRUE);
@@ -673,6 +672,28 @@ on_mount_op_reply (GMountOperation       *mount_operation,
 }
 
 static void
+on_mount_op_reply (GMountOperation       *mount_operation,
+                   GMountOperationResult result,
+                   gpointer              user_data)
+{
+  UnmountData *data = user_data;
+  gint choice;
+
+  /* disconnect the signal handler */
+  g_warn_if_fail (data->mount_op_reply_handler_id != 0);
+  g_signal_handler_disconnect (data->mount_operation,
+                               data->mount_op_reply_handler_id);
+  data->mount_op_reply_handler_id = 0;
+
+  choice = g_mount_operation_get_choice (mount_operation);
+  data->reply_result = result;
+  data->reply_choice = choice;
+  data->reply_set = TRUE;
+  if (!data->completed || !data->in_progress)
+    mount_op_reply_handle (data);
+}
+
+static void
 lsof_command_cb (GObject       *source_object,
                  GAsyncResult  *res,
                  gpointer       user_data)
@@ -788,6 +809,17 @@ unmount_show_busy (UnmountData  *data,
                    const gchar  *mount_point)
 {
   gchar *escaped_mount_point;
+
+  data->in_progress = FALSE;
+
+  /* We received an reply during an unmount operation which could not complete.
+   * Handle the reply now. */
+  if (data->reply_set)
+    {
+      mount_op_reply_handle (data);
+      return;
+    }
+
   escaped_mount_point = g_strescape (mount_point, NULL);
   gvfs_udisks2_utils_spawn (10, /* timeout in seconds */
                             data->cancellable,
@@ -914,6 +946,8 @@ unmount_do (UnmountData *data,
 {
   GVariantBuilder builder;
 
+  data->in_progress = TRUE;
+
   if (data->mount_operation != NULL)
     gvfs_udisks2_unmount_notify_start (data->mount_operation, 
                                        G_MOUNT (data->mount), NULL,