d/freebsd-nq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

bhyve e1000: Skip packets with a small header.

Browse Source 
Certain operations such as checksum insertion and VLAN insertion
require the device model to rewrite the packet header.  The first step
in rewriting the packet header is to copy the existing packet header
from the source packet.  This copy is done by copying data from an
iovec array that corresponds to the S/G entries described by transmit
descriptors.  However, if the total packet length is smaller than the
headers that need to be copied as the initial template, this copy can
overflow the iovec array and use garbage values as the source pointer
to memcpy.  The PR used a single descriptor with a length of 0 in its
PoC.

To fix, track the total packet length and drop requests to transmit
packets whose payload is smaller than the required header length.

While here, fix another issue where the final descriptor could have an
invalid length (too short) that could underflow 'len' when stripping
the checksum.  Skip those requests instead, too.

PR:		264372
Reported by:	Robert Morris <rtm@lcs.mit.edu>
Reviewed by:	grehan, markj
MFC after:	1 week
Sponsored by:	The FreeBSD Foundation
Differential Revision:	https://reviews.freebsd.org/D36182
This commit is contained in:
John Baldwin 2022-08-17 10:01:16 -07:00
parent e7439f6aeb
commit fa46f3704b
1 changed files with 29 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

41
usr.sbin/bhyve/pci_e82545.c
View File

@ -233,7 +233,7 @@ struct ck_info {
* Debug printf
*/
static int e82545_debug = 0;
#define WPRINTF(msg,params...) PRINTLN("e82545: " msg, params)
#define WPRINTF(msg,params...) PRINTLN("e82545: " msg, ##params)
#define DPRINTF(msg,params...) if (e82545_debug) WPRINTF(msg, params)
#define MIN(a,b) (((a)<(b))?(a):(b))
@ -1086,15 +1086,18 @@ e82545_transmit(struct e82545_softc *sc, uint16_t head, uint16_t tail,
union e1000_tx_udesc *dsc;
int desc, dtype, len, ntype, iovcnt, tcp, tso;
int mss, paylen, seg, tiovcnt, left, now, nleft, nnow, pv, pvoff;
unsigned hdrlen, vlen;
unsigned hdrlen, vlen, pktlen;
uint32_t tcpsum, tcpseq;
uint16_t ipcs, tcpcs, ipid, ohead;
bool invalid;
ckinfo[0].ck_valid = ckinfo[1].ck_valid = 0;
iovcnt = 0;
ntype = 0;
tso = 0;
pktlen = 0;
ohead = head;
invalid = false;
/* iovb[0/1] may be used for writable copy of headers. */
iov = &iovb[2];
@ -1144,17 +1147,23 @@ e82545_transmit(struct e82545_softc *sc, uint16_t head, uint16_t tail,
len = (dtype == E1000_TXD_TYP_L) ? dsc->td.lower.flags.length :
dsc->dd.lower.data & 0xFFFFF;
if (len > 0) {
/* Strip checksum supplied by guest. */
if ((dsc->td.lower.data & E1000_TXD_CMD_EOP) != 0 &&
(dsc->td.lower.data & E1000_TXD_CMD_IFCS) == 0)
/* Strip checksum supplied by guest. */
if ((dsc->td.lower.data & E1000_TXD_CMD_EOP) != 0 &&
(dsc->td.lower.data & E1000_TXD_CMD_IFCS) == 0) {
if (len <= 2) {
WPRINTF("final descriptor too short (%d) -- dropped",
len);
invalid = true;
} else
len -= 2;
if (iovcnt < I82545_MAX_TXSEGS) {
iov[iovcnt].iov_base = paddr_guest2host(
sc->esc_ctx, dsc->td.buffer_addr, len);
iov[iovcnt].iov_len = len;
}
}
if (len > 0 && iovcnt < I82545_MAX_TXSEGS) {
iov[iovcnt].iov_base = paddr_guest2host(sc->esc_ctx,
dsc->td.buffer_addr, len);
iov[iovcnt].iov_len = len;
iovcnt++;
pktlen += len;
}
/*
@ -1202,6 +1211,9 @@ e82545_transmit(struct e82545_softc *sc, uint16_t head, uint16_t tail,
}
}
if (invalid)
goto done;
if (iovcnt > I82545_MAX_TXSEGS) {
WPRINTF("tx too many descriptors (%d > %d) -- dropped",
iovcnt, I82545_MAX_TXSEGS);
@ -1295,8 +1307,13 @@ e82545_transmit(struct e82545_softc *sc, uint16_t head, uint16_t tail,
}
}
if (pktlen < hdrlen + vlen) {
WPRINTF("packet too small for writable header");
goto done;
}
/* Allocate, fill and prepend writable header vector. */
if (hdrlen != 0) {
if (hdrlen + vlen != 0) {
hdr = __builtin_alloca(hdrlen + vlen);
hdr += vlen;
for (left = hdrlen, hdrp = hdr; left > 0;