xmm: fix crash parsing XACT? response

Ensure we don't assert when processing an unexpected response.

We were not correctly handling the case where g_match_info_matches()
was not succeeding.

  0x00007d8be96cc462 (libc.so.6 + 0x00028462)     abort
  0x00007d8be995ff01 (libglib-2.0.so.0 - gtestutils.c: 3253)      g_assertion_message
  0x00007d8be995ff64 (libglib-2.0.so.0 - gtestutils.c: 3279)      g_assertion_message_expr
  0x00007d8be858086a (libmm-shared-xmm.so - mm-modem-helpers-xmm.c: 467)  mm_xmm_parse_xact_query_response
  0x00007d8be857d4e0 (libmm-shared-xmm.so - mm-shared-xmm.c: 310) xact_query_bands_ready
  0x00007d8be9a79463 (libgio-2.0.so.0 - gsimpleasyncresult.c: 802)        g_simple_async_result_complete
  0x00005c8d2a11e9b9 (ModemManager - mm-base-modem-at.c: 538)     at_command_ready
  0x00007d8be9a79463 (libgio-2.0.so.0 - gsimpleasyncresult.c: 802)        g_simple_async_result_complete
  0x00005c8d2a19376b (ModemManager - mm-port-serial-at.c) serial_command_ready
  0x00007d8be9a79463 (libgio-2.0.so.0 - gsimpleasyncresult.c: 802)        g_simple_async_result_complete
  0x00005c8d2a18f93f (ModemManager - mm-port-serial.c: 139)       command_context_complete_and_free
  0x00005c8d2a192985 (ModemManager - mm-port-serial.c: 753)       port_serial_got_response
  0x00005c8d2a192dff (ModemManager - mm-port-serial.c: 924)       common_input_available
  0x00007d8be993e3fc (libglib-2.0.so.0 - gmain.c: 3417)   g_main_context_dispatch
  0x00007d8be993e704 (libglib-2.0.so.0 - gmain.c: 4211)   g_main_context_iterate
  0x00007d8be993e978 (libglib-2.0.so.0 - gmain.c: 4411)   g_main_loop_run
  0x00005c8d2a105e66 (ModemManager - main.c: 217) main
  0x00007d8be96cc6c5 (libc.so.6 + 0x000286c5)     __libc_init_first
  0x00007d8be96cc781 (libc.so.6 + 0x00028781)     __libc_start_main
  0x00005c8d2a105b80 (ModemManager + 0x00061b80)  _start

2 files changed, 90 insertions, 58 deletions

diff --git a/src/plugins/xmm/mm-modem-helpers-xmm.c b/src/plugins/xmm/mm-modem-helpers-xmm.c
index 70e02a8f..67bc9ccc 100644
--- a/src/plugins/xmm/mm-modem-helpers-xmm.c
+++ b/src/plugins/xmm/mm-modem-helpers-xmm.c
@@ -366,10 +366,9 @@ mm_xmm_parse_xact_query_response (const gchar             *response,
 {
     g_autoptr(GRegex)      r = NULL;
     g_autoptr(GMatchInfo)  match_info = NULL;
+    g_autoptr(GArray)      bands = NULL;
     GError                *inner_error = NULL;
-    GArray                *bands = NULL;
     guint                  i;
-
     MMModemModeCombination mode = {
         .allowed   = MM_MODEM_MODE_NONE,
         .preferred = MM_MODEM_MODE_NONE,
@@ -390,73 +389,77 @@ mm_xmm_parse_xact_query_response (const gchar             *response,
     g_assert (r != NULL);
 
     g_regex_match_full (r, response, strlen (response), 0, 0, &match_info, &inner_error);
-    if (!inner_error && g_match_info_matches (match_info)) {
-        if (mode_out) {
-            guint xmm_mode;
-
-            /* Number at index 1 */
-            mm_get_uint_from_match_info (match_info, 1, &xmm_mode);
-            if (xmm_mode >= G_N_ELEMENTS (xmm_modes)) {
-                inner_error = g_error_new (MM_CORE_ERROR, MM_CORE_ERROR_FAILED, "Unsupported XACT AcT value: %u", xmm_mode);
-                goto out;
-            }
-            mode.allowed = xmm_modes[xmm_mode];
+    if (inner_error) {
+        g_propagate_error (error, inner_error);
+        return FALSE;
+    }
 
-            /* Number at index 2 */
-            if (mm_count_bits_set (mode.allowed) > 1 && mm_get_uint_from_match_info (match_info, 2, &xmm_mode)) {
-                if (xmm_mode >= G_N_ELEMENTS (xmm_modes)) {
-                    inner_error = g_error_new (MM_CORE_ERROR, MM_CORE_ERROR_FAILED, "Unsupported XACT preferred AcT value: %u", xmm_mode);
-                    goto out;
-                }
-                mode.preferred = xmm_modes[xmm_mode];
-            }
+    if (!g_match_info_matches (match_info)) {
+        g_set_error (error, MM_CORE_ERROR, MM_CORE_ERROR_FAILED,
+                     "Unsupported XACT? response: %s", response);
+        return FALSE;
+    }
 
-            /* Number at index 3: ignored */
+    if (mode_out) {
+        guint xmm_mode;
+
+        /* Number at index 1 */
+        mm_get_uint_from_match_info (match_info, 1, &xmm_mode);
+        if (xmm_mode >= G_N_ELEMENTS (xmm_modes)) {
+            g_set_error (error, MM_CORE_ERROR, MM_CORE_ERROR_FAILED,
+                         "Unsupported XACT AcT value: %u", xmm_mode);
+            return FALSE;
         }
+        mode.allowed = xmm_modes[xmm_mode];
 
-        if (bands_out) {
-            gchar  *bandstr;
-            GArray *nums;
+        /* Number at index 2 */
+        if (mm_count_bits_set (mode.allowed) > 1 && mm_get_uint_from_match_info (match_info, 2, &xmm_mode)) {
+            if (xmm_mode >= G_N_ELEMENTS (xmm_modes)) {
+                g_set_error (error, MM_CORE_ERROR, MM_CORE_ERROR_FAILED,
+                             "Unsupported XACT preferred AcT value: %u", xmm_mode);
+                return FALSE;
+            }
+            mode.preferred = xmm_modes[xmm_mode];
+        }
 
-            /* Bands start at index 4 */
-            bandstr = mm_get_string_unquoted_from_match_info (match_info, 4);
-            nums = mm_parse_uint_list (bandstr, &inner_error);
-            g_free (bandstr);
+        /* Number at index 3: ignored */
+    }
 
-            if (inner_error)
-                goto out;
-            if (!nums) {
-                inner_error = g_error_new (MM_CORE_ERROR, MM_CORE_ERROR_FAILED, "Invalid XACT? response");
-                goto out;
-            }
+    if (bands_out) {
+        g_autofree gchar *bandstr = NULL;
+        g_autoptr(GArray) nums = NULL;
+
+        /* Bands start at index 4 */
+        bandstr = mm_get_string_unquoted_from_match_info (match_info, 4);
+        nums = mm_parse_uint_list (bandstr, &inner_error);
+        if (inner_error) {
+            g_propagate_error (error, inner_error);
+            return FALSE;
+        }
+        if (!nums) {
+            g_set_error (error, MM_CORE_ERROR, MM_CORE_ERROR_FAILED,
+                         "Missing bands in XACT? response: %s", response);
+            return FALSE;
+        }
 
-            bands = g_array_sized_new (FALSE, FALSE, sizeof (MMModemBand), nums->len);
-            for (i = 0; i < nums->len; i++) {
-                MMModemBand band;
+        bands = g_array_sized_new (FALSE, FALSE, sizeof (MMModemBand), nums->len);
+        for (i = 0; i < nums->len; i++) {
+            MMModemBand band;
 
-                band = xact_num_to_band (g_array_index (nums, guint, i));
-                if (band != MM_MODEM_BAND_UNKNOWN)
-                    g_array_append_val (bands, band);
-            }
-            g_array_unref (nums);
+            band = xact_num_to_band (g_array_index (nums, guint, i));
+            if (band != MM_MODEM_BAND_UNKNOWN)
+                g_array_append_val (bands, band);
+        }
 
-            if (bands->len == 0) {
-                inner_error = g_error_new (MM_CORE_ERROR, MM_CORE_ERROR_FAILED, "Missing current band list");
-                goto out;
-            }
+        if (bands->len == 0) {
+            g_set_error (error, MM_CORE_ERROR, MM_CORE_ERROR_FAILED,
+                         "Invalid list of bands in XACT? response: %s", response);
+            return FALSE;
         }
     }
 
     /* success */
 
-out:
-    if (inner_error) {
-        if (bands)
-            g_array_unref (bands);
-        g_propagate_error (error, inner_error);
-        return FALSE;
-    }
-
     if (mode_out) {
         g_assert (mode.allowed != MM_MODEM_MODE_NONE);
         mode_out->allowed = mode.allowed;
@@ -465,7 +468,7 @@ out:
 
     if (bands_out) {
         g_assert (bands);
-        *bands_out = bands;
+        *bands_out = g_steal_pointer (&bands);
     }
 
     return TRUE;
diff --git a/src/plugins/xmm/tests/test-modem-helpers-xmm.c b/src/plugins/xmm/tests/test-modem-helpers-xmm.c
index e40ffcab..27738d2d 100644
--- a/src/plugins/xmm/tests/test-modem-helpers-xmm.c
+++ b/src/plugins/xmm/tests/test-modem-helpers-xmm.c
@@ -274,6 +274,33 @@ test_xact_query_3g_4g (void)
                                   expected_bands, G_N_ELEMENTS (expected_bands));
 }
 
+static void
+test_xact_query_no_match_mode (void)
+{
+    g_autoptr(GError)      error = NULL;
+    gboolean               ret;
+    MMModemModeCombination mode = {
+        .allowed = MM_MODEM_MODE_NONE,
+        .preferred = MM_MODEM_MODE_NONE,
+    };
+
+    ret = mm_xmm_parse_xact_query_response ("something here", &mode, NULL, &error);
+    g_assert (error);
+    g_assert (!ret);
+}
+
+static void
+test_xact_query_no_match_bands (void)
+{
+    g_autoptr(GError) error = NULL;
+    g_autoptr(GArray) bands = NULL;
+    gboolean          ret;
+
+    ret = mm_xmm_parse_xact_query_response ("something here", NULL, &bands, &error);
+    g_assert (error);
+    g_assert (!ret);
+}
+
 /*****************************************************************************/
 
 #define XACT_SET_TEST_MAX_BANDS 6
@@ -764,8 +791,10 @@ int main (int argc, char **argv)
     g_test_add_func ("/MM/xmm/xact/test/3g-4g",    test_xact_test_3g_4g);
     g_test_add_func ("/MM/xmm/xact/test/2g-3g-4g", test_xact_test_2g_3g_4g);
 
-    g_test_add_func ("/MM/xmm/xact/query/3g-only", test_xact_query_3g_only);
-    g_test_add_func ("/MM/xmm/xact/query/3g-4g",   test_xact_query_3g_4g);
+    g_test_add_func ("/MM/xmm/xact/query/3g-only",        test_xact_query_3g_only);
+    g_test_add_func ("/MM/xmm/xact/query/3g-4g",          test_xact_query_3g_4g);
+    g_test_add_func ("/MM/xmm/xact/query/no-match/mode",  test_xact_query_no_match_mode);
+    g_test_add_func ("/MM/xmm/xact/query/no-match/bands", test_xact_query_no_match_bands);
 
     g_test_add_func ("/MM/xmm/xact/set", test_xact_set);