| Message ID | 1473144455-5267-1-git-send-email-andrei.otcheretianski@intel.com |
|---|---|
| State | Changes Requested |
| Headers | show |
On Tue, Sep 06, 2016 at 09:47:35AM +0300, andrei.otcheretianski@intel.com wrote: > If DEAUTH event is received while authenticating, wpas_connection_failed() > is invoked cancelling the radio work. However, the flow might continue > calling sme_disassoc_while_authenticating() which leaves the wpa_supplicant > in the AUTHENTICATING state, thus allowing the continuation of the connection > flow (without radio work protection) in case AUTH frame is received. > > This issue was seen during EAPOL connection, when the client starts the fast > association in wpas_wps_eapol_cb, where the following race occurs: > > 1. DEAUTH after initial EAPOL HS > 2. Start fast associate and send AUTH > 3. DEAUTH event rebound from kernel -> wpas_connection_failed() is called, > stopping the connect radio work > 4. SCAN is started > 5. AUTH is received, and the connection flow is continued without > radio work protection > 6. SCAN_RESULTS received in the middle of association. > 7. Failure in wpa_driver_nl80211_check_bss_status due to > state mismatch - > DEAUTH with reason code 2. Would you be able to share a debug log showing such a case? > Fix this by not calling wpas_connection_failed() in step 4, if the > wpa_supplicant is in authenticating state and using SME (same conditions > that result in calling sme_disassoc_while_authenticating()). This does not sound correct.. sme-connect radio work should not be left running if a new connection is needed. In addition, it looks like wpa_supplicant_event_disassoc_finish() could call wpa_supplicant_connect() after that and that would result in another sme-connect radio work being added when trying to associate again. It sound like step 5 might need to add a new sme-connect radio work instead.
> -----Original Message----- > From: Jouni Malinen [mailto:j@w1.fi] > Sent: Sunday, September 25, 2016 22:15 > To: Otcheretianski, Andrei <andrei.otcheretianski@intel.com> > Cc: hostap@lists.infradead.org > Subject: Re: [PATCH 03/18] wpa_supplicant: Don't stop conn. radio work on > DEAUTH > > On Tue, Sep 06, 2016 at 09:47:35AM +0300, andrei.otcheretianski@intel.com > wrote: > > If DEAUTH event is received while authenticating, > > wpas_connection_failed() is invoked cancelling the radio work. > > However, the flow might continue calling > > sme_disassoc_while_authenticating() which leaves the wpa_supplicant in > > the AUTHENTICATING state, thus allowing the continuation of the > connection flow (without radio work protection) in case AUTH frame is > received. > > > > This issue was seen during EAPOL connection, when the client starts > > the fast association in wpas_wps_eapol_cb, where the following race > occurs: > > > > 1. DEAUTH after initial EAPOL HS > > 2. Start fast associate and send AUTH > > 3. DEAUTH event rebound from kernel -> wpas_connection_failed() is > called, > > stopping the connect radio work > > 4. SCAN is started > > 5. AUTH is received, and the connection flow is continued without > > radio work protection > > 6. SCAN_RESULTS received in the middle of association. > > 7. Failure in wpa_driver_nl80211_check_bss_status due to > > state mismatch - > DEAUTH with reason code 2. > > Would you be able to share a debug log showing such a case? Unfortunately I can't find the logs for this scenario now. This patch was sitting in our internal tree for quite a long period. I'll try to reproduce, but I'm travelling now, so it can take some time. > > > Fix this by not calling wpas_connection_failed() in step 4, if the > > wpa_supplicant is in authenticating state and using SME (same > > conditions that result in calling sme_disassoc_while_authenticating()). > > This does not sound correct.. sme-connect radio work should not be left > running if a new connection is needed. I don't think it's a new connection. After processing the DEAUTH we are left in >= WPA_AUTHENTICATING state (by sme_disassoc_while_authenticating()). So it doesn't sound correct to not have a radio work at this state. > In addition, it looks like > wpa_supplicant_event_disassoc_finish() could call > wpa_supplicant_connect() after that and that would result in another sme- > connect radio work being added when trying to associate again. > Since wpas is in AUTHENTICATING state, I think wpa_supplicant_connect() would just exit without doing anything. Also, scheduling another radio work might introduce its own races, as it can be delayed by other activities that required the radio. > It sound like step 5 might need to add a new sme-connect radio work > instead. > > -- > Jouni Malinen PGP id EFC895FA
diff --git a/wpa_supplicant/events.c b/wpa_supplicant/events.c index ba30780..1058115 100644 --- a/wpa_supplicant/events.c +++ b/wpa_supplicant/events.c @@ -2572,8 +2572,19 @@ static void wpa_supplicant_event_disassoc_finish(struct wpa_supplicant *wpa_s, bssid = wpa_s->bssid; if (is_zero_ether_addr(bssid)) bssid = wpa_s->pending_bssid; - if (wpa_s->wpa_state >= WPA_AUTHENTICATING) - wpas_connection_failed(wpa_s, bssid); + + if (wpa_s->wpa_state >= WPA_AUTHENTICATING) { + /* + * The connection shouldn't be failed if we will call + * sme_disassoc_while_authenticating, otherwise we may + * continue the connection, without radio work + * protection. + */ + if (!authenticating || + !(wpa_s->drv_flags & WPA_DRIVER_FLAGS_SME)) + wpas_connection_failed(wpa_s, bssid); + } + wpa_sm_notify_disassoc(wpa_s->wpa); if (locally_generated) wpa_s->disconnect_reason = -reason_code;