sim-mbim: fix race condition when sync requested during preload

This is an extremely tricky race condition.

 * During SIM object initialization, we try to load SIM type (first
   item loaded).
 * MMSimMbim SIM type loading step runs preload_subscriber_info(),
   which:
   ** Sets self->priv->preload = TRUE; so that it is not run anymore.
   ** Sets the sync monitor to clear preloaded info if sync needed.
   ** Runs the subscriber ready status operation asynchronously.
   ** Just before the subscriber ready status operation returns, the
      system goes to sleep.
   ** The resume logic kicks in, and we flag the modem with sync
      needed, which clears the self->priv->preload flag.
 * Then the subscriber ready status operation response arrives, and we
   store the IMSI and the other things.
 * When the next initialization step happens, given that
   self->priv->preload is cleared, we run attempt to run
   preload_subscriber_info() again, and this time it finds the info like
   IMSI is already set, so asserts:

    0x00007cbcd287523f (libglib-2.0.so.0 - gtestutils.c: 3253)      g_assertion_message
    0x00007cbcd28752a2 (libglib-2.0.so.0 - gtestutils.c: 3279)      g_assertion_message_expr
    0x00005cbdab0a2dc0 (ModemManager - mm-sim-mbim.c: 253)  subscriber_ready_status_ready
    0x00007cbcd29a173b (libgio-2.0.so.0 - gtask.c: 1230)    g_task_return_now
    0x00007cbcd29a0799 (libgio-2.0.so.0 - gtask.c: 1300)    g_task_return
    0x00007cbcd2a548e0 (libmbim-glib.so.4 - mbim-device.c: 264)     transaction_task_complete_and_free
    0x00007cbcd2a562fc (libmbim-glib.so.4 - mbim-device.c: 1047)    data_available
    0x00007cbcd28534a6 (libglib-2.0.so.0 - gmain.c: 3417)   g_main_context_dispatch
    0x00007cbcd28537b1 (libglib-2.0.so.0 - gmain.c: 4211)   g_main_context_iterate
    0x00007cbcd2853a25 (libglib-2.0.so.0 - gmain.c: 4411)   g_main_loop_run
    0x00005cbdab034d26 (ModemManager - main.c: 217) main
    0x00007cbcd25e16c5 (libc.so.6 + 0x000286c5)     __libc_init_first
    0x00007cbcd25e1781 (libc.so.6 + 0x00028781)     __libc_start_main
    0x00005cbdab034a40 (ModemManager + 0x00061a40)  _start

In order to solve this, upon a sync request the ongoing preload
operation will be cancelled.

1 files changed, 47 insertions, 19 deletions

diff --git a/src/mm-sim-mbim.c b/src/mm-sim-mbim.c
index 3cbac84b..68c35dca 100644
--- a/src/mm-sim-mbim.c
+++ b/src/mm-sim-mbim.c
@@ -38,6 +38,7 @@ G_DEFINE_TYPE (MMSimMbim, mm_sim_mbim, MM_TYPE_BASE_SIM)
 
 struct _MMSimMbimPrivate {
     gboolean           preload;
+    GCancellable      *preload_cancellable;
     GError            *preload_error;
     gchar             *imsi;
     gchar             *iccid;
@@ -154,6 +155,11 @@ setup_modem_sync_monitor (MMSimMbim *self)
 static void
 reset_subscriber_info (MMSimMbim *self)
 {
+    /* Request to stop any ongoing preload attempt */
+    g_cancellable_cancel (self->priv->preload_cancellable);
+    g_clear_object (&self->priv->preload_cancellable);
+
+    /* And reset the info right away */
     self->priv->preload = FALSE;
     g_clear_error (&self->priv->preload_error);
     g_clear_pointer (&self->priv->imsi, g_free);
@@ -185,13 +191,21 @@ application_list_query_ready (MbimDevice   *device,
     guint32                             application_count;
     guint32                             active_application_index;
     g_autoptr(MbimUiccApplicationArray) applications = NULL;
+    g_autoptr(GError)                   error = NULL;
 
     self = g_task_get_source_object (task);
 
-    g_assert (!self->priv->application_id);
-    g_assert (!self->priv->application_id_error);
+    response = mbim_device_command_finish (device, res, &error);
+
+    if (g_task_return_error_if_cancelled (task)) {
+        g_object_unref (task);
+        return;
+    }
+
+    g_clear_pointer (&self->priv->application_id, g_byte_array_unref);
+    g_clear_error (&self->priv->application_id_error);
+    self->priv->application_id_error = g_steal_pointer (&error);
 
-    response = mbim_device_command_finish (device, res, &self->priv->application_id_error);
     if (response &&
         mbim_message_response_get_result (response, MBIM_MESSAGE_TYPE_COMMAND_DONE, &self->priv->application_id_error) &&
         mbim_message_ms_uicc_low_level_access_application_list_response_parse (
@@ -219,6 +233,7 @@ application_list_query_ready (MbimDevice   *device,
 
     /* At this point we just complete, as all the info and errors have already
      * been stored */
+    g_clear_object (&self->priv->preload_cancellable);
     g_task_return_boolean (task, TRUE);
     g_object_unref (task);
 }
@@ -246,15 +261,24 @@ subscriber_ready_status_ready (MbimDevice   *device,
     MMSimMbim              *self;
     g_autoptr(MbimMessage)  response = NULL;
     g_autofree gchar       *raw_iccid = NULL;
+    g_autoptr(GError)       error = NULL;
 
     self = g_task_get_source_object (task);
 
-    g_assert (!self->priv->preload_error);
-    g_assert (!self->priv->imsi);
-    g_assert (!self->priv->iccid);
-    g_assert (!self->priv->iccid_error);
+    response = mbim_device_command_finish (device, res, &error);
+
+    if (g_task_return_error_if_cancelled (task)) {
+        g_object_unref (task);
+        return;
+    }
+
+    g_clear_error (&self->priv->preload_error);
+    g_clear_pointer (&self->priv->imsi, g_free);
+    g_clear_pointer (&self->priv->iccid, g_free);
+    g_clear_error (&self->priv->iccid_error);
+
+    self->priv->preload_error = g_steal_pointer (&error);
 
-    response = mbim_device_command_finish (device, res, &self->priv->preload_error);
     if (response && mbim_message_response_get_result (response, MBIM_MESSAGE_TYPE_COMMAND_DONE, &self->priv->preload_error)) {
         if (mbim_device_check_ms_mbimex_version (device, 3, 0)) {
             MbimSubscriberReadyStatusFlag flags = MBIM_SUBSCRIBER_READY_STATUS_FLAG_NONE;
@@ -338,13 +362,15 @@ preload_subscriber_info (MMSimMbim           *self,
                          GAsyncReadyCallback  callback,
                          gpointer             user_data)
 {
-    GTask      *task;
-    MbimDevice *device;
+    g_autoptr(GCancellable)  cancellable = NULL;
+    GTask                   *task;
+    MbimDevice              *device;
 
     if (!peek_device (self, &device, callback, user_data))
         return;
 
-    task = g_task_new (self, NULL, callback, user_data);
+    cancellable = g_cancellable_new ();
+    task = g_task_new (self, cancellable, callback, user_data);
 
     /* only preload one single time; the info of the SIM should not
      * change during runtime, unless we're handling hotplug events */
@@ -355,6 +381,8 @@ preload_subscriber_info (MMSimMbim           *self,
     }
     self->priv->preload = TRUE;
 
+    g_assert (!self->priv->preload_cancellable);
+    self->priv->preload_cancellable = g_steal_pointer (&cancellable);
 
 #if defined WITH_SUSPEND_RESUME
 
@@ -379,19 +407,21 @@ load_sim_identifier_finish (MMBaseSim     *_self,
     if (!preload_subscriber_info_finish (self, res, error))
         return NULL;
 
-    g_assert (self->priv->preload);
     if (self->priv->iccid_error) {
         g_propagate_error (error, g_error_copy (self->priv->iccid_error));
         return NULL;
     }
+
     if (self->priv->preload_error) {
         g_propagate_error (error, g_error_copy (self->priv->preload_error));
         return NULL;
     }
+
     if (!self->priv->iccid) {
         g_set_error (error, MM_CORE_ERROR, MM_CORE_ERROR_FAILED, "SIM iccid not available");
         return NULL;
     }
+
     return g_strdup (self->priv->iccid);
 }
 
@@ -416,15 +446,16 @@ load_imsi_finish (MMBaseSim     *_self,
     if (!preload_subscriber_info_finish (self, res, error))
         return NULL;
 
-    g_assert (self->priv->preload);
     if (self->priv->preload_error) {
         g_propagate_error (error, g_error_copy (self->priv->preload_error));
         return NULL;
     }
+
     if (!self->priv->imsi) {
         g_set_error (error, MM_CORE_ERROR, MM_CORE_ERROR_FAILED, "SIM imsi not available");
         return NULL;
     }
+
     return g_strdup (self->priv->imsi);
 }
 
@@ -436,7 +467,6 @@ load_imsi (MMBaseSim           *self,
     preload_subscriber_info (MM_SIM_MBIM (self), callback, user_data);
 }
 
-
 /*****************************************************************************/
 /* Load SIM identifier */
 
@@ -450,11 +480,11 @@ load_sim_type_finish (MMBaseSim     *_self,
     if (!preload_subscriber_info_finish (self, res, error))
         return MM_SIM_TYPE_UNKNOWN;
 
-    g_assert (self->priv->preload);
     if (self->priv->preload_error) {
         g_propagate_error (error, g_error_copy (self->priv->preload_error));
         return MM_SIM_TYPE_UNKNOWN;
     }
+
     return self->priv->sim_type;
 }
 
@@ -479,11 +509,11 @@ load_esim_status_finish (MMBaseSim     *_self,
     if (!preload_subscriber_info_finish (self, res, error))
         return MM_SIM_ESIM_STATUS_UNKNOWN;
 
-    g_assert (self->priv->preload);
     if (self->priv->preload_error) {
         g_propagate_error (error, g_error_copy (self->priv->preload_error));
         return MM_SIM_ESIM_STATUS_UNKNOWN;
     }
+
     return self->priv->esim_status;
 }
 
@@ -508,11 +538,11 @@ load_removability_finish (MMBaseSim     *_self,
     if (!preload_subscriber_info_finish (self, res, error))
         return MM_SIM_REMOVABILITY_UNKNOWN;
 
-    g_assert (self->priv->preload);
     if (self->priv->preload_error) {
         g_propagate_error (error, g_error_copy (self->priv->preload_error));
         return MM_SIM_REMOVABILITY_UNKNOWN;
     }
+
     return self->priv->removability;
 }
 
@@ -987,8 +1017,6 @@ read_binary_subscriber_info_ready (MMSimMbim    *self,
         return;
     }
 
-    g_assert (self->priv->preload);
-
     if (self->priv->application_id_error) {
         g_task_return_error (task, g_error_copy (self->priv->application_id_error));
         g_object_unref (task);