Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 19 Nov 2021 00:02:36 GMT
From:      "Bjoern A. Zeeb" <bz@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org
Subject:   git: 8a5ff0163853 - stable/13 - net80211: correct input_sta length checks and control frame handling
Message-ID:  <202111190002.1AJ02aLk072457@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch stable/13 has been updated by bz:

URL: https://cgit.FreeBSD.org/src/commit/?id=8a5ff0163853a4ccda3aaf5bbce33dfc6bbff4b8

commit 8a5ff0163853a4ccda3aaf5bbce33dfc6bbff4b8
Author:     Bjoern A. Zeeb <bz@FreeBSD.org>
AuthorDate: 2021-09-30 16:41:19 +0000
Commit:     Bjoern A. Zeeb <bz@FreeBSD.org>
CommitDate: 2021-11-19 00:01:25 +0000

    net80211: correct input_sta length checks and control frame handling
    
    Correct input_sta "assertion" checks.  CTS/ACK CTRL frames are shorter
    then sizeof(struct ieee80211_frame_min) and were thus running into the
    is_rx_tooshort error case.
    Use ieee80211_anyhdrsize() to handle this better but make sure we do
    at least have the first 2 octets needed for that.
    While here move the safety checks before any code which may not obey
    them later, just for good style.
    
    The non-scanning check further down assumes a frame format also not
    matching control frames.  For now skip the checks for control frames
    which allows us to deal with some of them at least now.
    
    Sponsored by:   The FreeBSD Foundation
    Obtained from:  20210906 wireless v0.91 code drop
    
    (cherry picked from commit 3dc7a1897e0bb9e4b529c01cb3f88e1c387af5e8)
---
 sys/net80211/ieee80211_sta.c | 66 +++++++++++++++++++++++++-------------------
 1 file changed, 37 insertions(+), 29 deletions(-)

diff --git a/sys/net80211/ieee80211_sta.c b/sys/net80211/ieee80211_sta.c
index cd62266ab942..7ea6187332b1 100644
--- a/sys/net80211/ieee80211_sta.c
+++ b/sys/net80211/ieee80211_sta.c
@@ -552,6 +552,35 @@ sta_input(struct ieee80211_node *ni, struct mbuf *m,
 	int is_hw_decrypted = 0;
 	int has_decrypted = 0;
 
+	KASSERT(ni != NULL, ("%s: null node, mbuf %p", __func__, m));
+
+	/* Early init in case of early error case. */
+	type = -1;
+
+	/*
+	 * Bit of a cheat here, we use a pointer for a 3-address
+	 * frame format but don't reference fields past outside
+	 * ieee80211_frame_min (or other shorter frames) w/o first
+	 * validating the data is present.
+	 */
+	wh = mtod(m, struct ieee80211_frame *);
+
+	if (m->m_pkthdr.len < 2 || m->m_pkthdr.len < ieee80211_anyhdrsize(wh)) {
+		IEEE80211_DISCARD_MAC(vap, IEEE80211_MSG_ANY,
+		    ni->ni_macaddr, NULL,
+		    "too short (1): len %u", m->m_pkthdr.len);
+		vap->iv_stats.is_rx_tooshort++;
+		goto err;
+	}
+	if ((wh->i_fc[0] & IEEE80211_FC0_VERSION_MASK) !=
+	    IEEE80211_FC0_VERSION_0) {
+		IEEE80211_DISCARD_MAC(vap, IEEE80211_MSG_ANY,
+		    ni->ni_macaddr, NULL, "wrong version, fc %02x:%02x",
+		    wh->i_fc[0], wh->i_fc[1]);
+		vap->iv_stats.is_rx_badversion++;
+		goto err;
+	}
+
 	/*
 	 * Some devices do hardware decryption all the way through
 	 * to pretending the frame wasn't encrypted in the first place.
@@ -569,7 +598,6 @@ sta_input(struct ieee80211_node *ni, struct mbuf *m,
 		 * with the M_AMPDU_MPDU flag and we can bypass most of
 		 * the normal processing.
 		 */
-		wh = mtod(m, struct ieee80211_frame *);
 		type = IEEE80211_FC0_TYPE_DATA;
 		dir = wh->i_fc[1] & IEEE80211_FC1_DIR_MASK;
 		subtype = IEEE80211_FC0_SUBTYPE_QOS;
@@ -577,39 +605,19 @@ sta_input(struct ieee80211_node *ni, struct mbuf *m,
 		goto resubmit_ampdu;
 	}
 
-	KASSERT(ni != NULL, ("null node"));
 	ni->ni_inact = ni->ni_inact_reload;
 
-	type = -1;			/* undefined */
-
-	if (m->m_pkthdr.len < sizeof(struct ieee80211_frame_min)) {
-		IEEE80211_DISCARD_MAC(vap, IEEE80211_MSG_ANY,
-		    ni->ni_macaddr, NULL,
-		    "too short (1): len %u", m->m_pkthdr.len);
-		vap->iv_stats.is_rx_tooshort++;
-		goto out;
-	}
-	/*
-	 * Bit of a cheat here, we use a pointer for a 3-address
-	 * frame format but don't reference fields past outside
-	 * ieee80211_frame_min w/o first validating the data is
-	 * present.
-	 */
-	wh = mtod(m, struct ieee80211_frame *);
-
-	if ((wh->i_fc[0] & IEEE80211_FC0_VERSION_MASK) !=
-	    IEEE80211_FC0_VERSION_0) {
-		IEEE80211_DISCARD_MAC(vap, IEEE80211_MSG_ANY,
-		    ni->ni_macaddr, NULL, "wrong version, fc %02x:%02x",
-		    wh->i_fc[0], wh->i_fc[1]);
-		vap->iv_stats.is_rx_badversion++;
-		goto err;
-	}
-
 	dir = wh->i_fc[1] & IEEE80211_FC1_DIR_MASK;
 	type = wh->i_fc[0] & IEEE80211_FC0_TYPE_MASK;
 	subtype = wh->i_fc[0] & IEEE80211_FC0_SUBTYPE_MASK;
-	if ((ic->ic_flags & IEEE80211_F_SCAN) == 0) {
+	/*
+	 * Control frames are not folowing the header scheme of data and mgmt
+	 * frames so we do not apply extra checks here.
+	 * We probably should do checks on RA (+TA) where available for those
+	 * too, but for now do not drop them.
+	 */
+	if (type != IEEE80211_FC0_TYPE_CTL &&
+	    (ic->ic_flags & IEEE80211_F_SCAN) == 0) {
 		bssid = wh->i_addr2;
 		if (!IEEE80211_ADDR_EQ(bssid, ni->ni_bssid)) {
 			/* not interested in */



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202111190002.1AJ02aLk072457>