From patchwork Mon Aug  6 16:30:02 2012
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: [RFC] pmksa: don't evict active entry when adding new ones
Date: Mon, 06 Aug 2012 06:30:02 -0000
From: Dan Williams <dcbw@redhat.com>
X-Patchwork-Id: 175422
Message-Id: <1344270602.11053.9.camel@dcbw.foobar.com>
To: hostap@lists.shmoo.com
Cc: j@w1.fi

If the PMKSA cache is full (ie, 32 candidates have been seen in scan
results and have not yet expired) then any additional entries can
potentially evict the current/active entry (if it is the first entry),
which triggers a pointless local deauthentication.  The supplicant
shouldn't replace the current/active entry if it is still valid, but
instead the oldest entry that is *not* the current/active one.

1343242112.312194: New scan results available
1343242112.312282: bgscan simple: scan result notification
1343242112.312343: RSN: Consider 00:xx:xx:xx:xx:40 for OKC
1343242112.312408: RSN: removed the oldest PMKSA cache entry (for
00:xx:xx:xx:xx:90) to make room for new one
1343242112.312454: RSN: removed current PMKSA entry
1343242112.312490: wpa_driver_nl80211_deauthenticate
1343242112.336808: State: COMPLETED -> DISCONNECTED   (oops...)

This is a bug at least as far back as 0.7, so the patch is desired for
0.7 (if you're ever updating that again), 1.0, and git master.

Signed-hostap: Dan Williams <dcbw@redhat.com>

---
Does this patch look correct?  I haven't runtime tested it yet, but
that's in the process of being done.  Somebody double-check my
linked-list logic, please :)

diff -up wpa_supplicant-0.7.3/src/rsn_supp/pmksa_cache.c.foo wpa_supplicant-0.7.3/src/rsn_supp/pmksa_cache.c
--- wpa_supplicant-0.7.3/src/rsn_supp/pmksa_cache.c.foo	2012-08-05 23:34:38.230809262 -0500
+++ wpa_supplicant-0.7.3/src/rsn_supp/pmksa_cache.c	2012-08-05 23:41:10.862900686 -0500
@@ -203,11 +203,23 @@ pmksa_cache_add(struct rsn_pmksa_cache *
 	if (pmksa->pmksa_count >= pmksa_cache_max_entries && pmksa->pmksa) {
 		/* Remove the oldest entry to make room for the new entry */
 		pos = pmksa->pmksa;
-		pmksa->pmksa = pos->next;
-		wpa_printf(MSG_DEBUG, "RSN: removed the oldest PMKSA cache "
-			   "entry (for " MACSTR ") to make room for new one",
-			   MAC2STR(pos->aa));
-		pmksa_cache_free_entry(pmksa, pos, 0);
+
+		/* Never remove the current PMKSA cache entry, since it's
+		 * in use, and removing it triggers a needless deauthentication.
+		 */
+		if (pos == pmksa->sm->cur_pmksa) {
+			pos = pos->next;
+			pmksa->pmksa->next = pos ? pos->next : NULL;
+		} else
+			pmksa->pmksa = pos->next;
+
+		if (pos) {
+			wpa_printf(MSG_DEBUG, "RSN: removed the oldest idle "
+				   "PMKSA cache entry (for " MACSTR ") to make "
+				   "room for new one",
+				   MAC2STR(pos->aa));
+			pmksa_cache_free_entry(pmksa, pos, 0);
+		}
 	}
 
 	/* Add the new entry; order by expiration time */