diff mbox

[2/3] wpa_supplicant: Complete radio works on disable event

Message ID 1392283441-13612-2-git-send-email-ilan.peer@intel.com
State Accepted
Headers show

Commit Message

Ilan Peer Feb. 13, 2014, 9:24 a.m. UTC 
From: Andrei Otcheretianski <andrei.otcheretianski@intel.com>

While testing rfkill blocking of a scanning interface, it
was seen that the ongoing scan never completes. This happens
since EVENT_SCAN_RESULTS is discarded on a disabled interface.

Fix this and also other possible radio work completion issues
by removing all the radio works (including started) of the
disabled interface.

To be able to remove already started radio works, make their
callbacks be reentrant with deinit flag (when the work
is started), so each radio work should be able to handle
it's own termination.

Signed-hostap: Andrei Otcheretianski <andrei.otcheretianski@intel.com>
---
 wpa_supplicant/ctrl_iface.c       |    6 ++++-
 wpa_supplicant/events.c           |    2 ++
 wpa_supplicant/gas_query.c        |    6 +++++
 wpa_supplicant/p2p_supplicant.c   |   47 ++++++++++++++++++++++++-------------
 wpa_supplicant/scan.c             |    8 ++++++-
 wpa_supplicant/sme.c              |    3 +++
 wpa_supplicant/wpa_supplicant.c   |   38 +++++++++++++++++++++++-------
 wpa_supplicant/wpa_supplicant_i.h |    4 ++--
 8 files changed, 86 insertions(+), 28 deletions(-)

Comments

Jouni Malinen Feb. 15, 2014, 10:30 p.m. UTC | #1 
On Thu, Feb 13, 2014 at 11:24:00AM +0200, Ilan Peer wrote:
> While testing rfkill blocking of a scanning interface, it
> was seen that the ongoing scan never completes. This happens
> since EVENT_SCAN_RESULTS is discarded on a disabled interface.
> 
> Fix this and also other possible radio work completion issues
> by removing all the radio works (including started) of the
> disabled interface.
> 
> To be able to remove already started radio works, make their
> callbacks be reentrant with deinit flag (when the work
> is started), so each radio work should be able to handle
> it's own termination.

Thanks, applied. I saw this in my hwsim test runs today as well (when a
P2P group interface was removed while a scan was in progress).
Ilan Peer Feb. 16, 2014, 7:21 a.m. UTC | #2 
> On Thu, Feb 13, 2014 at 11:24:00AM +0200, Ilan Peer wrote:
> > While testing rfkill blocking of a scanning interface, it was seen
> > that the ongoing scan never completes. This happens since
> > EVENT_SCAN_RESULTS is discarded on a disabled interface.
> >
> > Fix this and also other possible radio work completion issues by
> > removing all the radio works (including started) of the disabled
> > interface.
> >
> > To be able to remove already started radio works, make their callbacks
> > be reentrant with deinit flag (when the work is started), so each
> > radio work should be able to handle it's own termination.
> 
> Thanks, applied. I saw this in my hwsim test runs today as well (when a P2P
> group interface was removed while a scan was in progress).
> 

Great (we were not 100 % that this is the best approach ... :)). 

Thanks,

Ilan.
diff mbox

Patch

diff --git a/wpa_supplicant/ctrl_iface.c b/wpa_supplicant/ctrl_iface.c
index 4047b64..bc1165d 100644
--- a/wpa_supplicant/ctrl_iface.c
+++ b/wpa_supplicant/ctrl_iface.c
@@ -5371,7 +5371,7 @@  static void wpa_supplicant_ctrl_iface_flush(struct wpa_supplicant *wpa_s)
 	wpa_sm_set_param(wpa_s->wpa, RSNA_SA_TIMEOUT, 60);
 	eapol_sm_notify_logoff(wpa_s->eapol, FALSE);
 
-	radio_remove_unstarted_work(wpa_s, NULL);
+	radio_remove_works(wpa_s, NULL, 1);
 }
 
 
@@ -5423,6 +5423,10 @@  static void wpas_ctrl_radio_work_cb(struct wpa_radio_work *work, int deinit)
 	struct wpa_external_work *ework = work->ctx;
 
 	if (deinit) {
+		if (work->started)
+			eloop_cancel_timeout(wpas_ctrl_radio_work_timeout,
+					     work, NULL);
+
 		os_free(ework);
 		return;
 	}
diff --git a/wpa_supplicant/events.c b/wpa_supplicant/events.c
index 182b473..d1ac784 100644
--- a/wpa_supplicant/events.c
+++ b/wpa_supplicant/events.c
@@ -3231,6 +3231,8 @@  void wpa_supplicant_event(void *ctx, enum wpa_event_type event,
 #endif /* CONFIG_P2P */
 
 		wpa_supplicant_mark_disassoc(wpa_s);
+		radio_remove_works(wpa_s, NULL, 0);
+
 		wpa_supplicant_set_state(wpa_s, WPA_INTERFACE_DISABLED);
 		break;
 	case EVENT_CHANNEL_LIST_CHANGED:
diff --git a/wpa_supplicant/gas_query.c b/wpa_supplicant/gas_query.c
index abcb391..b255847 100644
--- a/wpa_supplicant/gas_query.c
+++ b/wpa_supplicant/gas_query.c
@@ -573,6 +573,12 @@  static void gas_query_start_cb(struct wpa_radio_work *work, int deinit)
 	struct gas_query *gas = query->gas;
 
 	if (deinit) {
+		if (work->started) {
+			gas->work = NULL;
+			gas_query_done(gas, query, GAS_QUERY_DELETED_AT_DEINIT);
+			return;
+		}
+
 		gas_query_free(query, 1);
 		return;
 	}
diff --git a/wpa_supplicant/p2p_supplicant.c b/wpa_supplicant/p2p_supplicant.c
index 113ef83..47758ea 100644
--- a/wpa_supplicant/p2p_supplicant.c
+++ b/wpa_supplicant/p2p_supplicant.c
@@ -252,7 +252,12 @@  static void wpas_p2p_trigger_scan_cb(struct wpa_radio_work *work, int deinit)
 	int ret;
 
 	if (deinit) {
-		wpa_scan_free_params(params);
+		if (!work->started) {
+			wpa_scan_free_params(params);
+			return;
+		}
+
+		wpa_s->p2p_scan_work = NULL;
 		return;
 	}
 
@@ -355,7 +360,7 @@  static int wpas_p2p_scan(void *ctx, enum p2p_scan_type type, int freq,
 		break;
 	}
 
-	radio_remove_unstarted_work(wpa_s, "p2p-scan");
+	radio_remove_works(wpa_s, "p2p-scan", 0);
 	if (radio_add_work(wpa_s, 0, "p2p-scan", 0, wpas_p2p_trigger_scan_cb,
 			   params) < 0)
 		goto fail;
@@ -1001,6 +1006,12 @@  static void wpas_send_action_cb(struct wpa_radio_work *work, int deinit)
 	struct send_action_work *awork = work->ctx;
 
 	if (deinit) {
+		if (work->started) {
+			eloop_cancel_timeout(wpas_p2p_send_action_work_timeout,
+					     wpa_s, NULL);
+			wpa_s->p2p_send_action_work = NULL;
+			offchannel_send_action_done(wpa_s);
+		}
 		os_free(awork);
 		return;
 	}
@@ -1740,12 +1751,30 @@  static void wpas_p2p_listen_work_done(struct wpa_supplicant *wpa_s)
 }
 
 
+static void wpas_stop_listen(void *ctx)
+{
+	struct wpa_supplicant *wpa_s = ctx;
+	if (wpa_s->off_channel_freq || wpa_s->roc_waiting_drv_freq) {
+		wpa_drv_cancel_remain_on_channel(wpa_s);
+		wpa_s->off_channel_freq = 0;
+		wpa_s->roc_waiting_drv_freq = 0;
+	}
+	wpa_drv_set_ap_wps_ie(wpa_s, NULL, NULL, NULL);
+	wpa_drv_probe_req_report(wpa_s, 0);
+	wpas_p2p_listen_work_done(wpa_s);
+}
+
+
 static void wpas_start_listen_cb(struct wpa_radio_work *work, int deinit)
 {
 	struct wpa_supplicant *wpa_s = work->wpa_s;
 	struct wpas_p2p_listen_work *lwork = work->ctx;
 
 	if (deinit) {
+		if (work->started) {
+			wpa_s->p2p_listen_work = NULL;
+			wpas_stop_listen(wpa_s);
+		}
 		wpas_p2p_listen_work_free(lwork);
 		return;
 	}
@@ -1813,20 +1842,6 @@  static int wpas_start_listen(void *ctx, unsigned int freq,
 }
 
 
-static void wpas_stop_listen(void *ctx)
-{
-	struct wpa_supplicant *wpa_s = ctx;
-	if (wpa_s->off_channel_freq || wpa_s->roc_waiting_drv_freq) {
-		wpa_drv_cancel_remain_on_channel(wpa_s);
-		wpa_s->off_channel_freq = 0;
-		wpa_s->roc_waiting_drv_freq = 0;
-	}
-	wpa_drv_set_ap_wps_ie(wpa_s, NULL, NULL, NULL);
-	wpa_drv_probe_req_report(wpa_s, 0);
-	wpas_p2p_listen_work_done(wpa_s);
-}
-
-
 static int wpas_send_probe_resp(void *ctx, const struct wpabuf *buf)
 {
 	struct wpa_supplicant *wpa_s = ctx;
diff --git a/wpa_supplicant/scan.c b/wpa_supplicant/scan.c
index 2d1eb2a..c66fa14 100644
--- a/wpa_supplicant/scan.c
+++ b/wpa_supplicant/scan.c
@@ -148,7 +148,13 @@  static void wpas_trigger_scan_cb(struct wpa_radio_work *work, int deinit)
 	int ret;
 
 	if (deinit) {
-		wpa_scan_free_params(params);
+		if (!work->started) {
+			wpa_scan_free_params(params);
+			return;
+		}
+		wpa_supplicant_notify_scanning(wpa_s, 0);
+		wpas_notify_scan_done(wpa_s, 0);
+		wpa_s->scan_work = NULL;
 		return;
 	}
 
diff --git a/wpa_supplicant/sme.c b/wpa_supplicant/sme.c
index 451f5ae..e712ac4 100644
--- a/wpa_supplicant/sme.c
+++ b/wpa_supplicant/sme.c
@@ -444,6 +444,9 @@  static void sme_auth_start_cb(struct wpa_radio_work *work, int deinit)
 	struct wpa_supplicant *wpa_s = work->wpa_s;
 
 	if (deinit) {
+		if (work->started)
+			wpa_s->connect_work = NULL;
+
 		wpas_connect_work_free(cwork);
 		return;
 	}
diff --git a/wpa_supplicant/wpa_supplicant.c b/wpa_supplicant/wpa_supplicant.c
index 455b158..f2e7abc 100644
--- a/wpa_supplicant/wpa_supplicant.c
+++ b/wpa_supplicant/wpa_supplicant.c
@@ -1401,6 +1401,13 @@  static void wpas_start_assoc_cb(struct wpa_radio_work *work, int deinit)
 #endif /* CONFIG_HT_OVERRIDES */
 
 	if (deinit) {
+		if (work->started) {
+			wpa_s->connect_work = NULL;
+
+			/* cancel possible auth. timeout */
+			eloop_cancel_timeout(wpa_supplicant_timeout, wpa_s,
+					     NULL);
+		}
 		wpas_connect_work_free(cwork);
 		return;
 	}
@@ -3075,25 +3082,40 @@  static void radio_start_next_work(void *eloop_ctx, void *timeout_ctx)
 }
 
 
-void radio_remove_unstarted_work(struct wpa_supplicant *wpa_s, const char *type)
+/*
+ * This function removes both started and pending radio works running on
+ * the provided interface's radio.
+ * Prior to the removal of the radio work, it's callback (cb) is called with
+ * deinit set to be 1. Each work's callback is responsible for clearing it's
+ * internal data and restoring to a correct state.
+ * @wpa_s: wpa_supplicant data
+ * @type: type of works to be removed
+ * @remove_all: 1 to remove all the works on this radio, 0 to remove only
+ * this interface's works.
+ */
+void radio_remove_works(struct wpa_supplicant *wpa_s,
+			const char *type, int remove_all)
 {
 	struct wpa_radio_work *work, *tmp;
 	struct wpa_radio *radio = wpa_s->radio;
 
 	dl_list_for_each_safe(work, tmp, &radio->work, struct wpa_radio_work,
 			      list) {
-		if (type && (work->started || os_strcmp(type, work->type) != 0))
+		if (type && (os_strcmp(type, work->type) != 0))
 			continue;
-		if (work->started) {
-			wpa_dbg(wpa_s, MSG_DEBUG, "Leaving started radio work '%s'@%p in the list",
-				work->type, work);
+
+		/* skip other ifaces' works */
+		if (!remove_all && (work->wpa_s != wpa_s))
 			continue;
-		}
-		wpa_dbg(wpa_s, MSG_DEBUG, "Remove unstarted radio work '%s'@%p",
+
+		wpa_dbg(wpa_s, MSG_DEBUG, "Remove radio work '%s'@%p",
 			work->type, work);
 		work->cb(work, 1);
 		radio_work_free(work);
 	}
+
+	/* in case we removed the started work */
+	radio_work_check_next(wpa_s);
 }
 
 
@@ -3113,7 +3135,7 @@  static void radio_remove_interface(struct wpa_supplicant *wpa_s)
 	}
 
 	wpa_printf(MSG_DEBUG, "Remove radio %s", radio->name);
-	radio_remove_unstarted_work(wpa_s, NULL);
+	radio_remove_works(wpa_s, NULL, 0);
 	eloop_cancel_timeout(radio_start_next_work, radio, NULL);
 	wpa_s->radio = NULL;
 	os_free(radio);
diff --git a/wpa_supplicant/wpa_supplicant_i.h b/wpa_supplicant/wpa_supplicant_i.h
index 4ca0306..bcdb4d0 100644
--- a/wpa_supplicant/wpa_supplicant_i.h
+++ b/wpa_supplicant/wpa_supplicant_i.h
@@ -305,8 +305,8 @@  int radio_add_work(struct wpa_supplicant *wpa_s, unsigned int freq,
 		   void (*cb)(struct wpa_radio_work *work, int deinit),
 		   void *ctx);
 void radio_work_done(struct wpa_radio_work *work);
-void radio_remove_unstarted_work(struct wpa_supplicant *wpa_s,
-				 const char *type);
+void radio_remove_works(struct wpa_supplicant *wpa_s,
+			const char *type, int remove_all);
 void radio_work_check_next(struct wpa_supplicant *wpa_s);
 
 struct wpa_connect_work {