From: stsp <stsp@openbsd.org>
Date: Thu, 1 Jul 2021 11:51:55 +0000 (+0000)
Subject: Prevent athn(4) from calling ieee80211_find_rxnode() on bad frames.
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=c253dea3fdf4e83d401d331a07b85fb6732d45a9;p=openbsd

Prevent athn(4) from calling ieee80211_find_rxnode() on bad frames.

This fixes an issue introduced with our workaround for bogus michael
mic failures seen when hardware receives control frames. We do need
to ignore the michael mic failure in this case but we should not call
ieee80211_find_rxnode() on such frames unconditionally. Do this only
if the transmitter's address has already been cached.

When ieee80211_find_rxnode() is called with an unknown source MAC address
it will create a new entry in the node cache. Frames flagged as incorrectly
received by hardware should not be passed to ieee80211_find_rxnode() without
further verification to avoid creating bogus cache entries based on corrupt
frame headers.

Prompted by an issue seen by kettenis@ on arm64 where the node cache
contains bogus entries. This change doesn't fix the issue but it is
a step in the right direction regardless since it fixes one possible
cause for the issue.

ok kettenis@
tested by myself and Mikolaj Kucharski
---

diff --git a/sys/dev/ic/ar5008.c b/sys/dev/ic/ar5008.c
index 816a3b09ee2..ea1f278ed8f 100644
--- a/sys/dev/ic/ar5008.c
+++ b/sys/dev/ic/ar5008.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ar5008.c,v 1.66 2021/05/03 08:23:05 stsp Exp $	*/
+/*	$OpenBSD: ar5008.c,v 1.67 2021/07/01 11:51:55 stsp Exp $	*/
 
 /*-
  * Copyright (c) 2009 Damien Bergamini <damien.bergamini@free.fr>
@@ -970,29 +970,52 @@ ar5008_rx_process(struct athn_softc *sc, struct mbuf_list *ml)
 	/* Finalize mbuf. */
 	m->m_pkthdr.len = m->m_len = len;
 
-	/* Grab a reference to the source node. */
 	wh = mtod(m, struct ieee80211_frame *);
-	ni = ieee80211_find_rxnode(ic, wh);
 
 	if (michael_mic_failure) {
 		/*
 		 * Check that it is not a control frame
 		 * (invalid MIC failures on valid ctl frames).
+		 * Validate the transmitter's address to avoid passing
+		 * corrupt frames with bogus addresses to net80211.
 		 */
-		if (!(wh->i_fc[0] & IEEE80211_FC0_TYPE_CTL) &&
-		    (ic->ic_flags & IEEE80211_F_RSNON) &&
-		    (ni->ni_rsncipher == IEEE80211_CIPHER_TKIP ||
-		    ni->ni_rsngroupcipher == IEEE80211_CIPHER_TKIP)) {
-			/* Report Michael MIC failures to net80211. */
-			ic->ic_stats.is_rx_locmicfail++;
-			ieee80211_michael_mic_failure(ic, 0);
+		if ((wh->i_fc[0] & IEEE80211_FC0_TYPE_CTL)) {
+			switch (ic->ic_opmode) {
+#ifndef IEEE80211_STA_ONLY
+			case IEEE80211_M_HOSTAP:
+				if (ieee80211_find_node(ic, wh->i_addr2))
+					michael_mic_failure = 0;
+				break;
+#endif
+			case IEEE80211_M_STA:
+				if (IEEE80211_ADDR_EQ(wh->i_addr2,
+				    ic->ic_bss->ni_macaddr))
+					michael_mic_failure = 0;
+				break;
+			case IEEE80211_M_MONITOR:
+				michael_mic_failure = 0;
+				break;
+			default:
+				break;
+			}
+		}
+
+		if (michael_mic_failure) {
+ 			/* Report Michael MIC failures to net80211. */
+			if ((ic->ic_rsnciphers & IEEE80211_CIPHER_TKIP) ||
+			    ic->ic_rsngroupcipher == IEEE80211_CIPHER_TKIP) {
+				ic->ic_stats.is_rx_locmicfail++;
+				ieee80211_michael_mic_failure(ic, 0);
+			}
 			ifp->if_ierrors++;
-			ieee80211_release_node(ic, ni);
 			m_freem(m);
 			goto skip;
 		}
 	}
 
+	/* Grab a reference to the source node. */
+	ni = ieee80211_find_rxnode(ic, wh);
+
 	/* Remove any HW padding after the 802.11 header. */
 	if (!(wh->i_fc[0] & IEEE80211_FC0_TYPE_CTL)) {
 		u_int hdrlen = ieee80211_get_hdrlen(wh);