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?

Index: usb_subr.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/usb_subr.c,v
retrieving revision 1.128
diff -u -p -r1.128 usb_subr.c
--- usb_subr.c  12 Sep 2016 07:43:10 -0000      1.128
+++ usb_subr.c  17 Sep 2016 13:38:02 -0000
@@ -1023,16 +1027,34 @@ usbd_status
 usbd_new_device(struct device *parent, struct usbd_bus *bus, int depth,
                int speed, int port, struct usbd_port *up)
 {
-       struct usbd_device *dev, *adev;
-       struct usbd_device *hub;
+       struct usbd_device *dev, *adev, *hub;
        usb_device_descriptor_t *dd;
        usbd_status err;
-       int addr;
-       int i;
-       int p;
+       uint32_t mps, mps0;
+       int addr, i, p;
 
        DPRINTF(("usbd_new_device bus=%p port=%d depth=%d speed=%d\n",
                 bus, port, depth, speed));
+
+       /*
+        * Fixed size for ep0 max packet, FULL device variable size is
+        * handled below.
+        */
+       switch (speed) {
+       case USB_SPEED_LOW:
+               mps0 = 8;
+               break;
+       case USB_SPEED_HIGH:
+       case USB_SPEED_FULL:
+               mps0 = 64;
+               break;
+       case USB_SPEED_SUPER:
+               mps0 = 512;
+               break;
+       default:
+               return (USBD_INVAL);
+       }
+
        addr = usbd_getnewaddr(bus);
        if (addr < 0) {
                printf("%s: No free USB addresses, new device ignored.\n",
@@ -1054,8 +1076,8 @@ usbd_new_device(struct device *parent, s
        dev->def_ep_desc.bDescriptorType = UDESC_ENDPOINT;
        dev->def_ep_desc.bEndpointAddress = USB_CONTROL_ENDPOINT;
        dev->def_ep_desc.bmAttributes = UE_CONTROL;
-       USETW(dev->def_ep_desc.wMaxPacketSize, USB_MAX_IPACKET);
        dev->def_ep_desc.bInterval = 0;
+       USETW(dev->def_ep_desc.wMaxPacketSize, mps0);
 
        dev->quirks = &usbd_no_quirk;
        dev->address = USB_START_ADDR;
@@ -1063,6 +1085,8 @@ usbd_new_device(struct device *parent, s
        dev->depth = depth;
        dev->powersrc = up;
        dev->myhub = up->parent;
+       dev->speed = speed;
+       dev->langid = USBD_NOLANG;
 
        up->device = dev;
 
@@ -1084,8 +1108,6 @@ usbd_new_device(struct device *parent, s
        } else {
                dev->myhsport = NULL;
        }
-       dev->speed = speed;
-       dev->langid = USBD_NOLANG;
 
        /* Establish the default pipe. */
        err = usbd_setup_pipe(dev, 0, &dev->def_ep, USBD_DEFAULT_INTERVAL,
@@ -1143,36 +1165,33 @@ usbd_new_device(struct device *parent, s
                return (err);
        }
 
-       if (speed == USB_SPEED_HIGH) {
-               /* Max packet size must be 64 (sec 5.5.3). */
-               if (dd->bMaxPacketSize != USB_2_MAX_CTRL_PACKET) {
-#ifdef DIAGNOSTIC
-                       printf("%s: addr=%d bad max packet size %d\n", __func__,
-                           addr, dd->bMaxPacketSize);
-#endif
-                       dd->bMaxPacketSize = USB_2_MAX_CTRL_PACKET;
-               }
-       }
-
        DPRINTF(("usbd_new_device: adding unit addr=%d, rev=%02x, class=%d, "
                 "subclass=%d, protocol=%d, maxpacket=%d, len=%d, speed=%d\n",
                 addr,UGETW(dd->bcdUSB), dd->bDeviceClass, dd->bDeviceSubClass,
                 dd->bDeviceProtocol, dd->bMaxPacketSize, dd->bLength,
                 dev->speed));
 
-       if (dd->bDescriptorType != UDESC_DEVICE) {
+       if ((dd->bDescriptorType != UDESC_DEVICE) ||
+           (dd->bLength < USB_DEVICE_DESCRIPTOR_SIZE)) {
                usb_free_device(dev);
                up->device = NULL;
                return (USBD_INVAL);
        }
 
-       if (dd->bLength < USB_DEVICE_DESCRIPTOR_SIZE) {
-               usb_free_device(dev);
-               up->device = NULL;
-               return (USBD_INVAL);
+       mps = dd->bMaxPacketSize;
+       if (speed == USB_SPEED_SUPER && mps == 0xff)
+               mps = 512;
+
+       if (mps != mps0) {
+               if ((speed == USB_SPEED_LOW) ||
+                   (mps != 8 || mps != 16 || mps != 32 || mps != 64)) {
+                       usb_free_device(dev);
+                       up->device = NULL;
+                       return (USBD_INVAL);
+               }
+               USETW(dev->def_ep_desc.wMaxPacketSize, mps);
        }
 
-       USETW(dev->def_ep_desc.wMaxPacketSize, dd->bMaxPacketSize);
 
        /* Set the address if the HC didn't do it already. */
        if (bus->methods->dev_setaddr != NULL &&

Reply via email to