A cell broadcast message using unsupported encodings would trigger an
assert in ModemManager and kill the process. A compromised base station
or modem firmware could craft or pass through these messages.
Reported by Yehia Ezzat and Keith Linneman
Impact: Moderate
CVSS:3.1/AV:A/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H (6.5 Medium)
CWE-617 (Reachable Assertion)
Signed-off-by: Dan Williams <dan@ioncontrol.co>
The qmi-proxy/mbim-proxy processes spawned by the daemon live in the
service's cgroup, so with the default KillMode=control-group systemd
sends SIGTERM to them at the same time as to the daemon on every unit
stop. The proxy exits immediately, tearing the control channel from
under the daemon while it is still disabling the modems:
<msg> caught signal, shutting down...
<msg> [modem0] state changed (connected -> disabling)
<msg> [modem0] port 'cdc-wdm0' no longer controllable, reprobing
<wrn> [/dev/cdc-wdm0] MBIM error: Device must be open to send commands
<wrn> shutdown failed: timeout waiting for sleep preparation to complete
The consequences of losing the proxy mid-shutdown:
- the modem disable/disconnect never reaches the device, so an MBIM
session with an active bearer is left open in the firmware (observed
to occasionally wedge the MBIM function of a Sierra EM7565 until a
firmware reset);
- the port teardown stalls waiting on the dead proxy until the
shutdown timeout, so stopping the unit always takes the full
MAX_SHUTDOWN_TIME_SECS instead of a second or two;
- the port invalidation triggers the device reprobe path in the middle
of the shutdown.
With KillMode=mixed only the daemon receives SIGTERM; the proxies
survive long enough for a clean disconnect and MBIM/QMI close, and
whatever remains in the cgroup afterwards (an idle, stateless proxy) is
killed when the unit finally stops.
Signed-off-by: Semyon Maryasin <simeon@maryasin.name>
g_cancellable_disconnect() must not be called from within the
cancellable callback, or it will deadlock.
Signed-off-by: Aleksander Morgado <aleksandermj@google.com>
Add the ID_MM_MAX_TIMEOUTS udev tag to allow users and system
administrators to configure the maximum number of consecutive
command timeouts before a modem is invalidated and re-probed,
on a per-device basis.
The tag accepts an integer value >= 0, where 0 disables the
timeout monitoring feature entirely. If not set, the existing
default of 10 consecutive timeouts is used.
This is needed for some older modems which become unresponsive
during consecutive data traffic, but come back alive when traffic
is lower. These modems have no further firmware updates available.
This follows the same pattern as other udev-configurable device
properties like ID_MM_MAX_MULTIPLEXED_LINKS.
Signed-off-by: Filip Pokryvka <fpokryvk@redhat.com>
Modem MC7304 has firmware flaw that PCO querry makes it unresponsive (firmware is not upgradable, no new release). There are possibily other older modems affected as well, I have no possibility to check. Add udev rule disabling PCO for affected modem (shares productID with RC7611).
Signed-off-by: Filip Pokryvka <fpokryvk@redhat.com>
The test_creg_match helper would only parse the first match, but actual
code e.g. parse_unsolicited() in src/mm-port-serial-at.c iterates over
all matches:
this updates the test to loop similarily to what is done in real code.
There were two tests assuming only the first match gets matched, so
their result were updated, and a third test was added matching the URC
received from a SIM7672 modem (including unrelated CNSMOD message)
Signed-off-by: Dominique Martinet <dominique.martinet@atmark-techno.com>
When receiving a message from modem containing multiple CREG events that
have different format, the parsing happens in the order CREG regexes are
registered, and not in the order the modem sent the events.
That means that when receiving the following URC:
<CR><LF>+CEREG: 2<CR><LF><CR><LF>+CNSMOD: 8<CR><LF><CR><LF>+CEREG: 1,AAAA,01234567,7<CR><LF>
we'd incorrectly parse the `CEREG: 1` before `CEREG: 2`, and remember
the state as searching (2) instead of home (1), which can cause trouble
connecting because of waiting for state to go home which would never
happen again.
This fixes that problem by merging all the regexes together, so the
parsing should properly process the events in matching order in
parse_unsolicited()
Unfortunately changing the regex like this means that
mm_3gpp_parse_creg_response() match info handling also needs to be
reworked: matches number are increased for each regex, so we can
reliably know which regex was matched based on match count.
This is actually cleaner than the old approach that just guessed if nth
field with "item_is_lac_not_stat()", but requires careful testing.
This was validated checking that tests fail if any of the indices is
changed by iterating over them all and running the test:
grep -n 'n_matches -' src/mm-modem-helpers.c | sed -e 's/:.*//' \
| while read lineno; do
echo $lineno
git show HEAD:src/mm-modem-helpers.c \
| awk -v l=$lineno '{ if (NR == l) {
print $1,$2,$3,$4,$5 - 1,";"
} else {
print
}}' > src/mm-modem-helpers.c
ninja -C build src/tests/test-modem-helpers || break
build/src/tests/test-modem-helpers && break
done
A test case was added to make this work, as the Samsung Wave S8500
sample 'act' doesn't seem to actually be a technology value?
(case 36/42 could actually be merged but were kept separate on purpose
for easier maintenance/testing)
Fixes: #1087
Signed-off-by: Dominique Martinet <dominique.martinet@atmark-techno.com>
Test the fix in MR !1469 that fixes the issue in #840.
If a multi-part PDU is sent from a device to its own number, on receipt
the DELIVER PDU was incorrectly dropped because when comparing the new
parts to existing ones, the comparison found the original SUBMIT PDU.
Signed-off-by: Dan Williams <dan@ioncontrol.co>
The patch resolves a coredump caused by using 'client' instead of 'self'
in mm_obj_warn() call when failing to register tracking events.
Signed-off-by: shuihan-001 <xing1989093@outlook.com>
On my modem, AT!ICCID? command gives response without a preceding `!`:
```
AT!ICCID?
ICCID: 8970162010103256136
OK
```
Signed-off-by: Semyon Maryasin <simeon@maryasin.name>
device_caps_query_ready() declares caps_custom_data_class_str as
g_autofree without an initializer. On every early error return
(command failure such as a transaction timeout or NotOpened, or a
response parse failure) the variable is never assigned, so the
autofree cleanup runs g_free() on uninitialized stack garbage,
crashing ModemManager with a glibc "invalid free" abort (or SIGSEGV)
whenever the device caps query fails during modem initialization.
Fixes: 19c817cfe9 ("broadband-modem-mbim: consistently handle custom data classes")
Closes: #1094
Signed-off-by: Semyon Maryasin <simeon@maryasin.name>
The qcdm wwan device does not cope well with writing single bytes,
while it does not have any issue if longer chunks are written.
Fix qcdm_send accordingly, since it should not hurt the previously
supported devices.
Signed-off-by: Daniele Palmas <dnlplm@gmail.com>
Instead of using CFLAGS="-DMM_DISABLE_DEPRECATED" meson ...
which doesn't handle -Werror very well let's make it an explicit
option. Defaults to true, and autodetects previous CFLAGS
mechanism to preserve backwards compatibility.
Signed-off-by: Dan Williams <dan@ioncontrol.co>
Validate the active, candidate, and neighbor count fields in
qcdm_cmd_pilot_sets_result() before parsing the sets.
Ensure that the sum of counts does not exceed the size of the static
sets array in the response structure, and that the calculated size
based on the counts fits within the received buffer len.
https://gitlab.freedesktop.org/mobile-broadband/ModemManager/-/work_items/1069
Signed-off-by: Aleksander Morgado <aleksandermj@google.com>
Update parse_unsolicited() to re-check handler->callback on each
iteration of the match dispatch loop. If a URC handler callback clears
itself during dispatch (e.g. to stop listening), subsequent matches in
the same buffer would attempt to call the NULL pointer, causing a crash.
https://gitlab.freedesktop.org/mobile-broadband/ModemManager/-/work_items/1074
Signed-off-by: Aleksander Morgado <aleksandermj@google.com>
Use g_steal_pointer() when passing g_autoptr(GError) to
g_task_return_error() to prevent double-free by the automatic cleanup.
GTask takes ownership of the passed GError. If we use g_autoptr(GError)
locally, it will also call g_error_free() when the function returns,
causing a double-free / heap corruption. Stealing the pointer clears the
local variable and prevents the automatic cleanup.
https://gitlab.freedesktop.org/mobile-broadband/ModemManager/-/work_items/1073
Signed-off-by: Aleksander Morgado <aleksandermj@google.com>
Defer the validity change (which triggers modem reprobe/removal) to an
idle callback in rdy_handler().
Calling mm_base_modem_set_valid(FALSE) synchronously from the URC
handler triggers the immediate destruction of the modem object while the
serial port is still iterating the unsolicited message handlers. Deferring
the operation to an idle callback and taking a reference to the modem
ensures the modem object is kept alive until parse_unsolicited() has
returned.
https://gitlab.freedesktop.org/mobile-broadband/ModemManager/-/work_items/1072
Signed-off-by: Aleksander Morgado <aleksandermj@google.com>
Disconnect the 'list-configs' indication handler immediately inside the
callback. If the modem sends a duplicate indication before the whole
config load sequence completes, it could overwrite the stored
'get-config-info' signal handler ID, orphaning the old handler and
leading to a Use-After-Free when the GTask is completed and the
orphaned handler eventually fires.
https://gitlab.freedesktop.org/mobile-broadband/ModemManager/-/work_items/1071
Signed-off-by: Aleksander Morgado <aleksandermj@google.com>
Use g_signal_connect_object() to connect to child list/object signals
in the 3GPP, Messaging, Voice, and Cell Broadcast interfaces.
Passing the raw D-Bus skeleton pointer as user_data to these signals
can lead to a Use-After-Free if the skeleton is disposed of (e.g. on
hot-unplug) while the parent object or child list survives. By using
g_signal_connect_object(), the signal handlers are automatically
disconnected when the D-Bus skeleton is destroyed.
https://gitlab.freedesktop.org/mobile-broadband/ModemManager/-/work_items/1068
Signed-off-by: Aleksander Morgado <aleksandermj@google.com>
Dan Williams
Semyon Maryasin
Aleksander Morgado
Filip Pokryvka
Dominique Martinet
Christian Wendt
shuihan-001
Daniele Palmas
Nikolay Savchenko
Aleksander Morgado
Andrey Skvortsov