From 48e37de3a4de76a4146d4a716c315031de658e4a Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Mon, 11 Oct 2010 20:30:40 -0500 Subject: [PATCH] supplicant: prevent a race condition due to D-Bus activation interface_add() could get called from two places: by the wifi/eth device class when activating (which if the supplicant isn't yet running will D-Bus activate it) and from the NameOwnerChanged handler for the wpa_supplicant dbus service smgr_running_cb(). So if the supplicant wasn't running, nm_supplicant_interface_new() would call interface_add() to bring the supplicant to life via activation, then go on and create priv->iface_proxy. When the supplicant appeared and D-Bus sent the NameOwnerChanged, smgr_running_cb() would also call interface_add(), creating a second priv->iface_proxy. The first one got lost and lived after its parent NMSupplicantInterface was killed, and could still respond to signals over the bus. Prevent that by adding another state, STARTING, that indicates that we've already started talking to the supplicant. Also be extra paranoid about disconnecting signal handlers on the proxy. --- .../nm-supplicant-interface.c | 51 +++++++++++++++---- .../nm-supplicant-interface.h | 1 + 2 files changed, 42 insertions(+), 10 deletions(-) diff --git a/src/supplicant-manager/nm-supplicant-interface.c b/src/supplicant-manager/nm-supplicant-interface.c index 20cce6de5c..415c46d6a3 100644 --- a/src/supplicant-manager/nm-supplicant-interface.c +++ b/src/supplicant-manager/nm-supplicant-interface.c @@ -40,8 +40,17 @@ #define WPAS_ERROR_EXISTS_ERROR WPAS_DBUS_INTERFACE ".ExistsError" -G_DEFINE_TYPE (NMSupplicantInterface, nm_supplicant_interface, G_TYPE_OBJECT) +static void wpas_iface_handle_state_change (DBusGProxy *proxy, + const char *str_new_state, + const char *str_old_state, + gpointer user_data); +static void wpas_iface_handle_scanning (DBusGProxy *proxy, + gboolean scanning, + gpointer user_data); + + +G_DEFINE_TYPE (NMSupplicantInterface, nm_supplicant_interface, G_TYPE_OBJECT) #define NM_SUPPLICANT_INTERFACE_GET_PRIVATE(o) (G_TYPE_INSTANCE_GET_PRIVATE ((o), \ NM_TYPE_SUPPLICANT_INTERFACE, \ @@ -362,6 +371,23 @@ set_state (NMSupplicantInterface *self, guint32 new_state) g_signal_handler_disconnect (priv->smgr, priv->smgr_running_id); priv->smgr_running_id = 0; } + + if (priv->iface_proxy) { + dbus_g_proxy_disconnect_signal (priv->iface_proxy, + "StateChange", + G_CALLBACK (wpas_iface_handle_state_change), + self); + + dbus_g_proxy_disconnect_signal (priv->iface_proxy, + "ScanResultsAvailable", + G_CALLBACK (wpas_iface_query_scan_results), + self); + + dbus_g_proxy_disconnect_signal (priv->iface_proxy, + "Scanning", + G_CALLBACK (wpas_iface_handle_scanning), + self); + } } priv->state = new_state; @@ -584,7 +610,7 @@ interface_add_cb (DBusGProxy *proxy, } static void -interface_add (NMSupplicantInterface * self, gboolean is_wireless) +interface_add (NMSupplicantInterface *self, gboolean is_wireless) { NMSupplicantInterfacePrivate *priv = NM_SUPPLICANT_INTERFACE_GET_PRIVATE (self); DBusGProxyCall *call; @@ -597,6 +623,9 @@ interface_add (NMSupplicantInterface * self, gboolean is_wireless) nm_log_dbg (LOGD_SUPPLICANT, "(%s): adding interface to supplicant", priv->dev); + /* Move to starting to prevent double-calls of interface_add() */ + set_state (self, NM_SUPPLICANT_INTERFACE_STATE_STARTING); + /* Try to add the interface to the supplicant. If the supplicant isn't * running, this will start it via D-Bus activation and return the response * when the supplicant has started. @@ -1010,6 +1039,8 @@ nm_supplicant_interface_state_to_string (guint32 state) switch (state) { case NM_SUPPLICANT_INTERFACE_STATE_INIT: return "init"; + case NM_SUPPLICANT_INTERFACE_STATE_STARTING: + return "starting"; case NM_SUPPLICANT_INTERFACE_STATE_READY: return "ready"; case NM_SUPPLICANT_INTERFACE_STATE_DISCONNECTED: @@ -1158,6 +1189,13 @@ dispose (GObject *object) } priv->disposed = TRUE; + /* Cancel pending calls before unrefing the dbus manager */ + cancel_all_callbacks (priv->other_pcalls); + nm_call_store_destroy (priv->other_pcalls); + + cancel_all_callbacks (priv->assoc_pcalls); + nm_call_store_destroy (priv->assoc_pcalls); + if (priv->iface_proxy) g_object_unref (priv->iface_proxy); @@ -1178,13 +1216,6 @@ dispose (GObject *object) g_free (priv->dev); - /* Cancel pending calls before unrefing the dbus manager */ - cancel_all_callbacks (priv->other_pcalls); - nm_call_store_destroy (priv->other_pcalls); - - cancel_all_callbacks (priv->assoc_pcalls); - nm_call_store_destroy (priv->assoc_pcalls); - if (priv->dbus_mgr) g_object_unref (priv->dbus_mgr); @@ -1212,7 +1243,7 @@ nm_supplicant_interface_class_init (NMSupplicantInterfaceClass *klass) g_object_class_install_property (object_class, PROP_STATE, g_param_spec_uint ("state", "State", - "State of the supplicant interface; INIT, READY, or DOWN", + "State of the supplicant interface", NM_SUPPLICANT_INTERFACE_STATE_INIT, NM_SUPPLICANT_INTERFACE_STATE_LAST - 1, NM_SUPPLICANT_INTERFACE_STATE_INIT, diff --git a/src/supplicant-manager/nm-supplicant-interface.h b/src/supplicant-manager/nm-supplicant-interface.h index d6e4a55969..8c51caa1d9 100644 --- a/src/supplicant-manager/nm-supplicant-interface.h +++ b/src/supplicant-manager/nm-supplicant-interface.h @@ -32,6 +32,7 @@ */ enum { NM_SUPPLICANT_INTERFACE_STATE_INIT = 0, + NM_SUPPLICANT_INTERFACE_STATE_STARTING, NM_SUPPLICANT_INTERFACE_STATE_READY, NM_SUPPLICANT_INTERFACE_STATE_DISCONNECTED, NM_SUPPLICANT_INTERFACE_STATE_INACTIVE,