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
