summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAleksander Morgado <aleksandermj@chromium.org>2023-02-21 12:15:23 +0000
committerAleksander Morgado <aleksandermj@chromium.org>2023-02-24 13:46:44 +0000
commite553c76b147f1d31ae869404a51699768366f72e (patch)
treeddbf26109a113656ec658caf0a4d6d92dd43597b
parent74aadb738923558d41f1c72068ad01c96038f5b0 (diff)
downloadModemManager-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.c59
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 */