In hostap mode, don't re-use association IDs (AIDs) of nodes which are

still lingering in the node cache. This could cause an AID to be assigned
twice, once to a newly associated node and once to a different node in
COLLECT cache state (i.e. marked for future eviction from the node cache).

Drivers (e.g. rt2860) may use AIDs to keep track of nodes in firmware
tables and get confused when AIDs aren't unique across the node cache.
The symptom observed with rt2860 were nodes stuck at 1 Mbps Tx rate since
the duplicate AID made the driver perform Tx rate (AMRR) accounting on
the wrong node object.

To find out if a node is associated we now check the node's cache state,
rather than comparing the node's AID against zero. An AID is assigned when
a node associates and it lasts until the node is eventually purged from the
node cache (previously, the AID was made available for re-use when the node
was placed in COLLECT state). There is no need to be stingy with AIDs since
the number of possible AIDs exceeds the maximum number of nodes in the cache.

Problem found by Nathanael Rensen.
Fix written by Nathanael and myself. Tested by Nathanael.
Comitting now to get this change tested across as many drivers as possible.

3 files changed, 7 insertions, 11 deletions

diff --git a/sys/net80211/ieee80211_input.c b/sys/net80211/ieee80211_input.c
index 7e4129bab09..7209e3192a4 100644
--- a/sys/net80211/ieee80211_input.c
+++ b/sys/net80211/ieee80211_input.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ieee80211_input.c,v 1.177 2016/05/10 06:37:15 dlg Exp $	*/
+/*	$OpenBSD: ieee80211_input.c,v 1.178 2016/05/18 08:15:28 stsp Exp $	*/
 
 /*-
  * Copyright (c) 2001 Atsushi Onoe
@@ -447,7 +447,7 @@ ieee80211_input(struct ifnet *ifp, struct mbuf *m, struct ieee80211_node *ni,
 				ic->ic_stats.is_rx_notassoc++;
 				goto err;
 			}
-			if (ni->ni_associd == 0) {
+			if (ni->ni_state != IEEE80211_STA_ASSOC) {
 				DPRINTF(("data from unassoc src %s\n",
 				    ether_sprintf(wh->i_addr2)));
 				IEEE80211_SEND_MGMT(ic, ni,
diff --git a/sys/net80211/ieee80211_node.c b/sys/net80211/ieee80211_node.c
index a7fcb4bd632..4191ceb56cb 100644
--- a/sys/net80211/ieee80211_node.c
+++ b/sys/net80211/ieee80211_node.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ieee80211_node.c,v 1.101 2016/04/12 14:33:27 mpi Exp $	*/
+/*	$OpenBSD: ieee80211_node.c,v 1.102 2016/05/18 08:15:28 stsp Exp $	*/
 /*	$NetBSD: ieee80211_node.c,v 1.14 2004/05/09 09:18:47 dyoung Exp $	*/
 
 /*-
@@ -1496,7 +1496,7 @@ void
 ieee80211_node_join(struct ieee80211com *ic, struct ieee80211_node *ni,
     int resp)
 {
-	int newassoc;
+	int newassoc = (ni->ni_state != IEEE80211_STA_ASSOC);
 
 	if (ni->ni_associd == 0) {
 		u_int16_t aid;
@@ -1518,13 +1518,11 @@ ieee80211_node_join(struct ieee80211com *ic, struct ieee80211_node *ni,
 		}
 		ni->ni_associd = aid | 0xc000;
 		IEEE80211_AID_SET(ni->ni_associd, ic->ic_aid_bitmap);
-		newassoc = 1;
 		if (ic->ic_curmode == IEEE80211_MODE_11G ||
 		    (ic->ic_curmode == IEEE80211_MODE_11N &&
 		    IEEE80211_IS_CHAN_2GHZ(ic->ic_bss->ni_chan)))
 			ieee80211_node_join_11g(ic, ni);
-	} else
-		newassoc = 0;
+	}
 
 	DPRINTF(("station %s %s associated at aid %d\n",
 	    ether_sprintf(ni->ni_macaddr), newassoc ? "newly" : "already",
@@ -1699,8 +1697,6 @@ ieee80211_node_leave(struct ieee80211com *ic, struct ieee80211_node *ni)
 	if (ic->ic_node_leave != NULL)
 		(*ic->ic_node_leave)(ic, ni);
 
-	IEEE80211_AID_CLR(ni->ni_associd, ic->ic_aid_bitmap);
-	ni->ni_associd = 0;
 	ieee80211_node_newstate(ni, IEEE80211_STA_COLLECT);
 
 #if NBRIDGE > 0
diff --git a/sys/net80211/ieee80211_proto.c b/sys/net80211/ieee80211_proto.c
index 83aae43fca4..0941831675f 100644
--- a/sys/net80211/ieee80211_proto.c
+++ b/sys/net80211/ieee80211_proto.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ieee80211_proto.c,v 1.66 2016/04/27 11:58:10 stsp Exp $	*/
+/*	$OpenBSD: ieee80211_proto.c,v 1.67 2016/05/18 08:15:28 stsp Exp $	*/
 /*	$NetBSD: ieee80211_proto.c,v 1.8 2004/04/30 23:58:20 dyoung Exp $	*/
 
 /*-
@@ -852,7 +852,7 @@ ieee80211_newstate(struct ieee80211com *ic, enum ieee80211_state nstate,
 			case IEEE80211_M_HOSTAP:
 				s = splnet();
 				RB_FOREACH(ni, ieee80211_tree, &ic->ic_tree) {
-					if (ni->ni_associd == 0)
+					if (ni->ni_state != IEEE80211_STA_ASSOC)
 						continue;
 					IEEE80211_SEND_MGMT(ic, ni,
 					    IEEE80211_FC0_SUBTYPE_DISASSOC,