diff options
author | Aleksander Morgado <aleksandermj@chromium.org> | 2023-02-21 12:15:23 +0000 |
---|---|---|
committer | Aleksander Morgado <aleksandermj@chromium.org> | 2023-02-24 13:46:44 +0000 |
commit | e553c76b147f1d31ae869404a51699768366f72e (patch) | |
tree | ddbf26109a113656ec658caf0a4d6d92dd43597b | |
parent | 74aadb738923558d41f1c72068ad01c96038f5b0 (diff) | |
download | ModemManager-e553c76b147f1d31ae869404a51699768366f72e.tar.gz |
base-manager: fix crash when uninhibiting partially removed device
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
(cherry picked from commit 7837944e257037bb1d4a9b2220bdc82fc2f5c8bf)
-rw-r--r-- | src/mm-base-manager.c | 59 |
1 files changed, 52 insertions, 7 deletions
diff --git a/src/mm-base-manager.c b/src/mm-base-manager.c index 69f0ecf8d..6ff5ae3fc 100644 --- a/src/mm-base-manager.c +++ b/src/mm-base-manager.c @@ -1105,15 +1105,58 @@ remove_device_inhibition (MMBaseManager *self, info->port_infos = NULL; g_hash_table_remove (self->priv->inhibited_devices, uid); + /* If any port info exists, we require explicit port probing that will be + * triggered via the artificial port notifications emitted with the + * device_added() calls */ if (port_infos) { GList *l; - /* Note that a device can only be inhibited if it had an existing - * modem exposed in the bus. And so, inhibition can only be placed - * AFTER all port probes have finished for a given device. This means - * that we either have a device tracked, or we have a list of port - * infos. Both at the same time should never happen. */ - g_assert (!device); + /* A device may exist at this point if e.g. not all ports were + * removed during the inhibition (i.e. the MMDevice was never fully + * removed) and new ports were then added while inhibited. In this + * case, we must fake all ports going away so that the MMDevice gets + * completely removed, otherwise the plugin manager won't start a new + * device probing task and therefore no port probing tasks. */ + if (device) { + GList *readded_port_infos = NULL; + GList *leftover_ports; + GList *l; + + /* Create a new list of inhibited device port infos from the existing + * port probes */ + leftover_ports = mm_device_peek_port_probe_list (device); + for (l = leftover_ports; l; l = g_list_next (l)) { + InhibitedDevicePortInfo *port_info; + MMPortProbe *port_probe; + + port_probe = MM_PORT_PROBE (l->data); + + port_info = g_slice_new0 (InhibitedDevicePortInfo); + port_info->kernel_port = mm_port_probe_get_port (port_probe); + port_info->manual_scan = TRUE; + readded_port_infos = g_list_append (readded_port_infos, port_info); + } + + /* Now, explicitly request to remove all ports, the device should go + * away as well while doing so. */ + for (l = readded_port_infos; l; l = g_list_next (l)) { + InhibitedDevicePortInfo *port_info; + + port_info = (InhibitedDevicePortInfo *)(l->data); + mm_obj_info (self, "fake releasing port %s/%s during uninhibition...", + mm_kernel_device_get_subsystem (port_info->kernel_port), + mm_kernel_device_get_name (port_info->kernel_port)); + device_removed (self, + mm_kernel_device_get_subsystem (port_info->kernel_port), + mm_kernel_device_get_name (port_info->kernel_port)); + } + + /* At this point, the device should have gone completely */ + g_assert (!find_device_by_physdev_uid (self, uid)); + + /* Added the ports to re-add in the pending list */ + port_infos = g_list_concat (port_infos, readded_port_infos); + } /* Report as added all port infos that we had tracked while the * device was inhibited. We can only report the added port after @@ -1126,10 +1169,12 @@ remove_device_inhibition (MMBaseManager *self, device_added (self, port_info->kernel_port, FALSE, port_info->manual_scan); } g_list_free_full (port_infos, (GDestroyNotify)inhibited_device_port_info_free); + return; } + /* The device may be totally gone from the system while we were * keeping the inhibition, so do not error out if not found. */ - else if (device) { + if (device) { GError *error = NULL; /* Uninhibit device, which will create and expose the modem object */ |