core: protect against modem removal in critical callbacks (rh #553953)

There are more places to handle, but these are the most critical.  If
the modem is removed while a command is in-progress, the mm-callback-info
code will set info->modem to NULL.  Make sure we check for that in
callbacks and return a reasonable error.  Previous code would just
blindly forge ahead and die on a null dereference.

src/mm-callback-info.c

@@ -11,6 +11,7 @@
  * GNU General Public License for more details:
  *
  * Copyright (C) 2008 Novell, Inc.
+ * Copyright (C) 2009 - 2010 Red Hat, Inc.
  */
 
 #include "mm-callback-info.h"
@@ -55,8 +56,8 @@ modem_destroyed_cb (gpointer data, GObject *destroyed)
     info->modem = NULL;
     if (!info->pending_id) {
         info->error = g_error_new_literal (MM_MODEM_ERROR,
-                                           MM_MODEM_ERROR_GENERAL,
+                                           MM_MODEM_ERROR_REMOVED,
-                                           "The modem was removed or disabled.");
+                                           "The modem was removed.");
         mm_callback_info_schedule (info);
     }
 }

src/mm-errors.c

@@ -11,7 +11,7 @@
  * GNU General Public License for more details:
  *
  * Copyright (C) 2008 Novell, Inc.
- * Copyright (C) 2009 Red Hat, Inc.
+ * Copyright (C) 2009 - 2010 Red Hat, Inc.
  */
 
 #include "mm-errors.h"
@@ -72,6 +72,7 @@ mm_modem_error_get_type (void)
             ENUM_ENTRY (MM_MODEM_ERROR_CONNECTED,               "Connected"),
             ENUM_ENTRY (MM_MODEM_ERROR_DISCONNECTED,            "Disconnected"),
             ENUM_ENTRY (MM_MODEM_ERROR_OPERATION_IN_PROGRESS,   "OperationInProgress"),
+            ENUM_ENTRY (MM_MODEM_ERROR_REMOVED,                 "Removed"),
             { 0, 0, 0 }
         };
 

src/mm-errors.h

@@ -38,7 +38,8 @@ enum {
     MM_MODEM_ERROR_OPERATION_NOT_SUPPORTED = 1,
     MM_MODEM_ERROR_CONNECTED = 2,
     MM_MODEM_ERROR_DISCONNECTED = 3,
-    MM_MODEM_ERROR_OPERATION_IN_PROGRESS = 4
+    MM_MODEM_ERROR_OPERATION_IN_PROGRESS = 4,
+    MM_MODEM_ERROR_REMOVED = 5
 };
 
 #define MM_MODEM_ERROR (mm_modem_error_quark ())

src/mm-generic-cdma.c

@@ -11,7 +11,7 @@
  * GNU General Public License for more details:
  *
  * Copyright (C) 2008 - 2009 Novell, Inc.
- * Copyright (C) 2009 Red Hat, Inc.
+ * Copyright (C) 2009 - 2010 Red Hat, Inc.
  */
 
 #include <string.h>
@@ -478,13 +478,15 @@ static void
 disable_all_done (MMModem *modem, GError *error, gpointer user_data)
 {
     MMCallbackInfo *info = user_data;
-    MMGenericCdma *self = MM_GENERIC_CDMA (info->modem);
-    MMGenericCdmaPrivate *priv = MM_GENERIC_CDMA_GET_PRIVATE (self);
 
-    if (error) {
+    info->error = mm_modem_check_removed (modem, error);
-        disable_set_previous_state (MM_MODEM (modem), info);
+    if (info->error) {
-        info->error = g_error_copy (error);
+        if (modem)
+            disable_set_previous_state (modem, info);
     } else {
+        MMGenericCdma *self = MM_GENERIC_CDMA (info->modem);
+        MMGenericCdmaPrivate *priv = MM_GENERIC_CDMA_GET_PRIVATE (self);
+
         mm_serial_port_close (priv->primary);
         mm_modem_set_state (modem, MM_MODEM_STATE_DISABLED, MM_MODEM_STATE_REASON_NONE);
 
@@ -501,18 +503,22 @@ disable_flash_done (MMSerialPort *port,
                     gpointer user_data)
 {
     MMCallbackInfo *info = user_data;
-    MMGenericCdma *self = MM_GENERIC_CDMA (info->modem);
+    MMGenericCdma *self;
 
-    if (error) {
+    info->error = mm_modem_check_removed (info->modem, error);
-        disable_set_previous_state (info->modem, info);
+    if (info->error) {
-        info->error = g_error_copy (error);
+        if (info->modem)
+            disable_set_previous_state (info->modem, info);
         mm_callback_info_schedule (info);
-    } else {
+        return;
-        if (MM_GENERIC_CDMA_GET_CLASS (self)->post_disable)
-            MM_GENERIC_CDMA_GET_CLASS (self)->post_disable (self, disable_all_done, info);
-        else
-            disable_all_done (MM_MODEM (self), NULL, info);
     }
+
+    self = MM_GENERIC_CDMA (info->modem);
+
+    if (MM_GENERIC_CDMA_GET_CLASS (self)->post_disable)
+        MM_GENERIC_CDMA_GET_CLASS (self)->post_disable (self, disable_all_done, info);
+    else
+        disable_all_done (MM_MODEM (self), NULL, info);
 }
 
 static void
@@ -557,13 +563,15 @@ dial_done (MMSerialPort *port,
            gpointer user_data)
 {
     MMCallbackInfo *info = (MMCallbackInfo *) user_data;
-    MMGenericCdma *self = MM_GENERIC_CDMA (info->modem);
-    MMGenericCdmaPrivate *priv = MM_GENERIC_CDMA_GET_PRIVATE (self);;
 
-    if (error) {
+    info->error = mm_modem_check_removed (info->modem, error);
-        info->error = g_error_copy (error);
+    if (info->error) {
-        update_enabled_state (MM_GENERIC_CDMA (info->modem), FALSE, MM_MODEM_STATE_REASON_NONE);
+        if (info->modem)
+            update_enabled_state (MM_GENERIC_CDMA (info->modem), FALSE, MM_MODEM_STATE_REASON_NONE);
     } else {
+        MMGenericCdma *self = MM_GENERIC_CDMA (info->modem);
+        MMGenericCdmaPrivate *priv = MM_GENERIC_CDMA_GET_PRIVATE (self);
+
         /* Clear reg tries; we're obviously registered by this point */
         registration_cleanup (self, 0, 0);
 
@@ -598,25 +606,22 @@ disconnect_flash_done (MMSerialPort *port,
                        gpointer user_data)
 {
     MMCallbackInfo *info = (MMCallbackInfo *) user_data;
-    MMGenericCdmaPrivate *priv = MM_GENERIC_CDMA_GET_PRIVATE (info->modem);
+    MMModemState prev_state;
 
-    if (error) {
+    info->error = mm_modem_check_removed (info->modem, error);
-        MMModemState prev_state;
+    if (info->error) {
-
+        if (info->modem) {
-        /* Reset old state since the operation failed */
+            /* Reset old state since the operation failed */
-        prev_state = GPOINTER_TO_UINT (mm_callback_info_get_data (info, MM_GENERIC_CDMA_PREV_STATE_TAG));
+            prev_state = GPOINTER_TO_UINT (mm_callback_info_get_data (info, MM_GENERIC_CDMA_PREV_STATE_TAG));
-        mm_modem_set_state (MM_MODEM (info->modem),
+            mm_modem_set_state (MM_MODEM (info->modem),
-                            prev_state,
+                                prev_state,
-                            MM_MODEM_STATE_REASON_NONE);
+                                MM_MODEM_STATE_REASON_NONE);
-
+        }
-        info->error = g_error_copy (error);
+    } else {
-        mm_callback_info_schedule (info);
+        mm_port_set_connected (MM_GENERIC_CDMA_GET_PRIVATE (info->modem)->data, FALSE);
-        return;
+        update_enabled_state (MM_GENERIC_CDMA (info->modem), FALSE, MM_MODEM_STATE_REASON_NONE);
     }
 
-    mm_port_set_connected (priv->data, FALSE);
-    update_enabled_state (MM_GENERIC_CDMA (info->modem), FALSE, MM_MODEM_STATE_REASON_NONE);
-
     mm_callback_info_schedule (info);
 }
 

src/mm-generic-gsm.c

@@ -11,7 +11,7 @@
  * GNU General Public License for more details:
  *
  * Copyright (C) 2008 - 2009 Novell, Inc.
- * Copyright (C) 2009 Red Hat, Inc.
+ * Copyright (C) 2009 - 2010 Red Hat, Inc.
  * Copyright (C) 2009 Ericsson
  */
 
@@ -538,13 +538,17 @@ disable_done (MMSerialPort *port,
               gpointer user_data)
 {
     MMCallbackInfo *info = user_data;
-    MMGenericGsmPrivate *priv = MM_GENERIC_GSM_GET_PRIVATE (info->modem);
 
-    mm_serial_port_close (port);
+    info->error = mm_modem_check_removed (info->modem, error);
-    mm_modem_set_state (MM_MODEM (info->modem),
+    if (!info->error) {
-                        MM_MODEM_STATE_DISABLED,
+        MMGenericGsmPrivate *priv = MM_GENERIC_GSM_GET_PRIVATE (info->modem);
-                        MM_MODEM_STATE_REASON_NONE);
+
-    priv->reg_status = MM_MODEM_GSM_NETWORK_REG_STATUS_UNKNOWN;
+        mm_serial_port_close (port);
+        mm_modem_set_state (MM_MODEM (info->modem),
+                            MM_MODEM_STATE_DISABLED,
+                            MM_MODEM_STATE_REASON_NONE);
+        priv->reg_status = MM_MODEM_GSM_NETWORK_REG_STATUS_UNKNOWN;
+    }
     mm_callback_info_schedule (info);
 }
 
@@ -554,18 +558,19 @@ disable_flash_done (MMSerialPort *port,
                     gpointer user_data)
 {
     MMCallbackInfo *info = user_data;
+    MMModemState prev_state;
     char *cmd = NULL;
 
-    if (error) {
+    info->error = mm_modem_check_removed (info->modem, error);
-        MMModemState prev_state;
+    if (info->error) {
+        if (info->modem) {
+            /* Reset old state since the operation failed */
+            prev_state = GPOINTER_TO_UINT (mm_callback_info_get_data (info, MM_GENERIC_GSM_PREV_STATE_TAG));
+            mm_modem_set_state (MM_MODEM (info->modem),
+                                prev_state,
+                                MM_MODEM_STATE_REASON_NONE);
+        }
 
-        /* Reset old state since the operation failed */
-        prev_state = GPOINTER_TO_UINT (mm_callback_info_get_data (info, MM_GENERIC_GSM_PREV_STATE_TAG));
-        mm_modem_set_state (MM_MODEM (info->modem),
-                            prev_state,
-                            MM_MODEM_STATE_REASON_NONE);
-
-        info->error = g_error_copy (error);
         mm_callback_info_schedule (info);
         return;
     }
@@ -1374,25 +1379,25 @@ disconnect_flash_done (MMSerialPort *port,
                        gpointer user_data)
 {
     MMCallbackInfo *info = (MMCallbackInfo *) user_data;
-    MMGenericGsm *self = MM_GENERIC_GSM (info->modem);
+    MMModemState prev_state;
-    MMGenericGsmPrivate *priv = MM_GENERIC_GSM_GET_PRIVATE (self);
 
-    if (error) {
+    info->error = mm_modem_check_removed (info->modem, error);
-        MMModemState prev_state;
+    if (info->error) {
+        if (info->modem) {
+            /* Reset old state since the operation failed */
+            prev_state = GPOINTER_TO_UINT (mm_callback_info_get_data (info, MM_GENERIC_GSM_PREV_STATE_TAG));
+            mm_modem_set_state (MM_MODEM (info->modem),
+                                prev_state,
+                                MM_MODEM_STATE_REASON_NONE);
+        }
+    } else {
+        MMGenericGsm *self = MM_GENERIC_GSM (info->modem);
+        MMGenericGsmPrivate *priv = MM_GENERIC_GSM_GET_PRIVATE (self);
 
-        /* Reset old state since the operation failed */
+        mm_port_set_connected (priv->data, FALSE);
-        prev_state = GPOINTER_TO_UINT (mm_callback_info_get_data (info, MM_GENERIC_GSM_PREV_STATE_TAG));
+        mm_generic_gsm_update_enabled_state (self, FALSE, MM_MODEM_STATE_REASON_NONE);
-        mm_modem_set_state (MM_MODEM (info->modem),
-                            prev_state,
-                            MM_MODEM_STATE_REASON_NONE);
-
-        info->error = g_error_copy (error);
-        mm_callback_info_schedule (info);
-        return;
     }
 
-    mm_port_set_connected (priv->data, FALSE);
-    mm_generic_gsm_update_enabled_state (self, FALSE, MM_MODEM_STATE_REASON_NONE);
     mm_callback_info_schedule (info);
 }
 

src/mm-modem.c

@@ -11,7 +11,7 @@
  * GNU General Public License for more details:
  *
  * Copyright (C) 2008 - 2009 Novell, Inc.
- * Copyright (C) 2009 Red Hat, Inc.
+ * Copyright (C) 2009 - 2010 Red Hat, Inc.
  */
 
 #include <string.h>
@@ -29,6 +29,28 @@ static void impl_modem_get_info (MMModem *modem, DBusGMethodInvocation *context)
 
 #include "mm-modem-glue.h"
 
+/* Should be used from callbacks to check whether the modem was removed after
+ * the callback's operation was started, but before the callback itself was
+ * called, in which case the MMModem passed to the callback is NULL.
+ */
+GError *
+mm_modem_check_removed (MMModem *self, const GError *error)
+{
+    if (g_error_matches (error, MM_MODEM_ERROR, MM_MODEM_ERROR_REMOVED))
+        return g_error_copy (error);
+
+    if (!self) {
+        /* If the modem was NULL, the error *should* have been
+         * MM_MODEM_ERROR_REMOVED.  If it wasn't, make it that.
+         */
+        return g_error_new_literal (MM_MODEM_ERROR,
+                                    MM_MODEM_ERROR_REMOVED,
+                                    "The modem was removed.");
+    }
+
+    return NULL;
+}
+
 static void
 async_op_not_supported (MMModem *self,
                         MMModemFn callback,
@@ -92,7 +114,6 @@ finish_disable (MMModem *self,
                 MMModemFn callback,
                 gpointer user_data)
 {
-
     if (MM_MODEM_GET_INTERFACE (self)->disable)
         MM_MODEM_GET_INTERFACE (self)->disable (self, callback, user_data);
     else
@@ -110,9 +131,27 @@ disable_disconnect_done (MMModem *self,
                          gpointer user_data)
 {
     DisableDisconnectInfo *cb_data = user_data;
+    GError *tmp_error = NULL;
+
+    /* Check for modem removal */
+    if (g_error_matches (error, MM_MODEM_ERROR, MM_MODEM_ERROR_REMOVED))
+        tmp_error = g_error_copy (error);
+    else if (!self) {
+        tmp_error = g_error_new_literal (MM_MODEM_ERROR,
+                                         MM_MODEM_ERROR_REMOVED,
+                                         "The modem was removed.");
+    }
+
+    /* And send an immediate error reply if the modem was removed */
+    if (tmp_error) {
+        cb_data->callback (NULL, tmp_error, cb_data->user_data);
+        g_free (cb_data);
+        g_error_free (tmp_error);
+        return;
+    }
 
-    /* ignore errors */
     if (error) {
+        /* Don't really care what the error was; log it and proceed to disable */
         g_warning ("%s: (%s): error disconnecting the modem while disabling: (%d) %s",
                    __func__,
                    mm_modem_get_device (self),
@@ -337,7 +376,6 @@ mm_modem_disconnect (MMModem *self,
         return;
     }
 
-
     if (MM_MODEM_GET_INTERFACE (self)->disconnect)
         MM_MODEM_GET_INTERFACE (self)->disconnect (self, callback, user_data);
     else

src/mm-modem.h

@@ -213,5 +213,7 @@ void mm_modem_set_state (MMModem *self,
                          MMModemState new_state,
                          MMModemStateReason reason);
 
+GError *mm_modem_check_removed (MMModem *self, const GError *error);
+
 #endif  /* MM_MODEM_H */