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 &&
