mbim-device: emit SIGNAL_ERROR only after completing the task

The task completion involves creating a duplicate of the MbimMessage,
so a duplicate of the contents of the internal `self->priv->response`
buffer.

This internal buffer may be cleared e.g. with a forced-close, which
users of the MbimDevice may decide to do upon a SIGNAL_ERROR, as the
mbim-proxy does.

So, avoid this race by making sure the task completion and the message
duplication happens before the SIGNAL_ERROR is emitted.

   Thread 0(id: 3296) CRASHED [ SIGSEGV /0x00000000@0x0000000000000004 ]
   0x00007ce3552f7c32 (libmbim-glib.so.4 - mbim-message.c: 1293) mbim_message_dup
   0x00007ce3552fbfd9 (libmbim-glib.so.4 - mbim-device.c: 661) data_available
   0x00007ce35525639a (libglib-2.0.so.0 - gmain.c: 3325) g_main_context_dispatch
   0x00007ce3552566a7 (libglib-2.0.so.0 - gmain.c: 4119) g_main_context_iterate
   0x00007ce355256923 (libglib-2.0.so.0 - gmain.c: 4317) g_main_loop_run
   0x00005ae0f48a5524 (mbim-proxy - mbim-proxy.c: 267) main
   0x00007ce35501ce04 (libc.so.6) __libc_start_main
   0x00005ae0f48a52d9 (mbim-proxy) _start
   0x00007ffcca5b6897

Fixes https://gitlab.freedesktop.org/mobile-broadband/ModemManager/-/issues/422

1 files changed, 19 insertions, 13 deletions

diff --git a/src/libmbim-glib/mbim-device.c b/src/libmbim-glib/mbim-device.c
index e7a9acf..422b11c 100644
--- a/src/libmbim-glib/mbim-device.c
+++ b/src/libmbim-glib/mbim-device.c
@@ -1061,16 +1061,6 @@ process_message (MbimDevice        *self,
         g_autoptr(GError)  error_indication = NULL;
         GTask             *task;
 
-        /* Try to match this transaction just per transaction ID */
-        task = device_release_transaction (self,
-                                           TRANSACTION_TYPE_HOST,
-                                           MBIM_MESSAGE_TYPE_INVALID,
-                                           mbim_message_get_transaction_id (message));
-
-        if (!task)
-            g_debug ("[%s] No transaction matched in received function error message",
-                     self->priv->path_display);
-
         if (mbim_utils_get_traces_enabled ()) {
             g_autofree gchar *printable = NULL;
 
@@ -1085,11 +1075,21 @@ process_message (MbimDevice        *self,
                      printable);
         }
 
-        /* Signals are emitted regardless of whether the transaction matched or not */
+        /* Build indication error before task completion, to ensure the message
+         * is valid */
         error_indication = mbim_message_error_get_error (message);
-        g_signal_emit (self, signals[SIGNAL_ERROR], 0, error_indication);
 
-        if (task) {
+        /* Try to match this transaction just per transaction ID */
+        task = device_release_transaction (self,
+                                           TRANSACTION_TYPE_HOST,
+                                           MBIM_MESSAGE_TYPE_INVALID,
+                                           mbim_message_get_transaction_id (message));
+
+        if (!task) {
+            g_debug ("[%s] No transaction matched in received function error message",
+                     self->priv->path_display);
+
+        } else {
             TransactionContext *ctx;
 
             ctx = g_task_get_task_data (task);
@@ -1099,6 +1099,12 @@ process_message (MbimDevice        *self,
             ctx->fragments = mbim_message_dup (message);
             transaction_task_complete_and_free (task, NULL);
         }
+
+        /* Signals are emitted regardless of whether the transaction matched or not;
+         * and emitted after the task completion, because the listeners of this
+         * signal may decide to force-close the device, which in turn clears the
+         * internal buffer and the MbimMessage. */
+        g_signal_emit (self, signals[SIGNAL_ERROR], 0, error_indication);
         return;
     }