From 50ad39b28e61adb3d9da178c47e41100f554adeb Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Tue, 19 Jan 2010 03:16:34 -0800 Subject: 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 | 5 ++-- src/mm-errors.c | 3 +- src/mm-errors.h | 3 +- src/mm-generic-cdma.c | 77 +++++++++++++++++++++++++++----------------------- src/mm-generic-gsm.c | 69 +++++++++++++++++++++++--------------------- src/mm-modem.c | 46 +++++++++++++++++++++++++++--- src/mm-modem.h | 2 ++ 7 files changed, 129 insertions(+), 76 deletions(-) (limited to 'src') diff --git a/src/mm-callback-info.c b/src/mm-callback-info.c index b554f79b..1882898a 100644 --- a/src/mm-callback-info.c +++ b/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, - "The modem was removed or disabled."); + MM_MODEM_ERROR_REMOVED, + "The modem was removed."); mm_callback_info_schedule (info); } } diff --git a/src/mm-errors.c b/src/mm-errors.c index 6448b807..34f56c19 100644 --- a/src/mm-errors.c +++ b/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 } }; diff --git a/src/mm-errors.h b/src/mm-errors.h index 4bf8ecad..c02a351b 100644 --- a/src/mm-errors.h +++ b/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 ()) diff --git a/src/mm-generic-cdma.c b/src/mm-generic-cdma.c index 7122c483..e3c6004f 100644 --- a/src/mm-generic-cdma.c +++ b/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 @@ -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) { - disable_set_previous_state (MM_MODEM (modem), info); - info->error = g_error_copy (error); + info->error = mm_modem_check_removed (modem, error); + if (info->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) { - disable_set_previous_state (info->modem, info); - info->error = g_error_copy (error); + info->error = mm_modem_check_removed (info->modem, error); + if (info->error) { + if (info->modem) + disable_set_previous_state (info->modem, info); mm_callback_info_schedule (info); - } else { - 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); + return; } + + 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 = g_error_copy (error); - update_enabled_state (MM_GENERIC_CDMA (info->modem), FALSE, MM_MODEM_STATE_REASON_NONE); + info->error = mm_modem_check_removed (info->modem, error); + if (info->error) { + 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); - - if (error) { - MMModemState prev_state; - - /* Reset old state since the operation failed */ - 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), - prev_state, - MM_MODEM_STATE_REASON_NONE); + MMModemState prev_state; - info->error = g_error_copy (error); - mm_callback_info_schedule (info); - return; + info->error = mm_modem_check_removed (info->modem, error); + 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_CDMA_PREV_STATE_TAG)); + mm_modem_set_state (MM_MODEM (info->modem), + prev_state, + MM_MODEM_STATE_REASON_NONE); + } + } else { + mm_port_set_connected (MM_GENERIC_CDMA_GET_PRIVATE (info->modem)->data, FALSE); + 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); } diff --git a/src/mm-generic-gsm.c b/src/mm-generic-gsm.c index 25c6c1ee..4954ca1b 100644 --- a/src/mm-generic-gsm.c +++ b/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); - 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; + info->error = mm_modem_check_removed (info->modem, error); + if (!info->error) { + MMGenericGsmPrivate *priv = MM_GENERIC_GSM_GET_PRIVATE (info->modem); + + 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) { - MMModemState prev_state; - - /* 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 = mm_modem_check_removed (info->modem, error); + 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); + } - 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); - MMGenericGsmPrivate *priv = MM_GENERIC_GSM_GET_PRIVATE (self); - - if (error) { - MMModemState prev_state; - - /* 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); + MMModemState prev_state; + + info->error = mm_modem_check_removed (info->modem, error); + 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); - 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_port_set_connected (priv->data, FALSE); - mm_generic_gsm_update_enabled_state (self, FALSE, MM_MODEM_STATE_REASON_NONE); mm_callback_info_schedule (info); } diff --git a/src/mm-modem.c b/src/mm-modem.c index 5f0f5fac..27b77954 100644 --- a/src/mm-modem.c +++ b/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 @@ -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 diff --git a/src/mm-modem.h b/src/mm-modem.h index b59525e2..e8dd7ea2 100644 --- a/src/mm-modem.h +++ b/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 */ -- cgit v1.2.3-70-g09d2