svn commit: r350619 - in stable: 11/usr.sbin/bhyve 12/usr.sbin/bhyve

[John Baldwin]

Author: jhb
Date: Mon Aug  5 22:04:16 2019
New Revision: 350619
URL: https://svnweb.freebsd.org/changeset/base/350619

Log:
  MFC 350618:
  Validate guest-supplied length of headers for TSO transmit requests.
  
  When transmitting a large TCP packet, the final transmit descriptor
  includes the length of the protocol headers to be duplicated on each
  segment.  The device model was trusting the guest-supplied value
  without validating it.  A value of zero would result in the guest
  being able to indirect a garbage pointer on the stack to overwrite
  arbitrary memory in the bhyve process.  A value that was non-zero but
  too small for the requested parameters resulted in the device model
  reading and writing values beyond the end of the on-stack buffer used
  to hold the template header.
  
  To fix, validate the supplied length and drop requests to transmit
  packets that would overflow the header buffer.  While here, initialize
  the header pointer to NULL as a preventive measure so that any access
  to an unallocated template header crashes they hypervisor
  deterministically.
  
  While here, only read the TCP sequence number if the packet being
  split is a TCP packet.  The e1000 logic supports a segmentation of UDP
  frames, and while UDP segmentation requires this part of the header to
  be valid (so there is no buffer overflow), only reading the field when
  needed is cleaner.
  
  admbugs:	918
  Reported by:	Reno Robert <renorobert at gmail.com>
  Approved by:	so
  Security:	CVE-2019-5609

Modified:
  stable/12/usr.sbin/bhyve/pci_e82545.c
Directory Properties:
  stable/12/   (props changed)

Changes in other areas also in this revision:
Modified:
  stable/11/usr.sbin/bhyve/pci_e82545.c
Directory Properties:
  stable/11/   (props changed)

Modified: stable/12/usr.sbin/bhyve/pci_e82545.c
==============================================================================
--- stable/12/usr.sbin/bhyve/pci_e82545.c	Mon Aug  5 21:39:55 2019	(r350618)
+++ stable/12/usr.sbin/bhyve/pci_e82545.c	Mon Aug  5 22:04:16 2019	(r350619)
@@ -1080,8 +1080,9 @@ e82545_transmit(struct e82545_softc *sc, uint16_t head
 	struct ck_info ckinfo[2];
 	struct iovec *iov;
 	union  e1000_tx_udesc *dsc;
-	int desc, dtype, len, ntype, iovcnt, tlen, hdrlen, vlen, tcp, tso;
+	int desc, dtype, len, ntype, iovcnt, tlen, tcp, tso;
 	int mss, paylen, seg, tiovcnt, left, now, nleft, nnow, pv, pvoff;
+	unsigned hdrlen, vlen;
 	uint32_t tcpsum, tcpseq;
 	uint16_t ipcs, tcpcs, ipid, ohead;
 
@@ -1225,6 +1226,68 @@ e82545_transmit(struct e82545_softc *sc, uint16_t head
 	} else {
 		/* In case of TSO header length provided by software. */
 		hdrlen = sc->esc_txctx.tcp_seg_setup.fields.hdr_len;
+
+		/*
+		 * Cap the header length at 240 based on 7.2.4.5 of
+		 * the Intel 82576EB (Rev 2.63) datasheet.
+		 */
+		if (hdrlen > 240) {
+			WPRINTF("TSO hdrlen too large: %d\r\n", hdrlen);
+			goto done;
+		}
+
+		/*
+		 * If VLAN insertion is requested, ensure the header
+		 * at least holds the amount of data copied during
+		 * VLAN insertion below.
+		 *
+		 * XXX: Realistic packets will include a full Ethernet
+		 * header before the IP header at ckinfo[0].ck_start,
+		 * but this check is sufficient to prevent
+		 * out-of-bounds access below.
+		 */
+		if (vlen != 0 && hdrlen < ETHER_ADDR_LEN*2) {
+			WPRINTF("TSO hdrlen too small for vlan insertion "
+			    "(%d vs %d) -- dropped\r\n", hdrlen,
+			    ETHER_ADDR_LEN*2);
+			goto done;
+		}
+
+		/*
+		 * Ensure that the header length covers the used fields
+		 * in the IP and TCP headers as well as the IP and TCP
+		 * checksums.  The following fields are accessed below:
+		 *
+		 * Header | Field | Offset | Length
+		 * -------+-------+--------+-------
+		 * IPv4   | len   | 2      | 2
+		 * IPv4   | ID    | 4      | 2
+		 * IPv6   | len   | 4      | 2
+		 * TCP    | seq # | 4      | 4
+		 * TCP    | flags | 13     | 1
+		 * UDP    | len   | 4      | 4
+		 */
+		if (hdrlen < ckinfo[0].ck_start + 6 ||
+		    hdrlen < ckinfo[0].ck_off + 2) {
+			WPRINTF("TSO hdrlen too small for IP fields (%d) "
+			    "-- dropped\r\n", hdrlen);
+			goto done;
+		}
+		if (sc->esc_txctx.cmd_and_length & E1000_TXD_CMD_TCP) {
+			if (hdrlen < ckinfo[1].ck_start + 14 ||
+			    (ckinfo[1].ck_valid &&
+			    hdrlen < ckinfo[1].ck_off + 2)) {
+				WPRINTF("TSO hdrlen too small for TCP fields "
+				    "(%d) -- dropped\r\n", hdrlen);
+				goto done;
+			}
+		} else {
+			if (hdrlen < ckinfo[1].ck_start + 8) {
+				WPRINTF("TSO hdrlen too small for UDP fields "
+				    "(%d) -- dropped\r\n", hdrlen);
+				goto done;
+			}
+		}
 	}
 
 	/* Allocate, fill and prepend writable header vector. */
@@ -1246,7 +1309,8 @@ e82545_transmit(struct e82545_softc *sc, uint16_t head
 		iovcnt++;
 		iov->iov_base = hdr;
 		iov->iov_len = hdrlen;
-	}
+	} else
+		hdr = NULL;
 
 	/* Insert VLAN tag. */
 	if (vlen != 0) {
@@ -1288,7 +1352,9 @@ e82545_transmit(struct e82545_softc *sc, uint16_t head
 	DPRINTF("tx %s segmentation offload %d+%d/%d bytes %d iovs\r\n",
 	    tcp ? "TCP" : "UDP", hdrlen, paylen, mss, iovcnt);
 	ipid = ntohs(*(uint16_t *)&hdr[ckinfo[0].ck_start + 4]);
-	tcpseq = ntohl(*(uint32_t *)&hdr[ckinfo[1].ck_start + 4]);
+	tcpseq = 0;
+	if (tcp)
+		tcpseq = ntohl(*(uint32_t *)&hdr[ckinfo[1].ck_start + 4]);
 	ipcs = *(uint16_t *)&hdr[ckinfo[0].ck_off];
 	tcpcs = 0;
 	if (ckinfo[1].ck_valid)	/* Save partial pseudo-header checksum. */