Our USB stack is very lazy at validating descriptor. In order to reject device with malformed endpoint descriptors I'd like to introduce a new function, usbd_parse_idesc(), that I will extend with more checks.
For now this function only contain the checks present in usbd_fill_iface_data() but it will be soon extended. ok? Index: usb_subr.c =================================================================== RCS file: /cvs/src/sys/dev/usb/usb_subr.c,v retrieving revision 1.127 diff -u -p -r1.127 usb_subr.c --- usb_subr.c 2 Sep 2016 11:14:17 -0000 1.127 +++ usb_subr.c 9 Sep 2016 15:24:07 -0000 @@ -73,6 +73,7 @@ usbd_status usbd_probe_and_attach(struct int usbd_printBCD(char *cp, size_t len, int bcd); void usb_free_device(struct usbd_device *); +int usbd_parse_idesc(struct usbd_device *, struct usbd_interface *); #ifdef USBVERBOSE #include <dev/usb/usbdevs_data.h> @@ -495,66 +496,34 @@ usbd_find_edesc(usb_config_descriptor_t return (NULL); } -usbd_status -usbd_fill_iface_data(struct usbd_device *dev, int ifaceidx, int altidx) +int +usbd_parse_idesc(struct usbd_device *dev, struct usbd_interface *ifc) { - struct usbd_interface *ifc = &dev->ifaces[ifaceidx]; - usb_interface_descriptor_t *idesc; +#define ed ((usb_endpoint_descriptor_t *)p) char *p, *end; - int endpt, nendpt; - - DPRINTFN(4,("%s: ifaceidx=%d altidx=%d\n", __func__, ifaceidx, altidx)); - - idesc = usbd_find_idesc(dev->cdesc, ifaceidx, altidx); - if (idesc == NULL) - return (USBD_INVAL); - - nendpt = idesc->bNumEndpoints; - DPRINTFN(4,("%s: found idesc nendpt=%d\n", __func__, nendpt)); - - ifc->device = dev; - ifc->idesc = idesc; - ifc->index = ifaceidx; - ifc->altindex = altidx; - ifc->endpoints = NULL; - ifc->priv = NULL; - LIST_INIT(&ifc->pipes); - - if (nendpt != 0) { - ifc->endpoints = mallocarray(nendpt, - sizeof(struct usbd_endpoint), M_USB, M_NOWAIT | M_ZERO); - if (ifc->endpoints == NULL) - return (USBD_NOMEM); - } + int i; p = (char *)ifc->idesc + ifc->idesc->bLength; end = (char *)dev->cdesc + UGETW(dev->cdesc->wTotalLength); -#define ed ((usb_endpoint_descriptor_t *)p) - for (endpt = 0; endpt < nendpt; endpt++) { - DPRINTFN(10,("%s: endpt=%d\n", __func__, endpt)); + + for (i = 0; i < ifc->idesc->bNumEndpoints; i++) { for (; p < end; p += ed->bLength) { - DPRINTFN(10,("%s: p=%p end=%p len=%d type=%d\n", - __func__, p, end, ed->bLength, - ed->bDescriptorType)); if (p + ed->bLength <= end && ed->bLength != 0 && ed->bDescriptorType == UDESC_ENDPOINT) - goto found; + break; + if (ed->bLength == 0 || ed->bDescriptorType == UDESC_INTERFACE) - break; + return (-1); } - /* passed end, or bad desc */ - printf("%s: bad descriptor(s): %s\n", __func__, - ed->bLength == 0 ? "0 length" : - ed->bDescriptorType == UDESC_INTERFACE ? "iface desc" : - "out of data"); - goto bad; - found: - ifc->endpoints[endpt].edesc = ed; + + if (p >= end) + return (-1); + if (dev->speed == USB_SPEED_HIGH) { - u_int mps; - /* Control and bulk endpoints have max packet - limits. */ + unsigned int mps; + + /* Control and bulk endpoints have max packet limits. */ switch (UE_GET_XFERTYPE(ed->bmAttributes)) { case UE_CONTROL: mps = USB_2_MAX_CTRL_PACKET; @@ -572,19 +541,55 @@ usbd_fill_iface_data(struct usbd_device break; } } - ifc->endpoints[endpt].refcnt = 0; - ifc->endpoints[endpt].savedtoggle = 0; + + ifc->endpoints[i].edesc = ed; + ifc->endpoints[i].refcnt = 0; + ifc->endpoints[i].savedtoggle = 0; p += ed->bLength; } + + return (0); #undef ed - return (USBD_NORMAL_COMPLETION); +} + +usbd_status +usbd_fill_iface_data(struct usbd_device *dev, int ifaceidx, int altidx) +{ + struct usbd_interface *ifc = &dev->ifaces[ifaceidx]; + usb_interface_descriptor_t *idesc; + int nendpt; + + DPRINTFN(4,("%s: ifaceidx=%d altidx=%d\n", __func__, ifaceidx, altidx)); + + idesc = usbd_find_idesc(dev->cdesc, ifaceidx, altidx); + if (idesc == NULL) + return (USBD_INVAL); + + nendpt = idesc->bNumEndpoints; + DPRINTFN(4,("%s: found idesc nendpt=%d\n", __func__, nendpt)); + + ifc->device = dev; + ifc->idesc = idesc; + ifc->index = ifaceidx; + ifc->altindex = altidx; + ifc->endpoints = NULL; + ifc->priv = NULL; + LIST_INIT(&ifc->pipes); - bad: - if (ifc->endpoints != NULL) { - free(ifc->endpoints, M_USB, 0); + if (nendpt != 0) { + ifc->endpoints = mallocarray(nendpt, sizeof(*ifc->endpoints), + M_USB, M_NOWAIT | M_ZERO); + if (ifc->endpoints == NULL) + return (USBD_NOMEM); + } + + if (usbd_parse_idesc(dev, ifc)) { + free(ifc->endpoints, M_USB, nendpt * sizeof(*ifc->endpoints)); ifc->endpoints = NULL; + return (USBD_INVAL); } - return (USBD_INVAL); + + return (USBD_NORMAL_COMPLETION); } void