wifi: improve fix to avoid device double release on disable

Per MEEGO-25999 <https://bugs.meego.com/show_bug.cgi?id=25999>, there
exists a race between scan_callback and wifi_disable such that a care-
fully-timed deassertion of the technology Powered property between
throw_wifi_scan and scan_callback will lead to a device reference
count underflow. A second Powered property false -> true -> false
transition will then deallocate the device and Wi-Fi will no longer
function until connman or the system is restarted.

This patch improves upon and revises the prior patch to address this
issue by recognizing that hidden networks involve a slightly different
code path and timing and that both this and the prior case can be
addressed by tracking the device scanning state and only releasing the
device when the state is asserted.

1 files changed, 11 insertions, 35 deletions

diff --git a/plugins/wifi.c b/plugins/wifi.c
index f8627031..07180d68 100644
--- a/plugins/wifi.c
+++ b/plugins/wifi.c
@@ -93,7 +93,6 @@ struct wifi_data {
 	GSList *networks;
 	GSupplicantInterface *interface;
 	GSupplicantState state;
-	connman_bool_t disabling;
 	connman_bool_t connected;
 	connman_bool_t disconnecting;
 	connman_bool_t tethering;
@@ -172,7 +171,6 @@ static int wifi_probe(struct connman_device *device)
 	if (wifi == NULL)
 		return -ENOMEM;
 
-	wifi->disabling = FALSE;
 	wifi->connected = FALSE;
 	wifi->disconnecting = FALSE;
 	wifi->tethering = FALSE;
@@ -512,6 +510,7 @@ static void scan_callback(int result, GSupplicantInterface *interface,
 {
 	struct connman_device *device = user_data;
 	struct wifi_data *wifi = connman_device_get_data(device);
+	connman_bool_t scanning;
 
 	DBG("result %d wifi %p", result, wifi);
 
@@ -524,18 +523,22 @@ static void scan_callback(int result, GSupplicantInterface *interface,
 	if (result < 0)
 		connman_device_reset_scanning(device);
 
-	connman_device_set_scanning(device, FALSE);
+	scanning = connman_device_get_scanning(device);
+
+	if (scanning == TRUE)
+		connman_device_set_scanning(device, FALSE);
 
 	if (result != -ENOLINK)
 		start_autoscan(device);
 
 	/*
-	 * If we are here then we were scanning; however, if we are also
-	 * mid-flight disabling the interface, then wifi_disable has
-	 * already unreferenced the device and we needn't do it here.
+	 * If we are here then we were scanning; however, if we are
+	 * also mid-flight disabling the interface, then wifi_disable
+	 * has already cleared the device scanning state and
+	 * unreferenced the device, obviating the need to do it here.
 	 */
 
-	if (wifi->disabling != TRUE)
+	if (scanning == TRUE)
 		connman_device_unref(device);
 }
 
@@ -733,27 +736,6 @@ static void interface_create_callback(int result,
 	}
 }
 
-/*
- * The sole function of this callback is to avoid a race between scan completion
- * and wifi_disable that can otherwise cause a reference count underflow if the
- * disabling state is not tracked and observed.
- */
-static void interface_remove_callback(int result,
-					GSupplicantInterface *interface,
-							void *user_data)
-{
-	struct wifi_data *wifi = user_data;
-
-	DBG("result %d ifname %s, wifi %p", result,
-				g_supplicant_interface_get_ifname(interface),
-				wifi);
-
-	if (result < 0 || wifi == NULL)
-		return;
-
-	wifi->disabling = FALSE;
-}
-
 static int wifi_enable(struct connman_device *device)
 {
 	struct wifi_data *wifi = connman_device_get_data(device);
@@ -772,8 +754,6 @@ static int wifi_enable(struct connman_device *device)
 	if (ret < 0)
 		return ret;
 
-	wifi->disabling = FALSE;
-
 	return -EINPROGRESS;
 }
 
@@ -803,14 +783,10 @@ static int wifi_disable(struct connman_device *device)
 
 	remove_networks(device, wifi);
 
-	ret = g_supplicant_interface_remove(wifi->interface,
-						interface_remove_callback,
-							wifi);
+	ret = g_supplicant_interface_remove(wifi->interface, NULL, NULL);
 	if (ret < 0)
 		return ret;
 
-	wifi->disabling = TRUE;
-
 	return -EINPROGRESS;
 }