broadband-modem-qmi,broadband-modem-mbim: always use SMS_PART_INVALID_INDEX unstored parts

MMSmsList has logic to allow multiple unstored messages as long as
they use SMS_PART_INVALID_INDEX. These messages aren't stored so
they don't have an index.

But the MBIM and QMI modems used index 0 for unstored messages,
meaning there could only ever be one and also that multipart
messages would fail to be combined.

Let's fix that by just using SMS_PART_INVALID_INDEX for every
scenario where an unstored message comes in.

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

Signed-off-by: Dan Williams <dan@ioncontrol.co>

5 files changed, 199 insertions, 16 deletions

diff --git a/src/meson.build b/src/meson.build
index a9be07b0..6e1aae89 100644
--- a/src/meson.build
+++ b/src/meson.build
@@ -210,6 +210,7 @@ libport_dep = declare_dependency(
 sources = files(
   'mm-auth-provider.c',
   'mm-context.c',
+  'mm-bind.c',
 )
 
 incs = [
@@ -239,6 +240,33 @@ libauth_dep = declare_dependency(
   link_with: libauth,
 )
 
+# SMS library
+sources = files(
+  'mm-sms-list.c',
+  'mm-base-sms.c',
+)
+
+incs = [
+  top_inc,
+]
+
+deps = [libmm_glib_dep, libhelpers_dep, libauth_dep]
+
+private_deps = []
+
+libsms = static_library(
+  'sms',
+  sources: sources,
+  include_directories: incs,
+  dependencies: deps,
+)
+
+libsms_dep = declare_dependency(
+  include_directories: ['.'],
+  dependencies: deps,
+  link_with: libsms,
+)
+
 # Daemon enums, required by plugins
 headers = files(
   'mm-base-bearer.h',
@@ -297,15 +325,12 @@ sources = files(
   'mm-base-modem-at.c',
   'mm-base-modem.c',
   'mm-base-sim.c',
-  'mm-base-sms.c',
   'mm-sms-at.c',
-  'mm-bind.c',
   'mm-bearer-list.c',
   'mm-broadband-bearer.c',
   'mm-broadband-modem.c',
   'mm-call-list.c',
   'mm-cbm-list.c',
-  'mm-context.c',
   'mm-device.c',
   'mm-dispatcher.c',
   'mm-dispatcher-connection.c',
@@ -334,7 +359,6 @@ sources = files(
   'mm-port-probe.c',
   'mm-port-probe-at.c',
   'mm-private-boxed-types.c',
-  'mm-sms-list.c',
   'mm-sleep-context.c',
 )
 
@@ -344,6 +368,7 @@ deps = [
   gmodule_dep,
   libport_dep,
   libqcdm_dep,
+  libsms_dep,
   libauth_dep,
 ]
 
diff --git a/src/mm-broadband-modem-mbim.c b/src/mm-broadband-modem-mbim.c
index 5d7e37be..9957fe9e 100644
--- a/src/mm-broadband-modem-mbim.c
+++ b/src/mm-broadband-modem-mbim.c
@@ -5840,6 +5840,7 @@ static gboolean process_pdu_messages (MMBroadbandModemMbim       *self,
                                       guint32                     messages_count,
                                       MbimSmsPduReadRecordArray  *pdu_messages,
                                       guint                       expected_index,
+                                      gboolean                    unstored,
                                       GError                    **error);
 
 static void
@@ -5864,6 +5865,7 @@ sms_notification_read_flash_sms (MMBroadbandModemMbim *self,
                                messages_count,
                                pdu_messages,
                                SMS_PART_INVALID_INDEX,
+                               TRUE,
                                &error))
         mm_obj_dbg (self, "flash SMS message reading failed: %s", error->message);
 }
@@ -5943,6 +5945,7 @@ alert_sms_read_query_ready (MbimDevice             *device,
                                messages_count,
                                pdu_messages,
                                ctx->expected_index,
+                               FALSE,
                                &error))
         mm_obj_dbg (ctx->self, "SMS message reading failed: %s", error->message);
 
@@ -9024,12 +9027,13 @@ load_initial_sms_parts_finish (MMIfaceModemMessaging  *self,
 static void
 add_sms_part (MMBroadbandModemMbim       *self,
               const MbimSmsPduReadRecord *pdu,
+              guint                       part_index,
               guint                       expected_index)
 {
     MMSmsPart         *part;
     g_autoptr(GError)  error = NULL;
 
-    part = mm_sms_part_3gpp_new_from_binary_pdu (pdu->message_index,
+    part = mm_sms_part_3gpp_new_from_binary_pdu (part_index,
                                                  pdu->pdu_data,
                                                  pdu->pdu_data_size,
                                                  self,
@@ -9071,6 +9075,7 @@ process_pdu_messages (MMBroadbandModemMbim       *self,
                       guint32                     messages_count,
                       MbimSmsPduReadRecordArray  *pdu_messages,
                       guint                       expected_index,
+                      gboolean                    unstored,
                       GError                    **error)
 {
     guint i;
@@ -9096,11 +9101,26 @@ process_pdu_messages (MMBroadbandModemMbim       *self,
 
     mm_obj_dbg (self, "%u SMS PDUs reported", messages_count);
     for (i = 0; i < messages_count; i++) {
-        if (pdu_messages[i]) {
-            mm_obj_dbg (self, "processing SMS PDU %u/%u...", i+1, messages_count);
-            add_sms_part (self, pdu_messages[i], expected_index);
-        } else
+        guint message_index;
+
+        if (!pdu_messages[i]) {
             mm_obj_dbg (self, "ignoring invalid SMS PDU %u/%u...", i+1, messages_count);
+            continue;
+        }
+
+        mm_obj_dbg (self, "processing SMS PDU %u/%u...", i+1, messages_count);
+
+        message_index = pdu_messages[i]->message_index;
+        if (unstored && message_index != 0) {
+            mm_obj_warn (self,
+                         "processed unstored PDU with non-zero message index (%u)",
+                         message_index);
+        }
+
+        add_sms_part (self,
+                      pdu_messages[i],
+                      unstored ? SMS_PART_INVALID_INDEX : message_index,
+                      expected_index);
     }
 
     return TRUE;
@@ -9135,6 +9155,7 @@ sms_read_query_ready (MbimDevice   *device,
                                messages_count,
                                pdu_messages,
                                SMS_PART_INVALID_INDEX,
+                               FALSE,
                                &error))
         g_task_return_error (task, error);
     else
diff --git a/src/mm-broadband-modem-qmi.c b/src/mm-broadband-modem-qmi.c
index 4f153009..6d5b8b20 100644
--- a/src/mm-broadband-modem-qmi.c
+++ b/src/mm-broadband-modem-qmi.c
@@ -8414,7 +8414,6 @@ messaging_event_report_indication_cb (QmiClientNas *client,
     QmiWmsAckIndicator ack_ind;
     guint32 transaction_id;
     QmiWmsMessageFormat msg_format;
-    QmiWmsMessageTagType tag;
     GArray *raw_data = NULL;
 
     /* Handle transfer-route MT messages */
@@ -8459,13 +8458,10 @@ messaging_event_report_indication_cb (QmiClientNas *client,
         }
 
         /* Defaults for transfer-route messages, which are not stored anywhere */
-        storage = QMI_WMS_STORAGE_TYPE_NONE;
-        memory_index = 0;
-        tag = QMI_WMS_MESSAGE_TAG_TYPE_MT_NOT_READ;
         add_new_read_sms_part (MM_IFACE_MODEM_MESSAGING (self),
-                               storage,
-                               memory_index,
-                               tag,
+                               QMI_WMS_STORAGE_TYPE_NONE,
+                               SMS_PART_INVALID_INDEX,
+                               QMI_WMS_MESSAGE_TAG_TYPE_MT_NOT_READ,
                                msg_format,
                                TRUE,
                                raw_data);
diff --git a/src/tests/meson.build b/src/tests/meson.build
index a6a1ae72..ea9ea3a3 100644
--- a/src/tests/meson.build
+++ b/src/tests/meson.build
@@ -10,6 +10,7 @@ test_units = {
   'modem-helpers': libhelpers_dep,
   'sms-part-3gpp': libhelpers_dep,
   'sms-part-cdma': libhelpers_dep,
+  'sms-list': libsms_dep,
   'udev-rules': libkerneldevice_dep,
 }
 
diff --git a/src/tests/test-sms-list.c b/src/tests/test-sms-list.c
new file mode 100644
index 00000000..335d6cec
--- /dev/null
+++ b/src/tests/test-sms-list.c
@@ -0,0 +1,140 @@
+/* -*- Mode: C; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
+/*
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details:
+ *
+ * Copyright (C) 2025 Dan Williams <dan@ioncontrol.co>
+ */
+
+#include <glib.h>
+#include <glib-object.h>
+#include <string.h>
+#include <stdio.h>
+#include <locale.h>
+
+#define _LIBMM_INSIDE_MM
+#include <libmm-glib.h>
+
+#include "mm-sms-part-3gpp.h"
+#include "mm-sms-list.h"
+#include "mm-log-test.h"
+#include "mm-base-modem.h"
+
+#include "mm-iface-modem-messaging.h"
+
+/****************************************************************/
+
+static void
+test_mbim_multipart_unstored (void)
+{
+    static const gchar *part1_pdu =
+        "07912160130300f4440b915155685703f900005240713104738a3e050003c40202da6f37881e96"
+        "9fcbf4b4fb0ccabfeb20f4fb0ea287e5e7323ded3e83dae17519747fcbd96490b95c6683d27310"
+        "1d5d0601";
+    static const gchar *part2_pdu =
+        "07912160130300f4440b915155685703f900005240713104738aa0050003c40201ac69373d7c2e"
+        "83e87538bc2cbf87e565d039dc2e83c220769a4e6797416132394d4fbfdda0fb5b4e4783c2ee3c"
+        "888e2e83e86fd0db0c1a86e769f71b647eb3d9ef7bda7d06a5e7a0b09b0c9ab3df74109c1dce83"
+        "e8e8301d44479741f9771d949e83e861f9b94c4fbbcf20f13b4c9f83e8e832485c068ddfedf6db"
+        "0da2a3cba0fcbb0e1abfdb";
+    MMSmsPart *part1, *part2;
+    MMSmsList *list;
+    MMBaseSms *sms1, *sms2;
+    GError *error = NULL;
+
+    /* Some MBIM devices (Dell 5821e) report SMSes via the MBIM_CID_SMS_READ
+     * unsolicited notification usually used for Class-0 (flash/alert) messages.
+     * These always have a message index of 0; thus when a multipart SMS
+     * arrives it comes as two individual notifications both with 0 indexes.
+     * The MBIM modem code used SMS_PART_INVALID_INDEX for unstored SMSes.
+     * Ensure the MMSmsList can handle combining the two parts with the same
+     * index.
+     */
+    part1 = mm_sms_part_3gpp_new_from_pdu (SMS_PART_INVALID_INDEX, part1_pdu, NULL, &error);
+    g_assert_no_error (error);
+    part2 = mm_sms_part_3gpp_new_from_pdu (SMS_PART_INVALID_INDEX, part2_pdu, NULL, &error);
+    g_assert_no_error (error);
+
+    list = mm_sms_list_new (NULL);
+
+    sms1 = MM_BASE_SMS (g_object_new (MM_TYPE_BASE_SMS,
+                                      MM_BASE_SMS_IS_3GPP, TRUE,
+                                      MM_BASE_SMS_DEFAULT_STORAGE, MM_SMS_STORAGE_MT,
+                                      NULL));
+    mm_sms_list_take_part (list,
+                           sms1,
+                           part1,
+                           MM_SMS_STATE_RECEIVED,
+                           MM_SMS_STORAGE_MT,
+                           &error);
+    g_assert_no_error (error);
+
+    sms2 = MM_BASE_SMS (g_object_new (MM_TYPE_BASE_SMS,
+                                      MM_BASE_SMS_IS_3GPP, TRUE,
+                                      MM_BASE_SMS_DEFAULT_STORAGE, MM_SMS_STORAGE_MT,
+                                      NULL));
+    mm_sms_list_take_part (list,
+                           sms2,
+                           part2,
+                           MM_SMS_STATE_RECEIVED,
+                           MM_SMS_STORAGE_MT,
+                           &error);
+    g_assert_no_error (error);
+
+    g_assert_cmpint (mm_sms_list_get_count (list), ==, 1);
+}
+
+/****************************************************************/
+
+static void
+test_mbim_multipart_zero_index (void)
+{
+    static const gchar *pdu =
+        "07912160130300f4440b915155685703f900005240713104738a3e050003c40202da6f37881e96"
+        "9fcbf4b4fb0ccabfeb20f4fb0ea287e5e7323ded3e83dae17519747fcbd96490b95c6683d27310"
+        "1d5d0601";
+    MMSmsPart *part;
+    MMSmsList *list;
+    MMBaseSms *sms;
+    GError *error = NULL;
+
+    part = mm_sms_part_3gpp_new_from_pdu (0, pdu, NULL, &error);
+    g_assert_no_error (error);
+
+    list = mm_sms_list_new (NULL);
+
+    sms = MM_BASE_SMS (g_object_new (MM_TYPE_BASE_SMS,
+                                     MM_BASE_SMS_IS_3GPP, TRUE,
+                                     MM_BASE_SMS_DEFAULT_STORAGE, MM_SMS_STORAGE_MT,
+                                     NULL));
+    mm_sms_list_take_part (list,
+                           sms,
+                           part,
+                           MM_SMS_STATE_RECEIVED,
+                           MM_SMS_STORAGE_MT,
+                           &error);
+    g_assert_no_error (error);
+
+    g_assert_cmpint (mm_sms_list_get_count (list), ==, 1);
+}
+
+/****************************************************************/
+
+int main (int argc, char **argv)
+{
+    setlocale (LC_ALL, "");
+
+    g_test_init (&argc, &argv, NULL);
+
+    g_test_add_func ("/MM/SMS/3GPP/sms-list/zero-index", test_mbim_multipart_zero_index);
+    g_test_add_func ("/MM/SMS/3GPP/sms-list/mbim-multipart-unstored", test_mbim_multipart_unstored);
+
+    return g_test_run ();
+}