Skip site navigation (1)Skip section navigation (2) 
Date:      Sat, 30 Sep 2017 21:00:46 +0000 (UTC)
From:      Andriy Voskoboinyk <avos@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r324144 - head/sys/dev/usb/wlan
Message-ID:  <201709302100.v8UL0kmr046139@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: avos
Date: Sat Sep 30 21:00:46 2017
New Revision: 324144
URL: https://svnweb.freebsd.org/changeset/base/324144

Log:
  uath(4): fix varible types, add missing checks for descriptor / command
  header structure fields.
  
  Reported by:	hselasky
  Reviewed by:	hselasky
  Differential Revision:	https://reviews.freebsd.org/D11786

Modified:
  head/sys/dev/usb/wlan/if_uath.c

Modified: head/sys/dev/usb/wlan/if_uath.c
==============================================================================
--- head/sys/dev/usb/wlan/if_uath.c	Sat Sep 30 21:00:08 2017	(r324143)
+++ head/sys/dev/usb/wlan/if_uath.c	Sat Sep 30 21:00:46 2017	(r324144)
@@ -2201,17 +2201,19 @@ uath_sysctl_node(struct uath_softc *sc)
 
 #undef UATH_SYSCTL_STAT_ADD32
 
+CTASSERT(sizeof(u_int) >= sizeof(uint32_t));
+
 static void
 uath_cmdeof(struct uath_softc *sc, struct uath_cmd *cmd)
 {
 	struct uath_cmd_hdr *hdr;
-	int dlen;
+	uint32_t dlen;
 
 	hdr = (struct uath_cmd_hdr *)cmd->buf;
 	/* NB: msgid is passed thru w/o byte swapping */
 #ifdef UATH_DEBUG
 	if (sc->sc_debug & UATH_DEBUG_CMDS) {
-		int len = be32toh(hdr->len);
+		uint32_t len = be32toh(hdr->len);
 		printf("%s: %s [ix %u] len %u status %u\n",
 		    __func__, uath_codename(be32toh(hdr->code)),
 		    hdr->msgid, len, be32toh(hdr->magic));
@@ -2227,15 +2229,9 @@ uath_cmdeof(struct uath_softc *sc, struct uath_cmd *cm
 	switch (hdr->code & 0xff) {
 	/* reply to a read command */
 	default:
-		dlen = hdr->len - sizeof(*hdr);
-		if (dlen < 0) {
-			device_printf(sc->sc_dev,
-			    "Invalid header length %d\n", dlen);
-			return;
-		}
 		DPRINTF(sc, UATH_DEBUG_RX_PROC | UATH_DEBUG_RECV_ALL,
-		    "%s: code %d data len %u\n",
-		    __func__, hdr->code & 0xff, dlen);
+		    "%s: code %d hdr len %u\n",
+		    __func__, hdr->code & 0xff, hdr->len);
 		/*
 		 * The first response from the target after the
 		 * HOST_AVAILABLE has an invalid msgid so we must
@@ -2245,8 +2241,8 @@ uath_cmdeof(struct uath_softc *sc, struct uath_cmd *cm
 			uint32_t *rp = (uint32_t *)(hdr+1);
 			u_int olen;
 
-			if (!(sizeof(*hdr) <= hdr->len &&
-			      hdr->len < UATH_MAX_CMDSZ)) {
+			if (sizeof(*hdr) > hdr->len ||
+			    hdr->len >= UATH_MAX_CMDSZ) {
 				device_printf(sc->sc_dev,
 				    "%s: invalid WDC msg length %u; "
 				    "msg ignored\n", __func__, hdr->len);
@@ -2258,7 +2254,8 @@ uath_cmdeof(struct uath_softc *sc, struct uath_cmd *cm
 			 * number of bytes--unless it's 0 in which
 			 * case a single 32-bit word should be present.
 			 */
-			if (dlen >= (int)sizeof(uint32_t)) {
+			dlen = hdr->len - sizeof(*hdr);
+			if (dlen >= sizeof(uint32_t)) {
 				olen = be32toh(rp[0]);
 				dlen -= sizeof(uint32_t);
 				if (olen == 0) {
@@ -2278,7 +2275,7 @@ uath_cmdeof(struct uath_softc *sc, struct uath_cmd *cm
 					    cmd->olen);
 					olen = cmd->olen;
 				}
-				if (olen > (u_int)dlen) {
+				if (olen > dlen) {
 					/* XXX complain, shouldn't happen */
 					device_printf(sc->sc_dev,
 					    "%s: cmd 0x%x olen %u dlen %u\n",
@@ -2300,8 +2297,10 @@ uath_cmdeof(struct uath_softc *sc, struct uath_cmd *cm
 			return;
 		}
 		dlen = hdr->len - sizeof(*hdr);
-		if (dlen != (int)sizeof(uint32_t)) {
-			/* XXX something wrong */
+		if (dlen != sizeof(uint32_t)) {
+			device_printf(sc->sc_dev,
+			    "%s: dlen (%u) != %zu!\n",
+			    __func__, dlen, sizeof(uint32_t));
 			return;
 		}
 		/* XXX have submitter do this */
@@ -2330,6 +2329,7 @@ uath_intr_rx_callback(struct usb_xfer *xfer, usb_error
 {
 	struct uath_softc *sc = usbd_xfer_softc(xfer);
 	struct uath_cmd *cmd;
+	struct uath_cmd_hdr *hdr;
 	struct usb_page_cache *pc;
 	int actlen;
 
@@ -2347,10 +2347,25 @@ uath_intr_rx_callback(struct usb_xfer *xfer, usb_error
 		STAILQ_INSERT_TAIL(&sc->sc_cmd_inactive, cmd, next);
 		UATH_STAT_INC(sc, st_cmd_inactive);
 
-		KASSERT(actlen >= (int)sizeof(struct uath_cmd_hdr),
-		    ("short xfer error"));
+		if (actlen < sizeof(struct uath_cmd_hdr)) {
+			device_printf(sc->sc_dev,
+			    "%s: short xfer error (actlen %d)\n",
+			    __func__, actlen);
+			goto setup;
+		}
+
 		pc = usbd_xfer_get_frame(xfer, 0);
 		usbd_copy_out(pc, 0, cmd->buf, actlen);
+
+		hdr = (struct uath_cmd_hdr *)cmd->buf;
+		hdr->len = be32toh(hdr->len);
+		if (hdr->len > (uint32_t)actlen) {
+			device_printf(sc->sc_dev,
+			    "%s: truncated xfer (len %u, actlen %d)\n",
+			    __func__, hdr->len, actlen);
+			goto setup;
+		}
+
 		uath_cmdeof(sc, cmd);
 	case USB_ST_SETUP:
 setup:
@@ -2451,6 +2466,8 @@ uath_update_rxstat(struct uath_softc *sc, uint32_t sta
 	}
 }
 
+CTASSERT(UATH_MIN_RXBUFSZ >= sizeof(struct uath_chunk));
+
 static struct mbuf *
 uath_data_rxeof(struct usb_xfer *xfer, struct uath_data *data,
     struct uath_rx_desc **pdesc)
@@ -2473,13 +2490,24 @@ uath_data_rxeof(struct usb_xfer *xfer, struct uath_dat
 	}
 
 	chunk = (struct uath_chunk *)data->buf;
-	if (chunk->seqnum == 0 && chunk->flags == 0 && chunk->length == 0) {
+	chunklen = be16toh(chunk->length);
+	if (chunk->seqnum == 0 && chunk->flags == 0 && chunklen == 0) {
 		device_printf(sc->sc_dev, "%s: strange response\n", __func__);
 		counter_u64_add(ic->ic_ierrors, 1);
 		UATH_RESET_INTRX(sc);
 		return (NULL);
 	}
 
+	if (chunklen > actlen) {
+		device_printf(sc->sc_dev,
+		    "%s: invalid chunk length (len %u > actlen %d)\n",
+		    __func__, chunklen, actlen);
+		counter_u64_add(ic->ic_ierrors, 1);
+		/* XXX cleanup? */
+		UATH_RESET_INTRX(sc);
+		return (NULL);
+	}
+
 	if (chunk->seqnum != sc->sc_intrx_nextnum) {
 		DPRINTF(sc, UATH_DEBUG_XMIT, "invalid seqnum %d, expected %d\n",
 		    chunk->seqnum, sc->sc_intrx_nextnum);
@@ -2496,9 +2524,19 @@ uath_data_rxeof(struct usb_xfer *xfer, struct uath_dat
 	    chunk->flags & UATH_CFLAGS_RXMSG)
 		UATH_STAT_INC(sc, st_multichunk);
 
-	chunklen = be16toh(chunk->length);
-	if (chunk->flags & UATH_CFLAGS_FINAL)
+	if (chunk->flags & UATH_CFLAGS_FINAL) {
+		if (chunklen < sizeof(struct uath_rx_desc)) {
+			device_printf(sc->sc_dev,
+			    "%s: invalid chunk length %d\n",
+			    __func__, chunklen);
+			counter_u64_add(ic->ic_ierrors, 1);
+			if (sc->sc_intrx_head != NULL)
+				m_freem(sc->sc_intrx_head);
+			UATH_RESET_INTRX(sc);
+			return (NULL);
+		}
 		chunklen -= sizeof(struct uath_rx_desc);
+	}
 
 	if (chunklen > 0 &&
 	    (!(chunk->flags & UATH_CFLAGS_FINAL) || !(chunk->seqnum == 0))) {
@@ -2559,6 +2597,19 @@ uath_data_rxeof(struct usb_xfer *xfer, struct uath_dat
 		(struct uath_rx_desc *)(((uint8_t *)chunk) + 
 		    sizeof(struct uath_chunk) + be16toh(chunk->length) -
 		    sizeof(struct uath_rx_desc));
+	if ((uint8_t *)chunk + actlen - sizeof(struct uath_rx_desc) <
+	    (uint8_t *)desc) {
+		device_printf(sc->sc_dev,
+		    "%s: wrong Rx descriptor pointer "
+		    "(desc %p chunk %p actlen %d)\n",
+		    __func__, desc, chunk, actlen);
+		counter_u64_add(ic->ic_ierrors, 1);
+		if (sc->sc_intrx_head != NULL)
+			m_freem(sc->sc_intrx_head);
+		UATH_RESET_INTRX(sc);
+		return (NULL);
+	}
+
 	*pdesc = desc;
 
 	DPRINTF(sc, UATH_DEBUG_RECV | UATH_DEBUG_RECV_ALL,
@@ -2586,8 +2637,33 @@ uath_data_rxeof(struct usb_xfer *xfer, struct uath_dat
 
 	/* finalize mbuf */
 	if (sc->sc_intrx_head == NULL) {
-		m->m_pkthdr.len = m->m_len =
-			be32toh(desc->framelen) - UATH_RX_DUMMYSIZE;
+		uint32_t framelen;
+
+		if (be32toh(desc->framelen) < UATH_RX_DUMMYSIZE) {
+			device_printf(sc->sc_dev,
+			    "%s: framelen too small (%u)\n",
+			    __func__, be32toh(desc->framelen));
+			counter_u64_add(ic->ic_ierrors, 1);
+			if (sc->sc_intrx_head != NULL)
+				m_freem(sc->sc_intrx_head);
+			UATH_RESET_INTRX(sc);
+			return (NULL);
+		}
+
+		framelen = be32toh(desc->framelen) - UATH_RX_DUMMYSIZE;
+		if (framelen > actlen - sizeof(struct uath_chunk) ||
+		    framelen < sizeof(struct ieee80211_frame_ack)) {
+			device_printf(sc->sc_dev,
+			    "%s: wrong frame length (%u, actlen %d)!\n",
+			    __func__, framelen, actlen);
+			counter_u64_add(ic->ic_ierrors, 1);
+			if (sc->sc_intrx_head != NULL)
+				m_freem(sc->sc_intrx_head);
+			UATH_RESET_INTRX(sc);
+			return (NULL);
+		}
+
+		m->m_pkthdr.len = m->m_len = framelen;
 		m->m_data += sizeof(struct uath_chunk);
 	} else {
 		mp = sc->sc_intrx_head;

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