From owner-svn-src-stable@freebsd.org Mon Jan 28 01:28:18 2019 Return-Path: Delivered-To: svn-src-stable@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id C843414C4744; Mon, 28 Jan 2019 01:28:18 +0000 (UTC) (envelope-from avos@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) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 5EFE48F799; Mon, 28 Jan 2019 01:28:18 +0000 (UTC) (envelope-from avos@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4733D186A6; Mon, 28 Jan 2019 01:28:18 +0000 (UTC) (envelope-from avos@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id x0S1SI9r091078; Mon, 28 Jan 2019 01:28:18 GMT (envelope-from avos@FreeBSD.org) Received: (from avos@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id x0S1SIhu091077; Mon, 28 Jan 2019 01:28:18 GMT (envelope-from avos@FreeBSD.org) Message-Id: <201901280128.x0S1SIhu091077@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: avos set sender to avos@FreeBSD.org using -f From: Andriy Voskoboinyk Date: Mon, 28 Jan 2019 01:28:18 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-12@freebsd.org Subject: svn commit: r343511 - stable/12/sys/dev/usb/wlan X-SVN-Group: stable-12 X-SVN-Commit-Author: avos X-SVN-Commit-Paths: stable/12/sys/dev/usb/wlan X-SVN-Commit-Revision: 343511 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Rspamd-Queue-Id: 5EFE48F799 X-Spamd-Bar: -- Authentication-Results: mx1.freebsd.org X-Spamd-Result: default: False [-2.96 / 15.00]; local_wl_from(0.00)[FreeBSD.org]; NEURAL_HAM_MEDIUM(-1.00)[-0.998,0]; NEURAL_HAM_LONG(-1.00)[-0.999,0]; NEURAL_HAM_SHORT(-0.96)[-0.960,0]; ASN(0.00)[asn:11403, ipnet:2610:1c1:1::/48, country:US] X-BeenThere: svn-src-stable@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: SVN commit messages for all the -stable branches of the src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 28 Jan 2019 01:28:19 -0000 Author: avos Date: Mon Jan 28 01:28:17 2019 New Revision: 343511 URL: https://svnweb.freebsd.org/changeset/base/343511 Log: MFC r343234: run(4): add more length checks in Rx path. - Discard frames that are bigger than MCLBYTES (to prevent buffer overrun). - Check buffer length before accessing its contents. - Fix len <-> dmalen check - the last includes Rx Wireless information structure size. - Fix out-of-bounds read during Rx node search for ACK / CTS frames (monitor mode only). While here: - Mark few suspicious places with comments. - Move common cleanup to the function end. Modified: stable/12/sys/dev/usb/wlan/if_run.c Directory Properties: stable/12/ (props changed) Modified: stable/12/sys/dev/usb/wlan/if_run.c ============================================================================== --- stable/12/sys/dev/usb/wlan/if_run.c Mon Jan 28 01:19:43 2019 (r343510) +++ stable/12/sys/dev/usb/wlan/if_run.c Mon Jan 28 01:28:17 2019 (r343511) @@ -2824,69 +2824,80 @@ run_rx_frame(struct run_softc *sc, struct mbuf *m, uin uint8_t ant, rssi; int8_t nf; - rxwi = mtod(m, struct rt2860_rxwi *); - len = le16toh(rxwi->len) & 0xfff; rxwisize = sizeof(struct rt2860_rxwi); if (sc->mac_ver == 0x5592) rxwisize += sizeof(uint64_t); else if (sc->mac_ver == 0x3593) rxwisize += sizeof(uint32_t); - if (__predict_false(len > dmalen)) { - m_freem(m); - counter_u64_add(ic->ic_ierrors, 1); + + if (__predict_false(dmalen < + rxwisize + sizeof(struct ieee80211_frame_ack))) { RUN_DPRINTF(sc, RUN_DEBUG_RECV, + "payload is too short: dma length %u < %zu\n", + dmalen, rxwisize + sizeof(struct ieee80211_frame_ack)); + goto fail; + } + + rxwi = mtod(m, struct rt2860_rxwi *); + len = le16toh(rxwi->len) & 0xfff; + + if (__predict_false(len > dmalen - rxwisize)) { + RUN_DPRINTF(sc, RUN_DEBUG_RECV, "bad RXWI length %u > %u\n", len, dmalen); - return; + goto fail; } + /* Rx descriptor is located at the end */ rxd = (struct rt2870_rxd *)(mtod(m, caddr_t) + dmalen); flags = le32toh(rxd->flags); if (__predict_false(flags & (RT2860_RX_CRCERR | RT2860_RX_ICVERR))) { - m_freem(m); - counter_u64_add(ic->ic_ierrors, 1); RUN_DPRINTF(sc, RUN_DEBUG_RECV, "%s error.\n", (flags & RT2860_RX_CRCERR)?"CRC":"ICV"); - return; + goto fail; } + if (flags & RT2860_RX_L2PAD) { + /* + * XXX OpenBSD removes padding between header + * and payload here... + */ + RUN_DPRINTF(sc, RUN_DEBUG_RECV, + "received RT2860_RX_L2PAD frame\n"); + len += 2; + } + m->m_data += rxwisize; - m->m_pkthdr.len = m->m_len -= rxwisize; + m->m_pkthdr.len = m->m_len = len; wh = mtod(m, struct ieee80211_frame *); + /* XXX wrong for monitor mode */ if (wh->i_fc[1] & IEEE80211_FC1_PROTECTED) { wh->i_fc[1] &= ~IEEE80211_FC1_PROTECTED; m->m_flags |= M_WEP; } - if (flags & RT2860_RX_L2PAD) { - RUN_DPRINTF(sc, RUN_DEBUG_RECV, - "received RT2860_RX_L2PAD frame\n"); - len += 2; - } + if (len >= sizeof(struct ieee80211_frame_min)) { + ni = ieee80211_find_rxnode(ic, + mtod(m, struct ieee80211_frame_min *)); + } else + ni = NULL; - ni = ieee80211_find_rxnode(ic, - mtod(m, struct ieee80211_frame_min *)); - if (__predict_false(flags & RT2860_RX_MICERR)) { /* report MIC failures to net80211 for TKIP */ if (ni != NULL) ieee80211_notify_michael_failure(ni->ni_vap, wh, rxwi->keyidx); - m_freem(m); - counter_u64_add(ic->ic_ierrors, 1); RUN_DPRINTF(sc, RUN_DEBUG_RECV, "MIC error. Someone is lying.\n"); - return; + goto fail; } ant = run_maxrssi_chain(sc, rxwi); rssi = rxwi->rssi[ant]; nf = run_rssi2dbm(sc, rssi, ant); - m->m_pkthdr.len = m->m_len = len; - if (__predict_false(ieee80211_radiotap_active(ic))) { struct run_rx_radiotap_header *tap = &sc->sc_rxtap; uint16_t phy; @@ -2934,6 +2945,12 @@ run_rx_frame(struct run_softc *sc, struct mbuf *m, uin } else { (void)ieee80211_input_all(ic, m, rssi, nf); } + + return; + +fail: + m_freem(m); + counter_u64_add(ic->ic_ierrors, 1); } static void @@ -2943,7 +2960,7 @@ run_bulk_rx_callback(struct usb_xfer *xfer, usb_error_ struct ieee80211com *ic = &sc->sc_ic; struct mbuf *m = NULL; struct mbuf *m0; - uint32_t dmalen; + uint32_t dmalen, mbuf_len; uint16_t rxwisize; int xferlen; @@ -3049,6 +3066,14 @@ tr_setup: break; } + mbuf_len = dmalen + sizeof(struct rt2870_rxd); + if (__predict_false(mbuf_len > MCLBYTES)) { + RUN_DPRINTF(sc, RUN_DEBUG_RECV_DESC | RUN_DEBUG_USB, + "payload is too big: mbuf_len %u\n", mbuf_len); + counter_u64_add(ic->ic_ierrors, 1); + break; + } + /* copy aggregated frames to another mbuf */ m0 = m_getcl(M_NOWAIT, MT_DATA, M_PKTHDR); if (__predict_false(m0 == NULL)) { @@ -3058,14 +3083,13 @@ tr_setup: break; } m_copydata(m, 4 /* skip 32-bit DMA-len header */, - dmalen + sizeof(struct rt2870_rxd), mtod(m0, caddr_t)); - m0->m_pkthdr.len = m0->m_len = - dmalen + sizeof(struct rt2870_rxd); + mbuf_len, mtod(m0, caddr_t)); + m0->m_pkthdr.len = m0->m_len = mbuf_len; run_rx_frame(sc, m0, dmalen); /* update data ptr */ - m->m_data += dmalen + 8; - m->m_pkthdr.len = m->m_len -= dmalen + 8; + m->m_data += mbuf_len + 4; + m->m_pkthdr.len = m->m_len -= mbuf_len + 4; } /* make sure we free the source buffer, if any */