The ieee80211_dup_bss() function internally calls
ieee80211_alloc_node_helper(), which means we should check
the node cache for an existing entry before calling ieee80211_dup_bss().

ieee80211_node.c already does this but there are some instances in
ieee80211_input.c where we fail to check for existing cache entries first.
This can leak nodes and distort our idea of how many nodes are in the cache.

The diff below also prints a warning if the interface is in debug
mode and a potential node leak is detected during the node cache
cleanup timeout, which runs once per hour. This might make spotting
such bugs easier in the future.

Index: ieee80211_input.c
===================================================================
RCS file: /cvs/src/sys/net80211/ieee80211_input.c,v
retrieving revision 1.120
diff -u -p -r1.120 ieee80211_input.c
--- ieee80211_input.c   13 Jul 2012 11:25:04 -0000      1.120
+++ ieee80211_input.c   13 Jul 2012 17:47:15 -0000
@@ -401,7 +401,9 @@ ieee80211_input(struct ifnet *ifp, struc
                                DPRINTF(("data from unknown src %s\n",
                                    ether_sprintf(wh->i_addr2)));
                                /* NB: caller deals with reference */
-                               ni = ieee80211_dup_bss(ic, wh->i_addr2);
+                               ni = ieee80211_find_node(ic, wh->i_addr2);
+                               if (ni == NULL)
+                                       ni = ieee80211_dup_bss(ic, wh->i_addr2);
                                if (ni != NULL) {
                                        IEEE80211_SEND_MGMT(ic, ni,
                                            IEEE80211_FC0_SUBTYPE_DEAUTH,
@@ -1725,7 +1727,9 @@ ieee80211_recv_probe_req(struct ieee8021
        }
 
        if (ni == ic->ic_bss) {
-               ni = ieee80211_dup_bss(ic, wh->i_addr2);
+               ni = ieee80211_find_node(ic, wh->i_addr2);
+               if (ni == NULL)
+                       ni = ieee80211_dup_bss(ic, wh->i_addr2);
                if (ni == NULL)
                        return;
                DPRINTF(("new probe req from %s\n",
@@ -1905,7 +1909,9 @@ ieee80211_recv_assoc_req(struct ieee8021
                DPRINTF(("deny %sassoc from %s, not authenticated\n",
                    reassoc ? "re" : "",
                    ether_sprintf((u_int8_t *)wh->i_addr2)));
-               ni = ieee80211_dup_bss(ic, wh->i_addr2);
+               ni = ieee80211_find_node(ic, wh->i_addr2);
+               if (ni == NULL)
+                       ni = ieee80211_dup_bss(ic, wh->i_addr2);
                if (ni != NULL) {
                        IEEE80211_SEND_MGMT(ic, ni,
                            IEEE80211_FC0_SUBTYPE_DEAUTH,
Index: ieee80211_node.c
===================================================================
RCS file: /cvs/src/sys/net80211/ieee80211_node.c,v
retrieving revision 1.69
diff -u -p -r1.69 ieee80211_node.c
--- ieee80211_node.c    13 Jul 2012 09:46:33 -0000      1.69
+++ ieee80211_node.c    13 Jul 2012 18:12:27 -0000
@@ -1150,6 +1150,10 @@ ieee80211_clean_nodes(struct ieee80211co
        struct ieee80211_node *ni, *next_ni;
        u_int gen = ic->ic_scangen++;           /* NB: ok 'cuz single-threaded*/
        int s;
+#ifndef IEEE80211_STA_ONLY
+       int nnodes = 0;
+       struct ifnet *ifp = &ic->ic_if;
+#endif
 
        s = splnet();
        for (ni = RB_MIN(ieee80211_tree, &ic->ic_tree);
@@ -1159,6 +1163,9 @@ ieee80211_clean_nodes(struct ieee80211co
                        break;
                if (ni->ni_scangen == gen)      /* previously handled */
                        continue;
+#ifndef IEEE80211_STA_ONLY
+               nnodes++;
+#endif
                ni->ni_scangen = gen;
                if (ni->ni_refcnt > 0)
                        continue;
@@ -1195,6 +1202,7 @@ ieee80211_clean_nodes(struct ieee80211co
                 * the driver calls ieee80211_release_node().
                 */
 #ifndef IEEE80211_STA_ONLY
+               nnodes--;
                if (ic->ic_opmode == IEEE80211_M_HOSTAP &&
                    ni->ni_state >= IEEE80211_STA_AUTH &&
                    ni->ni_state != IEEE80211_STA_COLLECT) {
@@ -1209,6 +1217,20 @@ ieee80211_clean_nodes(struct ieee80211co
                        ieee80211_free_node(ic, ni);
                ic->ic_stats.is_node_timeout++;
        }
+
+#ifndef IEEE80211_STA_ONLY
+       /* 
+        * During a cache timeout we iterate over all nodes.
+        * Check for node leaks by comparing the actual number of cached
+        * nodes with the ic_nnodes count, which is maintained while adding
+        * and removing nodes from the cache.
+        */
+       if ((ifp->if_flags & IFF_DEBUG) && cache_timeout &&
+           nnodes != ic->ic_nnodes)
+               printf("%s: number of cached nodes is %d, expected %d,"
+                   "possible nodes leak\n", ifp->if_xname, nnodes,
+                   ic->ic_nnodes);
+#endif
        splx(s);
 }

Reply via email to