diff mbox

[PATCHv2] bss: Don't remove a BSS that is in use

Message ID 20120517141618.A91392061B@glenhelen.mtv.corp.google.com
State Accepted
Commit a6b71f726a2c3223ea305bcf1bc6777635603827
Headers show

Commit Message

Paul Stewart May 16, 2012, 12:30 a.m. UTC 
When looking for a BSS to eject due to too many entries, never
pick one that is in use.  Otherwise, we run the risk of having
pointers to freed data.

Signed-hostap: Paul Stewart <pstew@chromium.org>
Cc: Jouni Malinen <j@w1.fi>
Cc: Johannes Berg <johannes@sipsolutions.net>
Cc: Kalle Valo <kvalo@adurom.com>
---
 wpa_supplicant/bss.c |   63 ++++++++++++++++++++++++++++---------------------
 1 files changed, 36 insertions(+), 27 deletions(-)

Comments

Jouni Malinen June 4, 2012, 6:53 p.m. UTC | #1 
On Tue, May 15, 2012 at 05:30:56PM -0700, Paul Stewart wrote:
> When looking for a BSS to eject due to too many entries, never
> pick one that is in use.  Otherwise, we run the risk of having
> pointers to freed data.

Thanks! Applied.
diff mbox

Patch

diff --git a/wpa_supplicant/bss.c b/wpa_supplicant/bss.c
index a373116..12b311f 100644
--- a/wpa_supplicant/bss.c
+++ b/wpa_supplicant/bss.c
@@ -35,14 +35,15 @@ 
 #define WPA_BSS_IES_CHANGED_FLAG	BIT(8)
 
 
-static void wpa_bss_remove(struct wpa_supplicant *wpa_s, struct wpa_bss *bss)
+static void wpa_bss_remove(struct wpa_supplicant *wpa_s, struct wpa_bss *bss,
+			   const char *reason)
 {
 	dl_list_del(&bss->list);
 	dl_list_del(&bss->list_id);
 	wpa_s->num_bss--;
 	wpa_dbg(wpa_s, MSG_DEBUG, "BSS: Remove id %u BSSID " MACSTR
-		" SSID '%s'", bss->id, MAC2STR(bss->bssid),
-		wpa_ssid_txt(bss->ssid, bss->ssid_len));
+		" SSID '%s' due to %s", bss->id, MAC2STR(bss->bssid),
+		wpa_ssid_txt(bss->ssid, bss->ssid_len), reason);
 	wpas_notify_bss_removed(wpa_s, bss->bssid, bss->id);
 #ifdef CONFIG_INTERWORKING
 	wpabuf_free(bss->anqp_venue_name);
@@ -120,13 +121,21 @@  static int wpa_bss_known(struct wpa_supplicant *wpa_s, struct wpa_bss *bss)
 }
 
 
+static int wpa_bss_in_use(struct wpa_supplicant *wpa_s, struct wpa_bss *bss)
+{
+	return bss == wpa_s->current_bss ||
+		os_memcmp(bss->bssid, wpa_s->bssid, ETH_ALEN) == 0 ||
+		os_memcmp(bss->bssid, wpa_s->pending_bssid, ETH_ALEN) == 0;
+}
+
+
 static int wpa_bss_remove_oldest_unknown(struct wpa_supplicant *wpa_s)
 {
 	struct wpa_bss *bss;
 
 	dl_list_for_each(bss, &wpa_s->bss, struct wpa_bss, list) {
 		if (!wpa_bss_known(wpa_s, bss)) {
-			wpa_bss_remove(wpa_s, bss);
+			wpa_bss_remove(wpa_s, bss, __func__);
 			return 0;
 		}
 	}
@@ -135,21 +144,28 @@  static int wpa_bss_remove_oldest_unknown(struct wpa_supplicant *wpa_s)
 }
 
 
-static void wpa_bss_remove_oldest(struct wpa_supplicant *wpa_s)
+static int wpa_bss_remove_oldest(struct wpa_supplicant *wpa_s)
 {
+	struct wpa_bss *bss;
+
 	/*
 	 * Remove the oldest entry that does not match with any configured
 	 * network.
 	 */
 	if (wpa_bss_remove_oldest_unknown(wpa_s) == 0)
-		return;
+		return 0;
 
 	/*
-	 * Remove the oldest entry since no better candidate for removal was
-	 * found.
+	 * Remove the oldest entry that isn't currently in use.
 	 */
-	wpa_bss_remove(wpa_s, dl_list_first(&wpa_s->bss,
-					    struct wpa_bss, list));
+	dl_list_for_each(bss, &wpa_s->bss, struct wpa_bss, list) {
+		if (!wpa_bss_in_use(wpa_s, bss)) {
+			wpa_bss_remove(wpa_s, bss, __func__);
+			return 0;
+		}
+	}
+
+	return -1;
 }
 
 
@@ -178,8 +194,13 @@  static void wpa_bss_add(struct wpa_supplicant *wpa_s,
 		" SSID '%s'",
 		bss->id, MAC2STR(bss->bssid), wpa_ssid_txt(ssid, ssid_len));
 	wpas_notify_bss_added(wpa_s, bss->bssid, bss->id);
-	if (wpa_s->num_bss > wpa_s->conf->bss_max_count)
-		wpa_bss_remove_oldest(wpa_s);
+	if (wpa_s->num_bss > wpa_s->conf->bss_max_count &&
+	    wpa_bss_remove_oldest(wpa_s) != 0) {
+		wpa_printf(MSG_ERROR, "Increasing the MAX BSS count "
+			   " to %d because all BSSes are in use.  We should "
+			   " normally not get here!", wpa_s->num_bss);
+		wpa_s->conf->bss_max_count = wpa_s->num_bss;
+	}
 }
 
 
@@ -350,14 +371,6 @@  static void wpa_bss_update(struct wpa_supplicant *wpa_s, struct wpa_bss *bss,
 }
 
 
-static int wpa_bss_in_use(struct wpa_supplicant *wpa_s, struct wpa_bss *bss)
-{
-	return bss == wpa_s->current_bss ||
-		os_memcmp(bss->bssid, wpa_s->bssid, ETH_ALEN) == 0 ||
-		os_memcmp(bss->bssid, wpa_s->pending_bssid, ETH_ALEN) == 0;
-}
-
-
 void wpa_bss_update_start(struct wpa_supplicant *wpa_s)
 {
 	wpa_s->bss_update_idx++;
@@ -469,9 +482,7 @@  void wpa_bss_update_end(struct wpa_supplicant *wpa_s, struct scan_info *info,
 			bss->scan_miss_count++;
 		if (bss->scan_miss_count >=
 		    wpa_s->conf->bss_expiration_scan_count) {
-			wpa_dbg(wpa_s, MSG_DEBUG, "BSS: Expire BSS %u due to "
-				"no match in scan", bss->id);
-			wpa_bss_remove(wpa_s, bss);
+			wpa_bss_remove(wpa_s, bss, "no match in scan");
 		}
 	}
 }
@@ -493,9 +504,7 @@  void wpa_bss_flush_by_age(struct wpa_supplicant *wpa_s, int age)
 			continue;
 
 		if (os_time_before(&bss->last_update, &t)) {
-			wpa_dbg(wpa_s, MSG_DEBUG, "BSS: Expire BSS %u due to "
-				"age", bss->id);
-			wpa_bss_remove(wpa_s, bss);
+			wpa_bss_remove(wpa_s, bss, __func__);
 		} else
 			break;
 	}
@@ -532,7 +541,7 @@  void wpa_bss_flush(struct wpa_supplicant *wpa_s)
 	dl_list_for_each_safe(bss, n, &wpa_s->bss, struct wpa_bss, list) {
 		if (wpa_bss_in_use(wpa_s, bss))
 			continue;
-		wpa_bss_remove(wpa_s, bss);
+		wpa_bss_remove(wpa_s, bss, __func__);
 	}
 }