wpa_supplicant: Track consecutive connection failures

Within wpas_connection_failed(), the 'count' value of wpa_blacklist is
erroneously used as a tally of the number times the device has failed
to associate to a given BSSID without making a successful connection.
This is not accurate because there are a variety of ways a BSS can be
added to the blacklist beyond failed association such as interference
or deauthentication. This 'count' is lost whenever the blacklist is
cleared, so the wpa_supplicant stores an additional value
'extra_blacklist_count' which helps persist the 'count' through clears.
These count values are used to determine how long to wait to rescan
after a failed connection attempt.

While this logic was already slightly wrong, it would have been
completely broken by the upcoming change which adds time-based
blacklisting functionality. With the upcoming change, 'count' values
are not cleared on association, and thus do not necessarily even
approximate the "consecutive connection failures" which they were being
used for.

This change seeks to remove this unnecessary overloading of the
blacklist 'count' by directly tracking consecutive connection failures
within the wpa_supplicant struct, independent of the blacklist. This new
'consecutive_conn_failures' is iterated with every connection failure
and cleared when any successful connection is made. This change also
removes the now unused 'extra_blacklist_count' value.

Signed-off-by: Kevin Lund <kglund@google.com>
Signed-off-by: Brian Norris <briannorris@chromium.org>
This commit is contained in:
Kevin Lund 2020-06-11 14:11:15 -07:00 committed by Jouni Malinen
parent 5653301409
commit 2fd35d9857
5 changed files with 17 additions and 34 deletions

View file

@ -123,19 +123,14 @@ int wpa_blacklist_del(struct wpa_supplicant *wpa_s, const u8 *bssid)
void wpa_blacklist_clear(struct wpa_supplicant *wpa_s) void wpa_blacklist_clear(struct wpa_supplicant *wpa_s)
{ {
struct wpa_blacklist *e, *prev; struct wpa_blacklist *e, *prev;
int max_count = 0;
e = wpa_s->blacklist; e = wpa_s->blacklist;
wpa_s->blacklist = NULL; wpa_s->blacklist = NULL;
while (e) { while (e) {
if (e->count > max_count)
max_count = e->count;
prev = e; prev = e;
e = e->next; e = e->next;
wpa_printf(MSG_DEBUG, "Removed BSSID " MACSTR " from " wpa_printf(MSG_DEBUG, "Removed BSSID " MACSTR " from "
"blacklist (clear)", MAC2STR(prev->bssid)); "blacklist (clear)", MAC2STR(prev->bssid));
os_free(prev); os_free(prev);
} }
wpa_s->extra_blacklist_count += max_count;
} }

View file

@ -8418,9 +8418,10 @@ static void wpa_supplicant_ctrl_iface_flush(struct wpa_supplicant *wpa_s)
wpa_s->set_sta_uapsd = 0; wpa_s->set_sta_uapsd = 0;
wpa_s->sta_uapsd = 0; wpa_s->sta_uapsd = 0;
wpa_s->consecutive_conn_failures = 0;
wpa_drv_radio_disable(wpa_s, 0); wpa_drv_radio_disable(wpa_s, 0);
wpa_blacklist_clear(wpa_s); wpa_blacklist_clear(wpa_s);
wpa_s->extra_blacklist_count = 0;
wpa_supplicant_ctrl_iface_remove_network(wpa_s, "all"); wpa_supplicant_ctrl_iface_remove_network(wpa_s, "all");
wpa_supplicant_ctrl_iface_remove_cred(wpa_s, "all"); wpa_supplicant_ctrl_iface_remove_cred(wpa_s, "all");
wpa_config_flush_blobs(wpa_s->conf); wpa_config_flush_blobs(wpa_s->conf);

View file

@ -985,7 +985,7 @@ void wpa_supplicant_set_state(struct wpa_supplicant *wpa_s,
#endif /* CONFIG_CTRL_IFACE || !CONFIG_NO_STDOUT_DEBUG */ #endif /* CONFIG_CTRL_IFACE || !CONFIG_NO_STDOUT_DEBUG */
wpas_clear_temp_disabled(wpa_s, ssid, 1); wpas_clear_temp_disabled(wpa_s, ssid, 1);
wpa_blacklist_clear(wpa_s); wpa_blacklist_clear(wpa_s);
wpa_s->extra_blacklist_count = 0; wpa_s->consecutive_conn_failures = 0;
wpa_s->new_connection = 0; wpa_s->new_connection = 0;
wpa_drv_set_operstate(wpa_s, 1); wpa_drv_set_operstate(wpa_s, 1);
#ifndef IEEE8021X_EAPOL #ifndef IEEE8021X_EAPOL
@ -7373,10 +7373,6 @@ void wpas_connection_failed(struct wpa_supplicant *wpa_s, const u8 *bssid)
/* /*
* Add the failed BSSID into the blacklist and speed up next scan * Add the failed BSSID into the blacklist and speed up next scan
* attempt if there could be other APs that could accept association. * attempt if there could be other APs that could accept association.
* The current blacklist count indicates how many times we have tried
* connecting to this AP and multiple attempts mean that other APs are
* either not available or has already been tried, so that we can start
* increasing the delay here to avoid constant scanning.
*/ */
count = wpa_blacklist_add(wpa_s, bssid); count = wpa_blacklist_add(wpa_s, bssid);
if (count == 1 && wpa_s->current_bss) { if (count == 1 && wpa_s->current_bss) {
@ -7401,19 +7397,19 @@ void wpas_connection_failed(struct wpa_supplicant *wpa_s, const u8 *bssid)
} }
} }
/* wpa_s->consecutive_conn_failures++;
* Add previous failure count in case the temporary blacklist was
* cleared due to no other BSSes being available.
*/
count += wpa_s->extra_blacklist_count;
if (count > 3 && wpa_s->current_ssid) { if (wpa_s->consecutive_conn_failures > 3 && wpa_s->current_ssid) {
wpa_printf(MSG_DEBUG, "Continuous association failures - " wpa_printf(MSG_DEBUG, "Continuous association failures - "
"consider temporary network disabling"); "consider temporary network disabling");
wpas_auth_failed(wpa_s, "CONN_FAILED"); wpas_auth_failed(wpa_s, "CONN_FAILED");
} }
/*
switch (count) { * Multiple consecutive connection failures mean that other APs are
* either not available or have already been tried, so we can start
* increasing the delay here to avoid constant scanning.
*/
switch (wpa_s->consecutive_conn_failures) {
case 1: case 1:
timeout = 100; timeout = 100;
break; break;
@ -7431,8 +7427,9 @@ void wpas_connection_failed(struct wpa_supplicant *wpa_s, const u8 *bssid)
break; break;
} }
wpa_dbg(wpa_s, MSG_DEBUG, "Blacklist count %d --> request scan in %d " wpa_dbg(wpa_s, MSG_DEBUG,
"ms", count, timeout); "Consecutive connection failures: %d --> request scan in %d ms",
wpa_s->consecutive_conn_failures, timeout);
/* /*
* TODO: if more than one possible AP is available in scan results, * TODO: if more than one possible AP is available in scan results,
@ -7796,7 +7793,6 @@ void wpas_request_connection(struct wpa_supplicant *wpa_s)
wpa_s->normal_scans = 0; wpa_s->normal_scans = 0;
wpa_s->scan_req = NORMAL_SCAN_REQ; wpa_s->scan_req = NORMAL_SCAN_REQ;
wpa_supplicant_reinit_autoscan(wpa_s); wpa_supplicant_reinit_autoscan(wpa_s);
wpa_s->extra_blacklist_count = 0;
wpa_s->disconnected = 0; wpa_s->disconnected = 0;
wpa_s->reassociate = 1; wpa_s->reassociate = 1;
wpa_s->last_owe_group = 0; wpa_s->last_owe_group = 0;

View file

@ -666,17 +666,8 @@ struct wpa_supplicant {
struct wpa_blacklist *blacklist; struct wpa_blacklist *blacklist;
/** /* Number of connection failures since last successful connection */
* extra_blacklist_count - Sum of blacklist counts after last connection unsigned int consecutive_conn_failures;
*
* This variable is used to maintain a count of temporary blacklisting
* failures (maximum number for any BSS) over blacklist clear
* operations. This is needed for figuring out whether there has been
* failures prior to the last blacklist clear operation which happens
* whenever no other not-blacklisted BSS candidates are available. This
* gets cleared whenever a connection has been established successfully.
*/
int extra_blacklist_count;
/** /**
* scan_req - Type of the scan request * scan_req - Type of the scan request

View file

@ -720,7 +720,7 @@ static void wpa_supplicant_wps_event_success(struct wpa_supplicant *wpa_s)
wpas_notify_wps_event_success(wpa_s); wpas_notify_wps_event_success(wpa_s);
if (wpa_s->current_ssid) if (wpa_s->current_ssid)
wpas_clear_temp_disabled(wpa_s, wpa_s->current_ssid, 1); wpas_clear_temp_disabled(wpa_s, wpa_s->current_ssid, 1);
wpa_s->extra_blacklist_count = 0; wpa_s->consecutive_conn_failures = 0;
/* /*
* Enable the networks disabled during wpas_wps_reassoc after 10 * Enable the networks disabled during wpas_wps_reassoc after 10