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>
This commit is contained in:
Dan Williams
2025-05-08 20:25:22 -05:00
parent 51ff44f1f5
commit 1d5cc0addb
5 changed files with 199 additions and 16 deletions
+29 -4
View File
@@ -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,
]
+26 -5
View File
@@ -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
+3 -7
View File
@@ -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);
+1
View File
@@ -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,
}
+140
View File
@@ -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 ();
}