Re: Division by zero in ehci(4)
On Mon, Sep 19, 2016 at 11:47:40AM +0200, Martin Pieuchot wrote:
> On 19/09/16(Mon) 14:20, Jonathan Gray wrote:
> > On Sat, Sep 17, 2016 at 04:36:12PM +0200, Martin Pieuchot wrote:
> > > One of the non-checked value read from an untrusted descriptor is the
> > > "maximum packet size" of an endpoint. If a device reports an incorrect
> > > value most of our HC drivers wont work and if this value is 0 ehci(4)
> > > will crash the kernel.
> > >
> > > So here's a diff to validate the value read from the device descriptor
> > > which ends up being the value of the default endpoint.
> > >
> > > ok?
> >
> > This patch made vmware hang when attaching xhci uhubs.
> >
> > usbd_new_device bus=0x801e4000 port=0 depth=0 speed=4
> > usbd_new_device: adding unit addr=1, rev=300, class=9, subclass=0,
> > protocol=1, maxpacket=9, len=18, speed=4
> > usb2: root hub problem
> >
> > usbd_new_device mps 9 mps0 512
> >
> > It would appear that for superspeed devices maxpacketsize0 is a power
> > of 2? ie 2^9 is 512.
>
> You're correct, here's a fix.
Thanks, this patch works with the same setup. ok jsg@
I think the
- (mps != 8 || mps != 16 || mps != 32 || mps != 64)) {
+ (mps != 8 && mps != 16 && mps != 32 && mps != 64))
part of the one I sent sound go in as well?
>
> Index: usb_subr.c
> ===
> RCS file: /cvs/src/sys/dev/usb/usb_subr.c,v
> retrieving revision 1.129
> diff -u -p -r1.129 usb_subr.c
> --- usb_subr.c18 Sep 2016 09:51:24 - 1.129
> +++ usb_subr.c19 Sep 2016 09:42:58 -
> @@ -1175,8 +1175,12 @@ usbd_new_device(struct device *parent, s
> }
>
> mps = dd->bMaxPacketSize;
> - if (speed == USB_SPEED_SUPER && mps == 0xff)
> - mps = 512;
> + if (speed == USB_SPEED_SUPER) {
> + if (mps == 0xff)
> + mps = 9;
> + /* xHCI Section 4.8.2.1 */
> + mps = (1 << mps);
> + }
>
> if (mps != mps0) {
> if ((speed == USB_SPEED_LOW) ||
>
Re: Division by zero in ehci(4)
On 19/09/16(Mon) 14:20, Jonathan Gray wrote:
> On Sat, Sep 17, 2016 at 04:36:12PM +0200, Martin Pieuchot wrote:
> > One of the non-checked value read from an untrusted descriptor is the
> > "maximum packet size" of an endpoint. If a device reports an incorrect
> > value most of our HC drivers wont work and if this value is 0 ehci(4)
> > will crash the kernel.
> >
> > So here's a diff to validate the value read from the device descriptor
> > which ends up being the value of the default endpoint.
> >
> > ok?
>
> This patch made vmware hang when attaching xhci uhubs.
>
> usbd_new_device bus=0x801e4000 port=0 depth=0 speed=4
> usbd_new_device: adding unit addr=1, rev=300, class=9, subclass=0,
> protocol=1, maxpacket=9, len=18, speed=4
> usb2: root hub problem
>
> usbd_new_device mps 9 mps0 512
>
> It would appear that for superspeed devices maxpacketsize0 is a power
> of 2? ie 2^9 is 512.
You're correct, here's a fix.
Index: usb_subr.c
===
RCS file: /cvs/src/sys/dev/usb/usb_subr.c,v
retrieving revision 1.129
diff -u -p -r1.129 usb_subr.c
--- usb_subr.c 18 Sep 2016 09:51:24 - 1.129
+++ usb_subr.c 19 Sep 2016 09:42:58 -
@@ -1175,8 +1175,12 @@ usbd_new_device(struct device *parent, s
}
mps = dd->bMaxPacketSize;
- if (speed == USB_SPEED_SUPER && mps == 0xff)
- mps = 512;
+ if (speed == USB_SPEED_SUPER) {
+ if (mps == 0xff)
+ mps = 9;
+ /* xHCI Section 4.8.2.1 */
+ mps = (1 << mps);
+ }
if (mps != mps0) {
if ((speed == USB_SPEED_LOW) ||
Re: Division by zero in ehci(4)
On Sat, Sep 17, 2016 at 04:36:12PM +0200, Martin Pieuchot wrote: > One of the non-checked value read from an untrusted descriptor is the > "maximum packet size" of an endpoint. If a device reports an incorrect > value most of our HC drivers wont work and if this value is 0 ehci(4) > will crash the kernel. > > So here's a diff to validate the value read from the device descriptor > which ends up being the value of the default endpoint. > > ok? This patch made vmware hang when attaching xhci uhubs. usbd_new_device bus=0x801e4000 port=0 depth=0 speed=4 usbd_new_device: adding unit addr=1, rev=300, class=9, subclass=0, protocol=1, maxpacket=9, len=18, speed=4 usb2: root hub problem usbd_new_device mps 9 mps0 512 It would appear that for superspeed devices maxpacketsize0 is a power of 2? ie 2^9 is 512. Bus 000 Device 001: ID 15ad: VMware Inc. Device Descriptor: bLength18 bDescriptorType 1 bcdUSB 2.00 bDeviceClass9 Hub bDeviceSubClass 0 Unused bDeviceProtocol 1 Single TT bMaxPacketSize064 idVendor 0x15ad VMware Inc. idProduct 0x bcdDevice1.00 iManufacturer 1 VMware iProduct2 EHCI root hub iSerial 0 bNumConfigurations 1 Configuration Descriptor: bLength 9 bDescriptorType 2 wTotalLength 25 bNumInterfaces 1 bConfigurationValue 1 iConfiguration 0 bmAttributes 0x40 (Missing must-be-set bit!) Self Powered MaxPower0mA Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber0 bAlternateSetting 0 bNumEndpoints 1 bInterfaceClass 9 Hub bInterfaceSubClass 0 Unused bInterfaceProtocol 0 Full speed (or root) hub iInterface 0 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x81 EP 1 IN bmAttributes3 Transfer TypeInterrupt Synch Type None Usage Type Data wMaxPacketSize 0x0008 1x 8 bytes bInterval 12 Hub Descriptor: bLength 10 bDescriptorType 41 nNbrPorts 6 wHubCharacteristic 0x0002 No power switching (usb 1.0) Ganged overcurrent protection TT think time 8 FS bits bPwrOn2PwrGood 200 * 2 milli seconds bHubContrCurrent 0 milli Ampere DeviceRemovable0x00 PortPwrCtrlMask0x00 Hub Port Status: Port 1: .0500 highspeed power Port 2: .0500 highspeed power Port 3: .0500 highspeed power Port 4: .0500 highspeed power Port 5: .0500 highspeed power Port 6: .0500 highspeed power Device Status: 0x0001 Self Powered Bus 001 Device 001: ID 15ad: VMware Inc. Device Descriptor: bLength18 bDescriptorType 1 bcdUSB 1.00 bDeviceClass9 Hub bDeviceSubClass 0 Unused bDeviceProtocol 0 Full speed (or root) hub bMaxPacketSize064 idVendor 0x15ad VMware Inc. idProduct 0x bcdDevice1.00 iManufacturer 1 VMware iProduct2 UHCI root hub iSerial 0 bNumConfigurations 1 Configuration Descriptor: bLength 9 bDescriptorType 2 wTotalLength 25 bNumInterfaces 1 bConfigurationValue 1 iConfiguration 0 bmAttributes 0x40 (Missing must-be-set bit!) Self Powered MaxPower0mA Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber0 bAlternateSetting 0 bNumEndpoints 1 bInterfaceClass 9 Hub bInterfaceSubClass 0 Unused bInterfaceProtocol 0 Full speed (or root) hub iInterface 0 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x81 EP 1 IN bmAttributes3 Transfer TypeInterrupt Synch Type None Usage Type Data wMaxPacketSize 0x0008 1x 8 bytes bInterval 255 Device Status: 0x0001 Self Powered Bus 001 Device 002: ID 0e0f:0002 VMware, Inc. Virtual USB Hub Device Descriptor: bLength18 bDescriptorType 1 bcdUSB 1.10 bDeviceClass9 Hub bDeviceSubClass 0 Unused bDeviceProtocol 0 Full speed (or root) hub bMaxPacketSize0 8 idVendor 0x0e0f VMware, Inc.
