Re: [PATCH] frame length validation for USB ethernet adapters
Hi, I updated the diff for axe(4) based on what Laurent sent me. He says the patch breaks his axe(4). I also added a comment to explain why it's done, in areas where there's a status bit called RX_STATUS. This is based on an issue I encountered with udav(4), wherein despite having a valid status bit, the frame length was bogus. It's even more important that we test this on as much USB adapters as possible. Thanks to the users who are doing a good job at testing the diffs. Index: src/sys/dev/usb/if_axe.c === RCS file: /cvs/src/sys/dev/usb/if_axe.c,v retrieving revision 1.105 diff -u -p -r1.105 if_axe.c --- src/sys/dev/usb/if_axe.c25 Jan 2011 20:03:35 - 1.105 +++ src/sys/dev/usb/if_axe.c21 Mar 2011 19:00:17 - @@ -1018,7 +1018,8 @@ axe_rxeof(usbd_xfer_handle xfer, usbd_pr do { if (sc->axe_flags & AX178 || sc->axe_flags & AX772) { - if (total_len < sizeof(hdr)) { + if (total_len < sizeof(hdr) || + total_len > sc->axe_bufsz) { ifp->if_ierrors++; goto done; } @@ -1048,8 +1049,15 @@ axe_rxeof(usbd_xfer_handle xfer, usbd_pr else total_len -= pktlen; } else { + if (total_len < sizeof(hdr) || + total_len > sc->axe_bufsz) { + ifp->if_ierrors++; + goto done; + } + else { pktlen = total_len; /* crc on the end? */ total_len = 0; + } } m = axe_newbuf(); Index: src/sys/dev/usb/if_aue.c === RCS file: /cvs/src/sys/dev/usb/if_aue.c,v retrieving revision 1.84 diff -u -p -r1.84 if_aue.c --- src/sys/dev/usb/if_aue.c25 Jan 2011 20:03:35 - 1.84 +++ src/sys/dev/usb/if_aue.c21 Mar 2011 19:00:23 - @@ -1078,12 +1078,13 @@ aue_rxeof(usbd_xfer_handle xfer, usbd_pr usbd_get_xfer_status(xfer, NULL, NULL, &total_len, NULL); - memcpy(mtod(c->aue_mbuf, char *), c->aue_buf, total_len); - - if (total_len <= 4 + ETHER_CRC_LEN) { + /* frame may still be valid but length is bogus */ + if (total_len <= 4 + ETHER_CRC_LEN || total_len > AUE_BUFSZ) { ifp->if_ierrors++; goto done; } + + memcpy(mtod(c->aue_mbuf, char *), c->aue_buf, total_len); memcpy(&r, c->aue_buf + total_len - 4, sizeof(r)); Index: src/sys/dev/usb/if_cdce.c === RCS file: /cvs/src/sys/dev/usb/if_cdce.c,v retrieving revision 1.49 diff -u -p -r1.49 if_cdce.c --- src/sys/dev/usb/if_cdce.c 25 Jan 2011 20:03:35 - 1.49 +++ src/sys/dev/usb/if_cdce.c 21 Mar 2011 19:00:25 - @@ -776,8 +776,11 @@ cdce_rxeof(usbd_xfer_handle xfer, usbd_p usbd_get_xfer_status(xfer, NULL, NULL, &total_len, NULL); if (sc->cdce_flags & CDCE_ZAURUS) total_len -= 4; /* Strip off CRC added by Zaurus */ - if (total_len <= 1) + + if (total_len <= 1 || total_len > CDCE_BUFSZ) { + ifp->if_ierrors++; goto done; + } m = c->cdce_mbuf; memcpy(mtod(m, char *), c->cdce_buf, total_len); Index: src/sys/dev/usb/if_cue.c === RCS file: /cvs/src/sys/dev/usb/if_cue.c,v retrieving revision 1.59 diff -u -p -r1.59 if_cue.c --- src/sys/dev/usb/if_cue.c25 Jan 2011 20:03:35 - 1.59 +++ src/sys/dev/usb/if_cue.c21 Mar 2011 19:00:29 - @@ -738,6 +738,11 @@ cue_rxeof(usbd_xfer_handle xfer, usbd_pr usbd_get_xfer_status(xfer, NULL, NULL, &total_len, NULL); + if (total_len < sizeof(struct ether_header) ||total_len > CUE_BUFSZ) { + ifp->if_ierrors++; + goto done; + } + memcpy(mtod(c->cue_mbuf, char *), c->cue_buf, total_len); m = c->cue_mbuf; Index: src/sys/dev/usb/if_kue.c === RCS file: /cvs/src/sys/dev/usb/if_kue.c,v retrieving revision 1.63 diff -u -p -r1.63 if_kue.c --- src/sys/dev/usb/if_kue.c25 Jan 2011 20:03:35 - 1.63 +++ src/sys/dev/usb/if_kue.c21 Mar 2011 19:00:34 - @@ -741,8 +741,10 @@ kue_rxeof(usbd_xfer_handle xfer, usbd_pr __func__, total_len, UGETW(mtod(c->kue_mbuf, u_int8_t *; - if (total_len <= 1) + if (total_len <= 1 || total_len > KUE_BUFSZ) { + ifp->if_ierrors++; goto done; + } m = c->kue_mbuf; /* copy data to mbuf */ Index: src/sys/dev/usb/i
Re: [PATCH] frame length validation for USB ethernet adapters
On 03/19/11 18:01, Loganaden Velvindron wrote: Hi, I'm uploading the new diffs for this. It concerns axe(4), aue(4), cdce(4), cue(4), kue(4), mos(4) and url(4). Of these, axe(4) is the most tricky, and I'd appreciate testing on all vaariants of axe(4). Please test both small and large packets (NFS and SCP). As usual, feedback is most welcomed. Hi, Running the patch here for Linksys USB adapter: x41:fred ~> dmesg|grep url url0 at uhub2 port 2 "Linksys Linksys USB LAN Adapter" rev 1.10/1.00 addr 2 url0: address 00:10:60:e9:42:87 urlphy0 at url0 phy 0: RTL internal phy No regressions so far. Thanks Fred -- PS dmesg follows: OpenBSD 4.9-current (usbeth) #0: Sun Mar 20 11:08:54 GMT 2011 f...@x41.crowsons.com:/usr/src/sys/arch/i386/compile/usbeth cpu0: Intel(R) Pentium(R) M processor 1.60GHz ("GenuineIntel" 686-class) 1.60 GHz cpu0: FPU,V86,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,TM,SBF,EST,TM2 real mem = 1600548864 (1526MB) avail mem = 1564225536 (1491MB) mainbus0 at root bios0 at mainbus0: AT/286+ BIOS, date 03/14/06, BIOS32 rev. 0 @ 0xfd750, SMBIOS rev. 2.33 @ 0xe0010 (59 entries) bios0: vendor IBM version "74ET61WW (2.06 )" date 03/14/2006 bios0: IBM 2525FAG apm0 at bios0: Power Management spec V1.2 apm0: battery life expectancy 100% apm0: AC on, battery charge high acpi at bios0 function 0x0 not configured pcibios0 at bios0: rev 2.1 @ 0xfd6e0/0x920 pcibios0: PCI IRQ Routing Table rev 1.0 @ 0xfdec0/240 (13 entries) pcibios0: PCI Interrupt Router at 000:31:0 ("Intel 82371FB ISA" rev 0x00) pcibios0: PCI bus #5 is the last bus bios0: ROM list: 0xc/0xe800! 0xce800/0x1600 0xd/0x1000 0xdc000/0x4000! 0xe/0x1 cpu0 at mainbus0: (uniprocessor) cpu0: Enhanced SpeedStep 1597 MHz: speeds: 1600, 1500, 1400, 1300, 1200, 1100, 1000, 900, 800, 600 MHz pci0 at mainbus0 bus 0: configuration mode 1 (bios) mem address conflict 0x5f70/0x8 io address conflict 0x5800/0x8 io address conflict 0x5808/0x4 io address conflict 0x5810/0x8 io address conflict 0x580c/0x4 pchb0 at pci0 dev 0 function 0 "Intel 82915GM Host" rev 0x03 vga1 at pci0 dev 2 function 0 "Intel 82915GM Video" rev 0x03 wsdisplay0 at vga1 mux 1: console (80x25, vt100 emulation) wsdisplay0: screen 1-5 added (80x25, vt100 emulation) intagp0 at vga1 agp0 at intagp0: aperture at 0xc000, size 0x1000 inteldrm0 at vga1: no ifp : irq 11 drm0 at inteldrm0 "Intel 82915GM Video" rev 0x03 at pci0 dev 2 function 1 not configured ppb0 at pci0 dev 28 function 0 "Intel 82801FB PCIE" rev 0x03: irq 11 pci1 at ppb0 bus 2 bge0 at pci1 dev 0 function 0 "Broadcom BCM5751M" rev 0x11, BCM5750 B1 (0x4101): irq 11, address 00:16:d3:2f:63:7c brgphy0 at bge0 phy 1: BCM5750 10/100/1000baseT PHY, rev. 0 uhci0 at pci0 dev 29 function 0 "Intel 82801FB USB" rev 0x03: irq 11 uhci1 at pci0 dev 29 function 1 "Intel 82801FB USB" rev 0x03: irq 11 uhci2 at pci0 dev 29 function 2 "Intel 82801FB USB" rev 0x03: irq 11 uhci3 at pci0 dev 29 function 3 "Intel 82801FB USB" rev 0x03: irq 11 ehci0 at pci0 dev 29 function 7 "Intel 82801FB USB" rev 0x03: irq 11 usb0 at ehci0: USB revision 2.0 uhub0 at usb0 "Intel EHCI root hub" rev 2.00/1.00 addr 1 ppb1 at pci0 dev 30 function 0 "Intel 82801BAM Hub-to-PCI" rev 0xd3 pci2 at ppb1 bus 4 cbb0 at pci2 dev 0 function 0 "Ricoh 5C476 CardBus" rev 0x8d: irq 11 sdhc0 at pci2 dev 0 function 1 "Ricoh 5C822 SD/MMC" rev 0x13: irq 11 sdmmc0 at sdhc0 iwi0 at pci2 dev 2 function 0 "Intel PRO/Wireless 2915ABG" rev 0x05: irq 11, address 00:16:6f:c1:16:40 cardslot0 at cbb0 slot 0 flags 0 cardbus0 at cardslot0: bus 5 device 0 cacheline 0x0, lattimer 0xb0 pcmcia0 at cardslot0 auich0 at pci0 dev 30 function 2 "Intel 82801FB AC97" rev 0x03: irq 11, ICH6 AC97 ac97: codec id 0x41445374 (Analog Devices AD1981B) ac97: codec features headphone, 20 bit DAC, No 3D Stereo audio0 at auich0 "Intel 82801FB Modem" rev 0x03 at pci0 dev 30 function 3 not configured ichpcib0 at pci0 dev 31 function 0 "Intel 82801FBM LPC" rev 0x03: PM disabled pciide0 at pci0 dev 31 function 2 "Intel 82801FBM SATA" rev 0x03: DMA, channel 0 wired to compatibility, channel 1 wired to compatibility wd0 at pciide0 channel 0 drive 0: wd0: 16-sector PIO, LBA, 57231MB, 117210240 sectors wd0(pciide0:0:0): using PIO mode 4, Ultra-DMA mode 5 atapiscsi0 at pciide0 channel 1 drive 0 scsibus0 at atapiscsi0: 2 targets cd0 at scsibus0 targ 0 lun 0: ATAPI 5/cdrom removable cd0(pciide0:1:0): using PIO mode 4, Ultra-DMA mode 2 ichiic0 at pci0 dev 31 function 3 "Intel 82801FB SMBus" rev 0x03: irq 11 iic0 at ichiic0 spdmem0 at iic0 addr 0x51: 1GB DDR2 SDRAM non-parity PC2-4200CL3 SO-DIMM usb1 at uhci0: USB revision 1.0 uhub1 at usb1 "Intel UHCI root hub" rev 1.00/1.00 addr 1 usb2 at uhci1: USB revision 1.0 uhub2 at usb2 "Intel UHCI root hub" rev 1.00/1.00 addr 1 usb3 at uhci2: USB revision 1.0 uhub3 at usb3 "Intel UHCI root hub" rev 1.00/1.00 addr 1 usb4 at uhci3: USB revision 1.0 uhub4 at usb4 "Intel UHCI root hub" rev 1.00
Re: [PATCH] frame length validation for USB ethernet adapters
Miod pointed out a wrong variable used for axe(4) diff. Thanks. Index: if_axe.c === RCS file: /cvs/src/sys/dev/usb/if_axe.c,v retrieving revision 1.105 diff -u -p -r1.105 if_axe.c --- if_axe.c25 Jan 2011 20:03:35 - 1.105 +++ if_axe.c20 Mar 2011 04:31:45 - @@ -1018,7 +1018,9 @@ axe_rxeof(usbd_xfer_handle xfer, usbd_pr do { if (sc->axe_flags & AX178 || sc->axe_flags & AX772) { - if (total_len < sizeof(hdr)) { + if (total_len < sizeof(hdr) || + total_len > (sc->axe_bufsz == AXE_178_MAX_BUFSZ) ? + AXE_178_MAX_BUFSZ : AXE_178_MIN_BUFSZ) { ifp->if_ierrors++; goto done; } @@ -1048,8 +1050,15 @@ axe_rxeof(usbd_xfer_handle xfer, usbd_pr else total_len -= pktlen; } else { + if (total_len < sizeof(hdr) || + total_len > AXE_172_BUFSZ) { + ifp->if_ierrors++; + goto done; + } + else { pktlen = total_len; /* crc on the end? */ total_len = 0; + } } m = axe_newbuf(); Index: if_aue.c === RCS file: /cvs/src/sys/dev/usb/if_aue.c,v retrieving revision 1.84 diff -u -p -r1.84 if_aue.c --- if_aue.c25 Jan 2011 20:03:35 - 1.84 +++ if_aue.c20 Mar 2011 04:31:46 - @@ -1078,12 +1078,12 @@ aue_rxeof(usbd_xfer_handle xfer, usbd_pr usbd_get_xfer_status(xfer, NULL, NULL, &total_len, NULL); - memcpy(mtod(c->aue_mbuf, char *), c->aue_buf, total_len); - - if (total_len <= 4 + ETHER_CRC_LEN) { + if (total_len <= 4 + ETHER_CRC_LEN || total_len > AUE_BUFSZ) { ifp->if_ierrors++; goto done; } + + memcpy(mtod(c->aue_mbuf, char *), c->aue_buf, total_len); memcpy(&r, c->aue_buf + total_len - 4, sizeof(r)); Index: if_cdce.c === RCS file: /cvs/src/sys/dev/usb/if_cdce.c,v retrieving revision 1.49 diff -u -p -r1.49 if_cdce.c --- if_cdce.c 25 Jan 2011 20:03:35 - 1.49 +++ if_cdce.c 20 Mar 2011 04:31:46 - @@ -776,8 +776,11 @@ cdce_rxeof(usbd_xfer_handle xfer, usbd_p usbd_get_xfer_status(xfer, NULL, NULL, &total_len, NULL); if (sc->cdce_flags & CDCE_ZAURUS) total_len -= 4; /* Strip off CRC added by Zaurus */ - if (total_len <= 1) + + if (total_len <= 1 || total_len > CDCE_BUFSZ) { + ifp->if_ierrors++; goto done; + } m = c->cdce_mbuf; memcpy(mtod(m, char *), c->cdce_buf, total_len); Index: if_cue.c === RCS file: /cvs/src/sys/dev/usb/if_cue.c,v retrieving revision 1.59 diff -u -p -r1.59 if_cue.c --- if_cue.c25 Jan 2011 20:03:35 - 1.59 +++ if_cue.c20 Mar 2011 04:31:49 - @@ -738,6 +738,11 @@ cue_rxeof(usbd_xfer_handle xfer, usbd_pr usbd_get_xfer_status(xfer, NULL, NULL, &total_len, NULL); + if (total_len < sizeof(struct ether_header) ||total_len > CUE_BUFSZ) { + ifp->if_ierrors++; + goto done; + } + memcpy(mtod(c->cue_mbuf, char *), c->cue_buf, total_len); m = c->cue_mbuf; Index: if_kue.c === RCS file: /cvs/src/sys/dev/usb/if_kue.c,v retrieving revision 1.63 diff -u -p -r1.63 if_kue.c --- if_kue.c25 Jan 2011 20:03:35 - 1.63 +++ if_kue.c20 Mar 2011 04:31:50 - @@ -741,8 +741,10 @@ kue_rxeof(usbd_xfer_handle xfer, usbd_pr __func__, total_len, UGETW(mtod(c->kue_mbuf, u_int8_t *; - if (total_len <= 1) + if (total_len <= 1 || total_len > KUE_BUFSZ) { + ifp->if_ierrors++; goto done; + } m = c->kue_mbuf; /* copy data to mbuf */ Index: if_mos.c === RCS file: /cvs/src/sys/dev/usb/if_mos.c,v retrieving revision 1.14 diff -u -p -r1.14 if_mos.c --- if_mos.c21 Feb 2011 19:48:41 - 1.14 +++ if_mos.c20 Mar 2011 04:31:52 - @@ -955,8 +955,10 @@ mos_rxeof(usbd_xfer_handle xfer, usbd_pr usbd_get_xfer_status(xfer, NULL, NULL, &total_len, NULL); - if (total_len <= 1) + if (total_len <= 1 || total_len > MOS_BUFSZ) { + ifp->if_ierrors++; goto done; + } /* evaluate status byte at the end */ pktlen = total_len - 1; Index: if_url.c =
Re: [PATCH] frame length validation for USB ethernet adapters
> Index: src/sys/dev/usb/if_axe.c > === > RCS file: /cvs/src/sys/dev/usb/if_axe.c,v > retrieving revision 1.105 > diff -u -p -r1.105 if_axe.c > --- src/sys/dev/usb/if_axe.c 25 Jan 2011 20:03:35 - 1.105 > +++ src/sys/dev/usb/if_axe.c 19 Mar 2011 17:45:08 - > @@ -1018,7 +1018,9 @@ axe_rxeof(usbd_xfer_handle xfer, usbd_pr > > do { > if (sc->axe_flags & AX178 || sc->axe_flags & AX772) { > - if (total_len < sizeof(hdr)) { > + if (total_len < sizeof(hdr) || > + total_len > (sc->axe_flags == AXE_178_MAX_BUFSZ ? ^^ This can't possibly be correct.
Re: [PATCH] frame length validation for USB ethernet adapters
Hi, I'm uploading the new diffs for this. It concerns axe(4), aue(4), cdce(4), cue(4), kue(4), mos(4) and url(4). Of these, axe(4) is the most tricky, and I'd appreciate testing on all vaariants of axe(4). Please test both small and large packets (NFS and SCP). As usual, feedback is most welcomed. Index: src/sys/dev/usb/if_axe.c === RCS file: /cvs/src/sys/dev/usb/if_axe.c,v retrieving revision 1.105 diff -u -p -r1.105 if_axe.c --- src/sys/dev/usb/if_axe.c25 Jan 2011 20:03:35 - 1.105 +++ src/sys/dev/usb/if_axe.c19 Mar 2011 17:45:08 - @@ -1018,7 +1018,9 @@ axe_rxeof(usbd_xfer_handle xfer, usbd_pr do { if (sc->axe_flags & AX178 || sc->axe_flags & AX772) { - if (total_len < sizeof(hdr)) { + if (total_len < sizeof(hdr) || + total_len > (sc->axe_flags == AXE_178_MAX_BUFSZ ? + AXE_178_MAX_BUFSZ : AXE_178_MIN_BUFSZ)) { ifp->if_ierrors++; goto done; } @@ -1048,8 +1050,15 @@ axe_rxeof(usbd_xfer_handle xfer, usbd_pr else total_len -= pktlen; } else { + if (total_len < sizeof(hdr) || + total_len > AXE_172_BUFSZ) { + ifp->if_ierrors++; + goto done; + } + else { pktlen = total_len; /* crc on the end? */ total_len = 0; + } } m = axe_newbuf(); Index: src/sys/dev/usb/if_aue.c === RCS file: /cvs/src/sys/dev/usb/if_aue.c,v retrieving revision 1.84 diff -u -p -r1.84 if_aue.c --- src/sys/dev/usb/if_aue.c25 Jan 2011 20:03:35 - 1.84 +++ src/sys/dev/usb/if_aue.c19 Mar 2011 17:45:09 - @@ -1078,12 +1078,12 @@ aue_rxeof(usbd_xfer_handle xfer, usbd_pr usbd_get_xfer_status(xfer, NULL, NULL, &total_len, NULL); - memcpy(mtod(c->aue_mbuf, char *), c->aue_buf, total_len); - - if (total_len <= 4 + ETHER_CRC_LEN) { + if (total_len <= 4 + ETHER_CRC_LEN || total_len > AUE_BUFSZ) { ifp->if_ierrors++; goto done; } + + memcpy(mtod(c->aue_mbuf, char *), c->aue_buf, total_len); memcpy(&r, c->aue_buf + total_len - 4, sizeof(r)); Index: src/sys/dev/usb/if_cdce.c === RCS file: /cvs/src/sys/dev/usb/if_cdce.c,v retrieving revision 1.49 diff -u -p -r1.49 if_cdce.c --- src/sys/dev/usb/if_cdce.c 25 Jan 2011 20:03:35 - 1.49 +++ src/sys/dev/usb/if_cdce.c 19 Mar 2011 17:45:10 - @@ -776,8 +776,11 @@ cdce_rxeof(usbd_xfer_handle xfer, usbd_p usbd_get_xfer_status(xfer, NULL, NULL, &total_len, NULL); if (sc->cdce_flags & CDCE_ZAURUS) total_len -= 4; /* Strip off CRC added by Zaurus */ - if (total_len <= 1) + + if (total_len <= 1 || total_len > CDCE_BUFSZ) { + ifp->if_ierrors++; goto done; + } m = c->cdce_mbuf; memcpy(mtod(m, char *), c->cdce_buf, total_len); Index: src/sys/dev/usb/if_cue.c === RCS file: /cvs/src/sys/dev/usb/if_cue.c,v retrieving revision 1.59 diff -u -p -r1.59 if_cue.c --- src/sys/dev/usb/if_cue.c25 Jan 2011 20:03:35 - 1.59 +++ src/sys/dev/usb/if_cue.c19 Mar 2011 17:45:11 - @@ -738,6 +738,11 @@ cue_rxeof(usbd_xfer_handle xfer, usbd_pr usbd_get_xfer_status(xfer, NULL, NULL, &total_len, NULL); + if (total_len < sizeof(struct ether_header) ||total_len > CUE_BUFSZ) { + ifp->if_ierrors++; + goto done; + } + memcpy(mtod(c->cue_mbuf, char *), c->cue_buf, total_len); m = c->cue_mbuf; Index: src/sys/dev/usb/if_kue.c === RCS file: /cvs/src/sys/dev/usb/if_kue.c,v retrieving revision 1.63 diff -u -p -r1.63 if_kue.c --- src/sys/dev/usb/if_kue.c25 Jan 2011 20:03:35 - 1.63 +++ src/sys/dev/usb/if_kue.c19 Mar 2011 17:45:11 - @@ -741,8 +741,10 @@ kue_rxeof(usbd_xfer_handle xfer, usbd_pr __func__, total_len, UGETW(mtod(c->kue_mbuf, u_int8_t *; - if (total_len <= 1) + if (total_len <= 1 || total_len > KUE_BUFSZ) { + ifp->if_ierrors++; goto done; + } m = c->kue_mbuf; /* copy data to mbuf */ Index: src/sys/dev/usb/if_mos.c === RCS file: /cvs/src/sys/dev/usb/if_mos.c,v retrieving revision 1.14 diff -u -p -r
Re: [PATCH] frame length validation for USB ethernet adapters
The broken udav(4) diff is my fault. I didn't test with NFS and large file transfers when jsg@ sent me his diff (privately). He trusted my feedback and that's how it was committed. Can we move on now ? //Logan C-x-C-c
Re: [PATCH] frame length validation for USB ethernet adapters
Hi Kiki, Can you send us a trace & dmesg ? //Logan C-x-C-c
Re: [PATCH] frame length validation for USB ethernet adapters
Hi, On Wed, Mar 16, 2011 at 04:58:19PM -0400, Loganaden Velvindron wrote: > After my experience with udav(4), I took a look at other USB > adapters. The diff makes all of them more consistent by checking > the frame length returned and validating it. > > Since I don't own any of those adapters, I must rely on you people to > test them. The diff is wrong unless successful reports are heard from users. > > Of course, if the adapters are broken after applying the diff, I'll > do my best to track down the problem. FYI: I just noticed (about 10 minutes ago, after bringing my zaurus up to date) that the recent if_udav patch breaks nfs client functionality. No time to debug it yet, though :-( Ciao, Kili
[PATCH] frame length validation for USB ethernet adapters
After my experience with udav(4), I took a look at other USB adapters. The diff makes all of them more consistent by checking the frame length returned and validating it. Since I don't own any of those adapters, I must rely on you people to test them. The diff is wrong unless successful reports are heard from users. Of course, if the adapters are broken after applying the diff, I'll do my best to track down the problem. Yes, even 2-line diffs for each adapter needs to be checked, since device drivers are a real minefield, so please test ! Index: if_axe.c === RCS file: /cvs/src/sys/dev/usb/if_axe.c,v retrieving revision 1.105 diff -u -p -r1.105 if_axe.c --- if_axe.c25 Jan 2011 20:03:35 - 1.105 +++ if_axe.c16 Mar 2011 20:34:42 - @@ -1018,7 +1018,8 @@ axe_rxeof(usbd_xfer_handle xfer, usbd_pr do { if (sc->axe_flags & AX178 || sc->axe_flags & AX772) { - if (total_len < sizeof(hdr)) { + if (total_len < ETHERMIN || + total_len > ifp->if_hardmtu) { ifp->if_ierrors++; goto done; } Index: if_aue.c === RCS file: /cvs/src/sys/dev/usb/if_aue.c,v retrieving revision 1.84 diff -u -p -r1.84 if_aue.c --- if_aue.c25 Jan 2011 20:03:35 - 1.84 +++ if_aue.c16 Mar 2011 20:34:43 - @@ -1078,12 +1078,12 @@ aue_rxeof(usbd_xfer_handle xfer, usbd_pr usbd_get_xfer_status(xfer, NULL, NULL, &total_len, NULL); - memcpy(mtod(c->aue_mbuf, char *), c->aue_buf, total_len); - - if (total_len <= 4 + ETHER_CRC_LEN) { + if (total_len < ETHERMIN || total_len > ifp->if_hardmtu) { ifp->if_ierrors++; goto done; } + + memcpy(mtod(c->aue_mbuf, char *), c->aue_buf, total_len); memcpy(&r, c->aue_buf + total_len - 4, sizeof(r)); Index: if_cdce.c === RCS file: /cvs/src/sys/dev/usb/if_cdce.c,v retrieving revision 1.49 diff -u -p -r1.49 if_cdce.c --- if_cdce.c 25 Jan 2011 20:03:35 - 1.49 +++ if_cdce.c 16 Mar 2011 20:34:45 - @@ -776,16 +776,13 @@ cdce_rxeof(usbd_xfer_handle xfer, usbd_p usbd_get_xfer_status(xfer, NULL, NULL, &total_len, NULL); if (sc->cdce_flags & CDCE_ZAURUS) total_len -= 4; /* Strip off CRC added by Zaurus */ - if (total_len <= 1) + if (total_len < ETHERMIN || total_len > ifp->if_hardmtu) { + ifp->if_ierrors++; goto done; + } m = c->cdce_mbuf; memcpy(mtod(m, char *), c->cdce_buf, total_len); - - if (total_len < sizeof(struct ether_header)) { - ifp->if_ierrors++; - goto done; - } ifp->if_ipackets++; Index: if_cue.c === RCS file: /cvs/src/sys/dev/usb/if_cue.c,v retrieving revision 1.59 diff -u -p -r1.59 if_cue.c --- if_cue.c25 Jan 2011 20:03:35 - 1.59 +++ if_cue.c16 Mar 2011 20:34:46 - @@ -738,6 +738,11 @@ cue_rxeof(usbd_xfer_handle xfer, usbd_pr usbd_get_xfer_status(xfer, NULL, NULL, &total_len, NULL); + if (total_len < ETHERMIN || total_len > ifp->if_hardmtu) { + ifp->if_ierrors++; + goto done; + } + memcpy(mtod(c->cue_mbuf, char *), c->cue_buf, total_len); m = c->cue_mbuf; Index: if_kue.c === RCS file: /cvs/src/sys/dev/usb/if_kue.c,v retrieving revision 1.63 diff -u -p -r1.63 if_kue.c --- if_kue.c25 Jan 2011 20:03:35 - 1.63 +++ if_kue.c16 Mar 2011 20:34:47 - @@ -741,8 +741,10 @@ kue_rxeof(usbd_xfer_handle xfer, usbd_pr __func__, total_len, UGETW(mtod(c->kue_mbuf, u_int8_t *; - if (total_len <= 1) + if (total_len < ETHERMIN || total_len > ifp->if_hardmtu) { + ifp->if_ierrors++; goto done; + } m = c->kue_mbuf; /* copy data to mbuf */ Index: if_mos.c === RCS file: /cvs/src/sys/dev/usb/if_mos.c,v retrieving revision 1.13 diff -u -p -r1.13 if_mos.c --- if_mos.c25 Jan 2011 20:03:35 - 1.13 +++ if_mos.c16 Mar 2011 20:34:48 - @@ -955,8 +955,10 @@ mos_rxeof(usbd_xfer_handle xfer, usbd_pr usbd_get_xfer_status(xfer, NULL, NULL, &total_len, NULL); - if (total_len <= 1) + if (total_len < ETHERMIN || total_len > ifp->if_hardmtu) { + ifp->if_ierrors++; goto done; + } /* evaluate status byte at the end */ pktlen = total_len - 1; Index: if_url.c =