From nobody Fri Oct 22 10:45:44 2021 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 9171C17F1B76; Fri, 22 Oct 2021 10:45:45 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4HbLbs0KHYz3JQk; Fri, 22 Oct 2021 10:45:44 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id CBF56514F; Fri, 22 Oct 2021 10:45:44 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 19MAjiHB091334; Fri, 22 Oct 2021 10:45:44 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 19MAjiib091333; Fri, 22 Oct 2021 10:45:44 GMT (envelope-from git) Date: Fri, 22 Oct 2021 10:45:44 GMT Message-Id: <202110221045.19MAjiib091333@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: "Bjoern A. Zeeb" Subject: git: 3dc7a1897e0b - main - net80211: correct input_sta length checks and control frame handling List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: bz X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 3dc7a1897e0bb9e4b529c01cb3f88e1c387af5e8 Auto-Submitted: auto-generated X-ThisMailContainsUnwantedMimeParts: N The branch main has been updated by bz: URL: https://cgit.FreeBSD.org/src/commit/?id=3dc7a1897e0bb9e4b529c01cb3f88e1c387af5e8 commit 3dc7a1897e0bb9e4b529c01cb3f88e1c387af5e8 Author: Bjoern A. Zeeb AuthorDate: 2021-09-30 16:41:19 +0000 Commit: Bjoern A. Zeeb CommitDate: 2021-10-22 10:42:06 +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 MFC after: 3 days Reviewed by: adrian Differential Revision: https://reviews.freebsd.org/D32238 --- 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 */