Commit 48e37de3 authored by Dan Williams's avatar Dan Williams
Browse files

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.
parent 5858c610
......@@ -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,
......
......@@ -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,
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment