As we do in the similar modem state wait operations in the modem
interface, the caller of the wait operation may ignore the state
result as it's really not required in this case. Only looking at
whether an error is set or not is enough.
This by itself fixes a crash where
mm_iface_modem_3gpp_wait_for_packet_service_state_finish() may have
returned UNKNOWN without error set.
0x000079616c2564ed (libglib-2.0.so.0 - gtestutils.c: 3256) g_assertion_message
0x000079616c25654b (libglib-2.0.so.0 - gtestutils.c: 3282) g_assertion_message_expr
0x000057ba46231150 (ModemManager - mm-iface-modem-simple.c: 344) packet_service_attach_in_3gpp_network_step
0x000079616c0efb02 (libgio-2.0.so.0 - gtask.c: 1232) g_task_return_now
0x000079616c0eeb54 (libgio-2.0.so.0 - gtask.c: 1301) g_task_return
0x000057ba4620e5bd (ModemManager - mm-iface-modem-3gpp.c: 189) wait_for_packet_service_state_context_complete
0x000057ba4620b441 (ModemManager - mm-iface-modem-3gpp.c: 261) packet_service_state_changed
Rely exclusively on whether a global IPv6 was given or not, regardless
of what the DNS or gateway settings are, same as we do for IPv4.
Also, assume we only get 1 single IPv6 address. No need to have a
FIXME to add support for more addresses.
This will make device probing slower unconditionally, but it should
also ensure newer more complex devices that take longer to boot are
handled properly.
NL668 doesn't correctly support more than one multiplexed PDN
connection. Limit this explicitly so that it never attempts to use
more than one.
# mmcli -m a --simple-connect="multiplex=requested,apn=internet,ip-type=ipv4v6,apn-type=default"
successfully connected the modem
# mmcli -m a --simple-connect="multiplex=requested,apn=internet2,ip-type=ipv4v6,apn-type=default"
error: couldn't connect the modem: 'GDBus.Error:org.freedesktop.ModemManager1.Error.Core.Unsupported: Invalid bearer properties: Maximum number of multiplexed bearers reached'
Certain QMI or MBIM devices may not be able to support multiplexing at
all, or they may support a limited amount of links.
The new ID_MM_MAX_MULTIPLEXED_LINKS udev tag allows specifying the
maximum number of supported multiplexed links; e.g. 0 to report none
supported or 1 to report that one single multiplexed link is
supported.
Both the register state indications and the register state set
response messages will be processed in the same way.
This ensures that the registration state reported during the set
response, along with the additional info provided in that
message (e.g. access technologies) are taken into account.
This change also avoids fully relying on the register state
indications sent by the modem, as the indication may not be sent after
a successful update operation (given that the operation response
already contains the last valid state).
When running the 3GPP registration checks (e.g. after reaching enabled
state, or when synchronizing state after a quick suspend/resume), we
also update the current packet service state explicitly, without
needing to fully rely on the indications.
Both the packet service indications and the packet service set
response messages will be processed in the same way.
This ensures that the packet service state reported during the set
response, along with the additional info provided in that
message (e.g. speeds) are taken into account.
This change also avoids fully relying on the packet service
indications sent by the modem, as the indication may not be sent after
a successful update operation (given that the operation response
already contains the last valid state).
register_in_network_cancelled() may be called early if the given
cancellable is already cancelled, and if so, we want it to remove the
timeout and signal handler, which should have been configured before
setting up the cancellation signal handler.
Returning an error in the registration set request completely breaks
the async operation requesting the explicit registration, even if the
modem may have already been registered via indications while waiting
for the set request to be completed.
We now ignore the registration set error if it returns a generic
failure with NwError=0, as in certain Qualcomm based devices. We
ignore unconditionally, without explicitly checking if we're
registered or not, because the upper logic will anyway do that.
Fixes https://gitlab.freedesktop.org/mobile-broadband/ModemManager/-/issues/723
So that it is reloaded fresh once re-enabled, otherwise we may be
using stale state values that are not in sync with the state reported
in the interface skeletons.
We now support modem-reported packet service state, instead of
guessing it in the 3GPP interface logic.
The DSD system status reporting a valid access technology is an
indication that the packet service is ready. Instead of holding back
the registration status until the DSD system status reports a valid
value, we will now report the modem as registered, and bind the DSD
system status exclusively to the packet service status.
In the simple connection attempt we're already waiting first for being
registered, and then for being attached in PS.
In certain protocols like QMI or MBIM we may be able to report an
exact packet service state, so there is no need to guess it, as the
guess may not always be right.
The logic will track automatically whether modem-reported packet
service states are available, and use them if so. Otherwise, it'll try
to guess as we were doing before (e.g. if registered in EPS, packet
service is considered attached).
Users with QMI or MBIM capable modems may want to ensure that these
are never managed using plain AT commands, as that also involves using
PPP. This fallback to AT could happen if the QMI or MBIM port probing
fails for whatever reason.
The new `ID_MM_REQUIRED` udev tag allows specifying that a given port
MUST be successfully grabbed when creating a new modem object, or
otherwise the modem object will not be created at all (even if there
are other fallback control ports like AT that could have been used).
Use this tag with caution.
It is assumed that when this tag is used some other external process
may be monitoring the existence of the modem object in DBus as exposed
by ModemManager, and if it does not appear for any reason then the
modem would be reseted with some other mechanism (e.g. GPIOs, if
available). If no such mechanism to autorecover the modem is in place,
using this tag may leave the modem exposed in the kernel but ignored
by ModemManager.
This tag must be applied on the specific port for which the existence
and usability must be ensured.
E.g. flagging the MBIM port of the Fibocom L850 module as required:
$ vim /lib/udev/rules.d/78-mm-test.rules
ACTION!="add|change|move|bind", GOTO="mm_test_rules_end"
SUBSYSTEMS=="usb", ATTRS{bInterfaceNumber}=="?*", ENV{.MM_USBIFNUM}="$attr{bInterfaceNumber}"
ATTRS{idVendor}=="2cb7", ATTRS{idProduct}=="0007", ENV{.MM_USBIFNUM}=="00", ENV{ID_MM_REQUIRED}="1"
LABEL="mm_test_rules_end"
$ sudo udevadm control --reload
$ sudo udevadm trigger
$ sudo udevadm info -p /sys/class/usbmisc/cdc-wdm0
...
E: ID_MM_REQUIRED=1
E: ID_MM_CANDIDATE=1
To make it clearer that the initial list of flags must be the one
based on which ones are expected in the subsystem and which one the
plugin is requesting.
We can remove the port type hint udev tags for the wwan subsystem, as
that logic is now incorporated in the port type hint processing logic
in the daemon itself.
This makes it clearer to know what exact hints are being used, as the
logic is in a single place and it has proper logging of all possible
cases.
Do not do this in the plugin; instead, do it along with the logic that
looks for port type hints in udev, so that we can properly warn if
things don't match.
The personalization feature enum used in "card status" is different to
the one used in other UIM operations like "depersonalization".
libqmi dependency updated to 1.33.6 to ensure we can use the new types.
The release buildtype will disable certain warnings that we do see in
debug builds. Ensure we have a test build with all features enabled in
debug mode.
Selecting the build type as release limits the amount of warnings that
are enabled, so ensure we always build with warnings treated as errors
so that we don't miss any warning that would happen on debug builds.
==174467== Invalid read of size 1
==174467== at 0x10B80C: read_bits (mm-sms-part-cdma.c:255)
==174467== by 0x10B886: read_bits (mm-sms-part-cdma.c:260)
==174467== by 0x10DC2F: read_bearer_data_user_data (mm-sms-part-cdma.c:882)
==174467== by 0x10DC2F: read_bearer_data (mm-sms-part-cdma.c:1000)
==174467== by 0x10DC2F: mm_sms_part_cdma_new_from_binary_pdu (mm-sms-part-cdma.c:1180)
==174467== by 0x10DF24: mm_sms_part_cdma_new_from_pdu (mm-sms-part-cdma.c:331)
==174467== by 0x10A91D: common_test_valid_part_from_hexpdu (test-sms-part-cdma.c:114)
==174467== by 0x10B0AC: common_test_valid_part_from_pdu (test-sms-part-cdma.c:126)
==174467== by 0x10B0AC: test_invalid_ascii_user_data (test-sms-part-cdma.c:412)
==174467== by 0x4A0264D: ??? (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
==174467== by 0x4A023B4: ??? (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
==174467== by 0x4A023B4: ??? (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
==174467== by 0x4A023B4: ??? (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
==174467== by 0x4A023B4: ??? (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
==174467== by 0x4A02B1A: g_test_run_suite (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
==174467== Address 0x51a6457 is 0 bytes after a block of size 7 alloc'd
==174467== at 0x48455EF: calloc (vg_replace_malloc.c:1328)
==174467== by 0x49DF6C0: g_malloc0 (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
==174467== by 0x48ABD24: mm_utils_hexstr2bin (mm-common-helpers.c:1884)
==174467== by 0x10DF06: mm_sms_part_cdma_new_from_pdu (mm-sms-part-cdma.c:325)
==174467== by 0x10A91D: common_test_valid_part_from_hexpdu (test-sms-part-cdma.c:114)
==174467== by 0x10B0AC: common_test_valid_part_from_pdu (test-sms-part-cdma.c:126)
==174467== by 0x10B0AC: test_invalid_ascii_user_data (test-sms-part-cdma.c:412)
==174467== by 0x4A0264D: ??? (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
==174467== by 0x4A023B4: ??? (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
==174467== by 0x4A023B4: ??? (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
==174467== by 0x4A023B4: ??? (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
==174467== by 0x4A023B4: ??? (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
==174467== by 0x4A02B1A: g_test_run_suite (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
Same fix also applied to latin encoded text as it also makes sense there.
==158856== Invalid read of size 1
==158856== at 0x10B814: read_bits (mm-sms-part-cdma.c:257)
==158856== by 0x10DB07: read_bearer_data_user_data (mm-sms-part-cdma.c:878)
==158856== by 0x10DB07: read_bearer_data (mm-sms-part-cdma.c:990)
==158856== by 0x10DB07: mm_sms_part_cdma_new_from_binary_pdu (mm-sms-part-cdma.c:1170)
==158856== by 0x10DE54: mm_sms_part_cdma_new_from_pdu (mm-sms-part-cdma.c:333)
==158856== by 0x10A916: common_test_invalid_part_from_hexpdu (test-sms-part-cdma.c:90)
==158856== by 0x10A916: common_test_invalid_part_from_pdu (test-sms-part-cdma.c:104)
==158856== by 0x4A0264D: ??? (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
==158856== by 0x4A023B4: ??? (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
==158856== by 0x4A023B4: ??? (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
==158856== by 0x4A023B4: ??? (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
==158856== by 0x4A023B4: ??? (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
==158856== by 0x4A02B1A: g_test_run_suite (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
==158856== by 0x4A02BBC: g_test_run (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
==158856== by 0x10A509: main (test-sms-part-cdma.c:595)
==158856== Address 0x51a627b is 0 bytes after a block of size 11 alloc'd
==158856== at 0x48455EF: calloc (vg_replace_malloc.c:1328)
==158856== by 0x49DF6C0: g_malloc0 (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
==158856== by 0x48ABD24: mm_utils_hexstr2bin (mm-common-helpers.c:1884)
==158856== by 0x10DE36: mm_sms_part_cdma_new_from_pdu (mm-sms-part-cdma.c:327)
==158856== by 0x10A916: common_test_invalid_part_from_hexpdu (test-sms-part-cdma.c:90)
==158856== by 0x10A916: common_test_invalid_part_from_pdu (test-sms-part-cdma.c:104)
==158856== by 0x4A0264D: ??? (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
==158856== by 0x4A023B4: ??? (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
==158856== by 0x4A023B4: ??? (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
==158856== by 0x4A023B4: ??? (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
==158856== by 0x4A023B4: ??? (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
==158856== by 0x4A02B1A: g_test_run_suite (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
==158856== by 0x4A02BBC: g_test_run (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
==158856==
GLib-CRITICAL **: 21:21:51.419: g_convert: assertion 'str != NULL' failed
Program received signal SIGTRAP, Trace/breakpoint trap.
0x00007ffff7db3e82 in g_logv () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
(gdb) bt
#0 0x00007ffff7db3e82 in g_logv () at /lib/x86_64-linux-gnu/libglib-2.0.so.0
#1 0x00007ffff7db40ef in g_log () at /lib/x86_64-linux-gnu/libglib-2.0.so.0
#2 0x00007ffff7d8a5da in g_convert () at /lib/x86_64-linux-gnu/libglib-2.0.so.0
#3 0x00005555555592cf in read_bearer_data_user_data (log_object=0x0, subparameter=<optimized out>, sms_part=0x555555578000)
at ../src/mm-sms-part-cdma.c:929
#4 read_bearer_data (log_object=0x0, parameter=<optimized out>, sms_part=0x555555578000) at ../src/mm-sms-part-cdma.c:982
#5 mm_sms_part_cdma_new_from_binary_pdu
[debug] parsing PDU (0)...
[debug] no SMSC address given
[debug] submit type PDU detected
[debug] message reference: 1
[debug] address size: 1 digits (1 bytes)
[debug] number parsed: 00
[debug] validity available, format relative
[debug] PID: 0
[debug] user data encoding is GSM7
[debug] user data length: 0 elements
[debug] user data length: 0 bytes
==125780== Command: ./build/test/mmsmspdu --pdu=00F101010C0000000000 --verbose
==125780==
==125780== Invalid read of size 1
==125780== at 0x10B422: mm_sms_part_3gpp_new_from_binary_pdu (mm-sms-part-3gpp.c:698)
==125780== by 0x10BF57: mm_sms_part_3gpp_new_from_pdu (mm-sms-part-3gpp.c:368)
==125780== by 0x10A44D: main (mmsmspdu.c:242)
==125780== Address 0x519988a is 0 bytes after a block of size 10 alloc'd
==125780== at 0x48455EF: calloc (vg_replace_malloc.c:1328)
==125780== by 0x49DF6C0: g_malloc0 (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
==125780== by 0x48ABD24: mm_utils_hexstr2bin (mm-common-helpers.c:1884)
==125780== by 0x10BF36: mm_sms_part_3gpp_new_from_pdu (mm-sms-part-3gpp.c:362)
==125780== by 0x10A44D: main (mmsmspdu.c:242)
==101461== Command: ./build/test/mmsmspdu --pdu=004100010100014B00002E --verbose
==101461==
[debug] parsing PDU (0)...
[debug] no SMSC address given
[debug] submit type PDU detected
[debug] message reference: 0
[debug] address size: 1 digits (1 bytes)
[debug] number parsed: 00
[debug] PID: 1
[debug] user data encoding is GSM7
[debug] user data length: 0 elements
[debug] user data length: 0 bytes
[debug] decoding SMS text with 4294967294 elements
Based on a patch from Michal Mazur <mkm@semihalf.com>.
Before the actual number digits there is always a Type of Address byte
that we were not considering during the size check.
[debug] parsing PDU (0)...
[debug] no SMSC address given
[debug] deliver type PDU detected
[debug] address size: 1 digits (1 bytes)
==90832== Command: ./build/test/mmsmspdu --pdu=001C011C --verbose
==90832==
==90832== Invalid read of size 1
==90832== at 0x10AC90: sms_semi_octets_to_bcd_string (mm-sms-part-3gpp.c:71)
==90832== by 0x10AC90: sms_decode_address (mm-sms-part-3gpp.c:157)
==90832== by 0x10B0C5: mm_sms_part_3gpp_new_from_binary_pdu (mm-sms-part-3gpp.c:512)
==90832== by 0x10BF77: mm_sms_part_3gpp_new_from_pdu (mm-sms-part-3gpp.c:368)
==90832== by 0x10A44D: main (mmsmspdu.c:242)
==90832== Address 0x5199874 is 0 bytes after a block of size 4 alloc'd
==90832== at 0x48455EF: calloc (vg_replace_malloc.c:1328)
==90832== by 0x49DF6C0: g_malloc0 (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
==90832== by 0x48ABD24: mm_utils_hexstr2bin (mm-common-helpers.c:1884)
==90832== by 0x10BF56: mm_sms_part_3gpp_new_from_pdu (mm-sms-part-3gpp.c:362)
==90832== by 0x10A44D: main (mmsmspdu.c:242)
[debug] parsing PDU (0)...
[debug] no SMSC address given
[debug] status report type PDU detected
[debug] message reference: 191
[debug] address size: 0 digits (0 bytes)
==78906== Command: ./build/test/mmsmspdu --pdu=000ABF00 --verbose
==78906==
==78906== Invalid read of size 1
==78906== at 0x10AA80: sms_decode_address (mm-sms-part-3gpp.c:132)
==78906== by 0x10AF7C: mm_sms_part_3gpp_new_from_binary_pdu (mm-sms-part-3gpp.c:507)
==78906== by 0x10BE17: mm_sms_part_3gpp_new_from_pdu (mm-sms-part-3gpp.c:368)
==78906== by 0x10A44D: main (mmsmspdu.c:202)
==78906== Address 0x5199874 is 0 bytes after a block of size 4 alloc'd
==78906== at 0x48455EF: calloc (vg_replace_malloc.c:1328)
==78906== by 0x49DF6C0: g_malloc0 (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
==78906== by 0x48ABD24: mm_utils_hexstr2bin (mm-common-helpers.c:1884)
==78906== by 0x10BDF6: mm_sms_part_3gpp_new_from_pdu (mm-sms-part-3gpp.c:362)
==78906== by 0x10A44D: main (mmsmspdu.c:202)
This reverts commit dad3e82747.
Disabling profile change indications just before doing our profile
updates and re-enabling them just after is not enough. The modem may
still emit the profile change indication right after, so all this
logic is unnecessary.
Just ignoring the received indications is not enough, because they
could arrive after the operation response has been processed.
We now explicitly disable the indications by reconfiguring the modem
before and after every profile update operation triggered by our own
logic.
[1/2] Compiling C object src/ModemManager.p/mm-base-manager.c.o
../src/mm-base-manager.c: In function ‘remove_device_inhibition’:
../src/mm-base-manager.c:1127:20: warning: declaration of ‘l’ shadows a previous local [-Wshadow]
1127 | GList *l;
| ^
../src/mm-base-manager.c:1116:16: note: shadowed declaration is here
1116 | GList *l;
| ^
If the device is not fully removed from the system during inhibition
and port additions happen, we reach a state where the MMDevice object
is still tracked by ModemManager and we also have new port additions
tracked that will require explicit port probings before a new modem
object can be created.
Solve this mixup by faking the removal of all existing device ports,
which will end up completely removing hte MMDevice, so that new port
additions reported afterwards also involve the full device probing
process triggered by the plugin manager.
The issue could be reproduced easily on a MBIM device that also
exposed TTYs, as follows:
* mmcli -m a --inhibit, leave it running
* rmmod cdc_mbim && sleep 5 && modprobe cdc_mbim (so that the
cdc-wdm and net ports go away from the system but NOT the TTYs.)
* Cltr+C to stop the inhibit in the mmcli call.
ModemManager would assert as follows:
0x000079918bcf2a3f (libc.so.6 + 0x00087a3f) pthread_key_delete
0x000079918bca7c6c (libc.so.6 + 0x0003cc6c) gsignal
0x000079918bc93462 (libc.so.6 + 0x00028462) abort
0x000079918bf26f03 (libglib-2.0.so.0 - gtestutils.c: 3253) g_assertion_message
0x000079918bf26f66 (libglib-2.0.so.0 - gtestutils.c: 3279) g_assertion_message_expr
0x0000594ff1093d0c (ModemManager - mm-base-manager.c: 1110) remove_device_inhibition
0x0000594ff1093968 (ModemManager - mm-base-manager.c: 1247) inhibit_device_auth_ready
0x000079918c0536a8 (libgio-2.0.so.0 - gtask.c: 1230) g_task_return_now
0x000079918c0536db (libgio-2.0.so.0 - gtask.c: 1244) complete_in_idle_cb
0x000079918bf053fc (libglib-2.0.so.0 - gmain.c: 3417) g_main_context_dispatch
0x000079918bf05704 (libglib-2.0.so.0 - gmain.c: 4211) g_main_context_iterate
0x000079918bf05978 (libglib-2.0.so.0 - gmain.c: 4411) g_main_loop_run
0x0000594ff108de66 (ModemManager - main.c: 217) main
0x000079918bc936c5 (libc.so.6 + 0x000286c5) __libc_init_first
0x000079918bc93781 (libc.so.6 + 0x00028781) __libc_start_main
0x0000594ff108db80 (ModemManager + 0x00061b80) _start
Aleksander Morgado