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
=
