trim the FCS off Ethernet packets before sending them up the stack.
authordlg <dlg@openbsd.org>
Sat, 27 Mar 2021 01:12:01 +0000 (01:12 +0000)
committerdlg <dlg@openbsd.org>
Sat, 27 Mar 2021 01:12:01 +0000 (01:12 +0000)
Jurjen Oskam on tech@ found that ure in a veb caused these extra
fcs bytes to be transmitted by other veb members. the extra bytes
aren't a problem usually because our network stack ignores them if
they're present, eg, the ip stack reads an ip packet length and
trims bytes in an mbuf if there's more. bridge(4) masked this problem
because it always parses IP packets going over the bridge and trims
them like the IP stack before pushing them out another port.

veb(4) generally just moves packets around based on the Ethernet
header, by default it doesn't look too deeply into packets, which
is why this issue popped out.

it is more correct for ure to just not pass the fcs bytes up.

ok jmatthew@ kevlo@

sys/dev/usb/if_ure.c

index 48582e8..7cf5d43 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: if_ure.c,v 1.21 2020/10/14 23:47:55 jsg Exp $ */
+/*     $OpenBSD: if_ure.c,v 1.22 2021/03/27 01:12:01 dlg Exp $ */
 /*-
  * Copyright (c) 2015, 2016, 2019 Kevin Lo <kevlo@openbsd.org>
  * Copyright (c) 2020 Jonathon Fletcher <jonathon.fletcher@gmail.com>
@@ -1896,10 +1896,17 @@ ure_rxeof(struct usbd_xfer *xfer, void *priv, usbd_status status)
                        ifp->if_ierrors++;
                        goto done;
                }
+               if (pktlen < ETHER_MIN_LEN) {
+                       DPRINTF(("Ethernet frame is too short\n"));
+                       ifp->if_ierrors++;
+                       goto done;
+               }
 
                total_len -= roundup(pktlen, URE_RX_BUF_ALIGN);
                buf += sizeof(rxhdr);
 
+               /* trim fcs */
+               pktlen -= ETHER_CRC_LEN;
                m = m_devget(buf, pktlen, ETHER_ALIGN);
                if (m == NULL) {
                        DPRINTF(("unable to allocate mbuf for next packet\n"));