Bug 99160

Created attachment 128587 [details] [review]
patch

Hi,

the SIM hot swap is not working anymore and a crash occurs when the SIM is inserted/removed

ModemManager[28465]: <info>  ModemManager (version 1.7.0) starting in system bus...
ModemManager[28465]: <info>  Couldn't check support for device '/sys/devices/pci0000:00/0000:00:19.0': not supported by any plugin
ModemManager[28465]: <info>  [device /sys/devices/pci0000:00/0000:00:16.3] creating modem with plugin 'Generic' and '1' ports
ModemManager[28465]: <warn>  Could not grab port (tty/ttyS4): 'Cannot add port 'tty/ttyS4', unhandled serial type'
ModemManager[28465]: <warn>  Couldn't create modem for device '/sys/devices/pci0000:00/0000:00:16.3': Failed to find primary AT port
ModemManager[28465]: <info>  [device /sys/devices/pci0000:00/0000:00:14.0/usb3/3-4] creating modem with plugin 'Telit' and '6' ports
ModemManager[28465]: <info>  Modem for device '/sys/devices/pci0000:00/0000:00:14.0/usb3/3-4' successfully created
ModemManager[28465]: <warn>  Parse error in step 3: Could not parse reponse '+CSIM: 4,"6A86"'.
ModemManager[28465]: <warn>  Parse error in step 4: Could not parse reponse '+CSIM: 4,"6A86"'.
ModemManager[28465]: <warn>  couldn't load list of Own Numbers: 'Not found'
ModemManager[28465]: <info>  Modem: state changed (unknown -> disabled)
ModemManager[28465]: <info>  Modem /org/freedesktop/ModemManager1/Modem/0: state changed (disabled -> enabling)
ModemManager[28465]: <info>  Modem /org/freedesktop/ModemManager1/Modem/0: 3GPP Registration state changed (unknown -> registering)
ModemManager[28465]: <info>  Modem /org/freedesktop/ModemManager1/Modem/0: 3GPP Registration state changed (registering -> home)
ModemManager[28465]: <info>  Modem /org/freedesktop/ModemManager1/Modem/0: state changed (enabling -> registered)
ModemManager[28465]: <info>  QSS: SIM removed
ModemManager[28465]: <info>  Modem /org/freedesktop/ModemManager1/Modem/0: state changed (registered -> disabling)
ModemManager[28465]: <info>  Modem /org/freedesktop/ModemManager1/Modem/0: 3GPP Registration state changed (home -> unknown)
ModemManager[28465]: <info>  Modem /org/freedesktop/ModemManager1/Modem/0: state changed (disabling -> disabled)
ModemManager[28465]: <info>  [device /sys/devices/pci0000:00/0000:00:14.0/usb3/3-4] creating modem with plugin 'Telit' and '6' ports

(ModemManager:28465): GLib-GObject-CRITICAL **: g_object_ref: assertion 'G_IS_OBJECT (object)' failed
ModemManager[28465]: <warn>  couldn't load Supported IP families: 'SIM not inserted'
ModemManager[28465]: <info>  Modem: state changed (unknown -> failed)
ModemManager[28465]: <warn>  Modem couldn't be initialized: Couldn't check unlock status: SIM not inserted
ModemManager[28465]: <info>  SIM is missing, but the modem supports SIM hot swap. Waiting for SIM...
**
ERROR:mm-device.c:249:export_modem: assertion failed: (G_IS_DBUS_OBJECT_MANAGER (self->priv->object_manager))
Aborted (core dumped)


I think that the problem is in this part of the code

```
   /* Modem no longer valid */
   mm_device_remove_modem (self);

   if (mm_base_modem_get_reprobe (modem)) {
       GError *error = NULL;

       if (!mm_device_create_modem (self, self->priv->object_manager, &error)) {
```

where mm_device_create_modem is fed with a `self->priv->object_manager` just cleared out by
`mm_device_remove_modem`. This change has been committed some time ago, but
apparently later changes made this bug appear.

I say in advance that the original solution (below) was not correct. Probably it worked because
the instance of `object_manager` was still in memory when used even after the clear.

```
   GDBusObjectManagerServer *object_manager = self->priv->object_manager;
   /* Modem no longer valid */
   mm_device_remove_modem (self);

   if (mm_base_modem_get_reprobe (modem)) {
       GError *error = NULL;

       if (!mm_device_create_modem (self, object_manager , &error)) {
```

Assuming that to use the same `object_manager` for the newly created modem is correct (since the
device is the same), I would solve this crash with the following patch.

---

From a32a0e494b50c9b29e99368b9077844b1e0c1358 Mon Sep 17 00:00:00 2001
From: Carlo Lobrano <c.lobrano@gmail.com>
Date: Tue, 20 Dec 2016 12:34:01 +0100
Subject: [PATCH] Fixed crash in SIM hot swap when removing the SIM

mm_device_create_modem needs a valid object_manager instance to create the new
modem, while the one provided before was cleared out by a call to
mm_device_remove_modem few lines before.

Since the newly created modem refers to the same HW device, we can use
the same object_manager also.

---
 src/mm-device.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/src/mm-device.c b/src/mm-device.c
index 979c397..7e01cae 100644
--- a/src/mm-device.c
+++ b/src/mm-device.c
@@ -316,21 +316,25 @@ modem_valid (MMBaseModem *modem,
              MMDevice    *self)
 {
     if (!mm_base_modem_get_valid (modem)) {
+        GDBusObjectManagerServer *object_manager = g_object_ref (self->priv->object_manager);
+
         /* Modem no longer valid */
         mm_device_remove_modem (self);
 
         if (mm_base_modem_get_reprobe (modem)) {
             GError *error = NULL;
 
-            if (!mm_device_create_modem (self, self->priv->object_manager, &error)) {
-                 mm_warn ("Could not recreate modem for device '%s': %s",
-                          self->priv->uid,
-                          error ? error->message : "unknown");
+            if (!mm_device_create_modem (self, object_manager, &error)) {
+                mm_warn ("Could not recreate modem for device '%s': %s",
+                         self->priv->uid,
+                         error ? error->message : "unknown");
                 g_error_free (error);
             } else {
                 mm_dbg ("Modem recreated for device '%s'", self->priv->uid);
             }
         }
+
+        g_object_unref (object_manager);
     } else {
         /* Modem now valid, export it, but only if we really have it around.
          * It may happen that the initialization sequence fails because the
-- 
2.7.4