While analysing okan@'s NULL-dereference in ugen_do_clos() I could not
convince myself that usbd_set_interface() was not modifying `iface' if
usbd_fill_iface_data() failed.
So I de-obfuscate usbd_fill_iface_data() as below. As a bonus this add
various sizes to free(9) and change a usbd_status -> int because there's
no I/O going on here!
Works for me, more tests with ugen(4) and reviews welcome.
Index: usb_subr.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/usb_subr.c,v
retrieving revision 1.117
diff -u -p -r1.117 usb_subr.c
--- usb_subr.c 23 Mar 2015 22:26:01 -0000 1.117
+++ usb_subr.c 14 Oct 2015 09:44:34 -0000
@@ -416,10 +416,6 @@ usbd_find_idesc(usb_config_descriptor_t
for (curidx = lastidx = -1; p < end; ) {
d = (usb_interface_descriptor_t *)p;
- DPRINTFN(4,("usbd_find_idesc: idx=%d(%d) altidx=%d(%d) len=%d "
- "type=%d\n",
- ifaceidx, curidx, altidx, curaidx,
- d->bLength, d->bDescriptorType));
if (d->bLength == 0) /* bad descriptor */
break;
p += d->bLength;
@@ -470,102 +466,102 @@ usbd_find_edesc(usb_config_descriptor_t
return (NULL);
}
-usbd_status
+static inline usb_endpoint_descriptor_t *
+usbd_fix_ed(struct usbd_device *dev, usb_endpoint_descriptor_t *ed)
+{
+ uint32_t mps;
+
+ if (dev->speed == USB_SPEED_HIGH) {
+ /* Control and bulk endpoints have max packet limits. */
+ switch (UE_GET_XFERTYPE(ed->bmAttributes)) {
+ case UE_CONTROL:
+ mps = USB_2_MAX_CTRL_PACKET;
+ goto check;
+ case UE_BULK:
+ mps = USB_2_MAX_BULK_PACKET;
+ check:
+ if (UGETW(ed->wMaxPacketSize) != mps) {
+ USETW(ed->wMaxPacketSize, mps);
+#ifdef DIAGNOSTIC
+ printf("%s: bad max packet size\n", __func__);
+#endif
+ }
+ break;
+ default:
+ break;
+ }
+ }
+
+ return (ed);
+}
+
+int
usbd_fill_iface_data(struct usbd_device *dev, int ifaceidx, int altidx)
{
- struct usbd_interface *ifc = &dev->ifaces[ifaceidx];
- usb_interface_descriptor_t *idesc;
+ struct usbd_interface *iface = &dev->ifaces[ifaceidx];
+ usb_interface_descriptor_t *id;
+ usb_endpoint_descriptor_t *ed;
+ struct usbd_endpoint *endpoints = NULL;
char *p, *end;
- int endpt, nendpt;
+ int endpt;
+
+ id = usbd_find_idesc(dev->cdesc, ifaceidx, altidx);
+ if (id == NULL)
+ return (-1);
- DPRINTFN(4,("usbd_fill_iface_data: ifaceidx=%d altidx=%d\n",
- ifaceidx, altidx));
- idesc = usbd_find_idesc(dev->cdesc, ifaceidx, altidx);
- if (idesc == NULL)
- return (USBD_INVAL);
- ifc->device = dev;
- ifc->idesc = idesc;
- ifc->index = ifaceidx;
- ifc->altindex = altidx;
- nendpt = ifc->idesc->bNumEndpoints;
- DPRINTFN(4,("usbd_fill_iface_data: found idesc nendpt=%d\n", nendpt));
- if (nendpt != 0) {
- ifc->endpoints = mallocarray(nendpt,
- sizeof(struct usbd_endpoint), M_USB, M_NOWAIT);
- if (ifc->endpoints == NULL)
- return (USBD_NOMEM);
- } else
- ifc->endpoints = NULL;
- ifc->priv = NULL;
- p = (char *)ifc->idesc + ifc->idesc->bLength;
+ if (id->bNumEndpoints != 0)
+ endpoints = mallocarray(id->bNumEndpoints, sizeof(*endpoints),
+ M_USB, M_WAITOK);
+
+ p = (char *)id + id->bLength;
end = (char *)dev->cdesc + UGETW(dev->cdesc->wTotalLength);
-#define ed ((usb_endpoint_descriptor_t *)p)
- for (endpt = 0; endpt < nendpt; endpt++) {
- DPRINTFN(10,("usbd_fill_iface_data: endpt=%d\n", endpt));
- for (; p < end; p += ed->bLength) {
- DPRINTFN(10,("usbd_fill_iface_data: p=%p end=%p "
- "len=%d type=%d\n", p, end, ed->bLength,
- ed->bDescriptorType));
- if (p + ed->bLength <= end && ed->bLength != 0 &&
- ed->bDescriptorType == UDESC_ENDPOINT)
- goto found;
+
+ for (endpt = 0; endpt < id->bNumEndpoints; endpt++) {
+ while (p < end) {
+ ed = (usb_endpoint_descriptor_t *)p;
+
if (ed->bLength == 0 ||
ed->bDescriptorType == UDESC_INTERFACE)
break;
+
+ if (p + ed->bLength <= end &&
+ ed->bDescriptorType == UDESC_ENDPOINT) {
+ endpoints[endpt].edesc = usbd_fix_ed(dev, ed);
+ endpoints[endpt].refcnt = 0;
+ endpoints[endpt].savedtoggle = 0;
+ goto found;
+ }
+
+ p += ed->bLength;
}
+
/* passed end, or bad desc */
- printf("usbd_fill_iface_data: bad descriptor(s): %s\n",
- ed->bLength == 0 ? "0 length" :
- ed->bDescriptorType == UDESC_INTERFACE ? "iface desc" :
- "out of data");
- goto bad;
+ free(endpoints, M_USB, id->bNumEndpoints * sizeof(*endpoints));
+ return (-1);
+
found:
- ifc->endpoints[endpt].edesc = ed;
- if (dev->speed == USB_SPEED_HIGH) {
- u_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;
- goto check;
- case UE_BULK:
- mps = USB_2_MAX_BULK_PACKET;
- check:
- if (UGETW(ed->wMaxPacketSize) != mps) {
- USETW(ed->wMaxPacketSize, mps);
-#ifdef DIAGNOSTIC
- printf("usbd_fill_iface_data: bad max "
- "packet size\n");
-#endif
- }
- break;
- default:
- break;
- }
- }
- ifc->endpoints[endpt].refcnt = 0;
- ifc->endpoints[endpt].savedtoggle = 0;
p += ed->bLength;
}
-#undef ed
- LIST_INIT(&ifc->pipes);
- return (USBD_NORMAL_COMPLETION);
-
- bad:
- if (ifc->endpoints != NULL) {
- free(ifc->endpoints, M_USB, 0);
- ifc->endpoints = NULL;
- }
- return (USBD_INVAL);
+
+ iface->device = dev;
+ iface->idesc = id;
+ iface->index = ifaceidx;
+ iface->altindex = altidx;
+ iface->endpoints = endpoints;
+ iface->priv = NULL;
+ LIST_INIT(&iface->pipes);
+ iface->claimed = 0;
+
+ return (0);
}
void
usbd_free_iface_data(struct usbd_device *dev, int ifcno)
{
- struct usbd_interface *ifc = &dev->ifaces[ifcno];
- if (ifc->endpoints)
- free(ifc->endpoints, M_USB, 0);
+ struct usbd_interface *iface = &dev->ifaces[ifcno];
+ int nendpt = iface->idesc->bNumEndpoints;
+
+ free(iface->endpoints, M_USB, nendpt * sizeof(*iface->endpoints));
}
usbd_status
Index: usbdi.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/usbdi.c,v
retrieving revision 1.83
diff -u -p -r1.83 usbdi.c
--- usbdi.c 10 Jul 2015 15:47:48 -0000 1.83
+++ usbdi.c 13 Oct 2015 20:58:02 -0000
@@ -629,20 +629,20 @@ usbd_status
usbd_set_interface(struct usbd_interface *iface, int altidx)
{
usb_device_request_t req;
- usbd_status err;
- void *endpoints;
+ struct usbd_endpoint *endpoints;
+ int nendpt;
if (LIST_FIRST(&iface->pipes) != 0)
return (USBD_IN_USE);
endpoints = iface->endpoints;
- err = usbd_fill_iface_data(iface->device, iface->index, altidx);
- if (err)
- return (err);
+ nendpt = iface->idesc->bNumEndpoints;
+
+ if (usbd_fill_iface_data(iface->device, iface->index, altidx))
+ return (USBD_INVAL);
/* new setting works, we can free old endpoints */
- if (endpoints != NULL)
- free(endpoints, M_USB, 0);
+ free(endpoints, M_USB, nendpt * sizeof(*endpoints));
#ifdef DIAGNOSTIC
if (iface->idesc == NULL) {
Index: usbdivar.h
===================================================================
RCS file: /cvs/src/sys/dev/usb/usbdivar.h,v
retrieving revision 1.69
diff -u -p -r1.69 usbdivar.h
--- usbdivar.h 21 Dec 2014 12:04:01 -0000 1.69
+++ usbdivar.h 13 Oct 2015 20:51:32 -0000
@@ -238,7 +238,7 @@ usbd_status usbd_setup_pipe(struct usbd_
int usbd_set_address(struct usbd_device *, int);
usbd_status usbd_new_device(struct device *, struct usbd_bus *,
int, int, int, struct usbd_port *);
-usbd_status usbd_fill_iface_data(struct usbd_device *, int, int);
+int usbd_fill_iface_data(struct usbd_device *, int, int);
usbd_status usb_insert_transfer(struct usbd_xfer *);
void usb_transfer_complete(struct usbd_xfer *);
