From c253dea3fdf4e83d401d331a07b85fb6732d45a9 Mon Sep 17 00:00:00 2001 From: stsp Date: Thu, 1 Jul 2021 11:51:55 +0000 Subject: [PATCH] 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 --- sys/dev/ic/ar5008.c | 45 ++++++++++++++++++++++++++++++++++----------- 1 file changed, 34 insertions(+), 11 deletions(-) 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 @@ -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); -- 2.20.1