From 1c15ab5e770d230037b9b9c888889fd3bd875215 Mon Sep 17 00:00:00 2001 From: Taylor R Campbell Date: Sat, 12 Jun 2021 14:07:25 +0000 Subject: [PATCH] usb(4): Fix races between usbd_open_pipe* and usbd_set_interface. --- sys/dev/usb/usb_subr.c | 62 ++++++++++++++++++++++++++++++++++++++---- sys/dev/usb/usbdi.c | 21 ++++++++++---- sys/dev/usb/usbdivar.h | 1 + 3 files changed, 72 insertions(+), 12 deletions(-) diff --git a/sys/dev/usb/usb_subr.c b/sys/dev/usb/usb_subr.c index abdd27502fca..18fb57eba1ed 100644 --- a/sys/dev/usb/usb_subr.c +++ b/sys/dev/usb/usb_subr.c @@ -403,6 +403,39 @@ usbd_find_edesc(usb_config_descriptor_t *cd, int ifaceidx, int altidx, return NULL; } +static void +usbd_iface_init(struct usbd_device *dev, int ifaceidx) +{ + struct usbd_interface *ifc = &dev->ud_ifaces[ifaceidx]; + + memset(ifc, 0, sizeof(*ifc)); + + ifc->ui_dev = dev; + ifc->ui_idesc = NULL; + ifc->ui_index = 0; + ifc->ui_altindex = 0; + ifc->ui_endpoints = NULL; + ifc->ui_priv = NULL; + LIST_INIT(&ifc->ui_pipes); + mutex_init(&ifc->ui_pipelock, MUTEX_DEFAULT, IPL_NONE); +} + +static void +usbd_iface_fini(struct usbd_device *dev, int ifaceidx) +{ + struct usbd_interface *ifc = &dev->ud_ifaces[ifaceidx]; + + KASSERT(ifc->ui_dev == dev); + KASSERT(ifc->ui_idesc == NULL); + KASSERT(ifc->ui_index == 0); + KASSERT(ifc->ui_altindex == 0); + KASSERT(ifc->ui_endpoints == NULL); + KASSERT(ifc->ui_priv == NULL); + KASSERT(LIST_EMPTY(&ifc->ui_pipes)); + + mutex_destroy(&ifc->ui_pipelock); +} + usbd_status usbd_fill_iface_data(struct usbd_device *dev, int ifaceidx, int altidx) { @@ -414,10 +447,12 @@ usbd_fill_iface_data(struct usbd_device *dev, int ifaceidx, int altidx) char *p, *end; int endpt, nendpt; + KASSERT(ifc->ui_dev == dev); + KASSERT(LIST_EMPTY(&ifc->ui_pipes)); + idesc = usbd_find_idesc(dev->ud_cdesc, ifaceidx, altidx); if (idesc == NULL) return USBD_INVAL; - ifc->ui_dev = dev; ifc->ui_idesc = idesc; ifc->ui_index = ifaceidx; ifc->ui_altindex = altidx; @@ -488,7 +523,6 @@ usbd_fill_iface_data(struct usbd_device *dev, int ifaceidx, int altidx) p += ed->bLength; } #undef ed - LIST_INIT(&ifc->ui_pipes); return USBD_NORMAL_COMPLETION; bad: @@ -499,16 +533,25 @@ usbd_fill_iface_data(struct usbd_device *dev, int ifaceidx, int altidx) return USBD_INVAL; } -void +Static void usbd_free_iface_data(struct usbd_device *dev, int ifcno) { struct usbd_interface *ifc = &dev->ud_ifaces[ifcno]; + + KASSERT(ifc->ui_dev == dev); + KASSERT(ifc->ui_idesc != NULL); + KASSERT(LIST_EMPTY(&ifc->ui_pipes)); + if (ifc->ui_endpoints) { int nendpt = ifc->ui_idesc->bNumEndpoints; size_t sz = nendpt * sizeof(struct usbd_endpoint); kmem_free(ifc->ui_endpoints, sz); ifc->ui_endpoints = NULL; } + + ifc->ui_altindex = 0; + ifc->ui_index = 0; + ifc->ui_idesc = NULL; } usbd_status @@ -557,8 +600,10 @@ usbd_set_config_index(struct usbd_device *dev, int index, int msg) DPRINTF("free old config", 0, 0, 0, 0); /* Free all configuration data structures. */ nifc = dev->ud_cdesc->bNumInterface; - for (ifcidx = 0; ifcidx < nifc; ifcidx++) + for (ifcidx = 0; ifcidx < nifc; ifcidx++) { usbd_free_iface_data(dev, ifcidx); + usbd_iface_fini(dev, ifcidx); + } kmem_free(dev->ud_ifaces, nifc * sizeof(struct usbd_interface)); kmem_free(dev->ud_cdesc, UGETW(dev->ud_cdesc->wTotalLength)); if (dev->ud_bdesc != NULL) @@ -730,10 +775,13 @@ usbd_set_config_index(struct usbd_device *dev, int index, int msg) dev->ud_cdesc = cdp; dev->ud_config = cdp->bConfigurationValue; for (ifcidx = 0; ifcidx < nifc; ifcidx++) { + usbd_iface_init(dev, ifcidx); err = usbd_fill_iface_data(dev, ifcidx, 0); if (err) { - while (--ifcidx >= 0) + while (--ifcidx >= 0) { usbd_free_iface_data(dev, ifcidx); + usbd_iface_fini(dev, ifcidx); + } kmem_free(dev->ud_ifaces, nifc * sizeof(struct usbd_interface)); dev->ud_ifaces = NULL; @@ -1649,8 +1697,10 @@ usb_free_device(struct usbd_device *dev) usbd_kill_pipe(dev->ud_pipe0); if (dev->ud_ifaces != NULL) { nifc = dev->ud_cdesc->bNumInterface; - for (ifcidx = 0; ifcidx < nifc; ifcidx++) + for (ifcidx = 0; ifcidx < nifc; ifcidx++) { usbd_free_iface_data(dev, ifcidx); + usbd_iface_fini(dev, ifcidx); + } kmem_free(dev->ud_ifaces, nifc * sizeof(struct usbd_interface)); } diff --git a/sys/dev/usb/usbdi.c b/sys/dev/usb/usbdi.c index 59e4f42dc59b..ebaf27628ca8 100644 --- a/sys/dev/usb/usbdi.c +++ b/sys/dev/usb/usbdi.c @@ -244,7 +244,9 @@ usbd_open_pipe_ival(struct usbd_interface *iface, uint8_t address, err = usbd_setup_pipe_flags(iface->ui_dev, iface, ep, ival, &p, flags); if (err) return err; + mutex_enter(&iface->ui_pipelock); LIST_INSERT_HEAD(&iface->ui_pipes, p, up_next); + mutex_exit(&iface->ui_pipelock); *pipe = p; SDT_PROBE5(usb, device, pipe, open, iface, address, flags, ival, p); @@ -313,13 +315,14 @@ usbd_close_pipe(struct usbd_pipe *pipe) KASSERT(SIMPLEQ_EMPTY(&pipe->up_queue)); - LIST_REMOVE(pipe, up_next); - pipe->up_methods->upm_close(pipe); usbd_unlock_pipe(pipe); if (pipe->up_intrxfer != NULL) usbd_destroy_xfer(pipe->up_intrxfer); + mutex_enter(&pipe->up_iface->ui_pipelock); + LIST_REMOVE(pipe, up_next); + mutex_exit(&pipe->up_iface->ui_pipelock); usb_rem_task_wait(pipe->up_dev, &pipe->up_async_task, USB_TASKQ_DRIVER, NULL); usbd_endpoint_release(pipe->up_dev, pipe->up_endpoint); @@ -872,8 +875,11 @@ usbd_set_interface(struct usbd_interface *iface, int altidx) USBHIST_FUNC(); - if (LIST_FIRST(&iface->ui_pipes) != NULL) - return USBD_IN_USE; + mutex_enter(&iface->ui_pipelock); + if (LIST_FIRST(&iface->ui_pipes) != NULL) { + err = USBD_IN_USE; + goto out; + } endpoints = iface->ui_endpoints; int nendpt = iface->ui_idesc->bNumEndpoints; @@ -882,7 +888,7 @@ usbd_set_interface(struct usbd_interface *iface, int altidx) iface->ui_idesc->bNumEndpoints, 0); err = usbd_fill_iface_data(iface->ui_dev, iface->ui_index, altidx); if (err) - return err; + goto out; /* new setting works, we can free old endpoints */ if (endpoints != NULL) { @@ -897,7 +903,10 @@ usbd_set_interface(struct usbd_interface *iface, int altidx) USETW(req.wValue, iface->ui_idesc->bAlternateSetting); USETW(req.wIndex, iface->ui_idesc->bInterfaceNumber); USETW(req.wLength, 0); - return usbd_do_request(iface->ui_dev, &req, 0); + err = usbd_do_request(iface->ui_dev, &req, 0); + +out: mutex_exit(&iface->ui_pipelock); + return err; } int diff --git a/sys/dev/usb/usbdivar.h b/sys/dev/usb/usbdivar.h index 9d48a56f05d3..19e4c2bcb8ff 100644 --- a/sys/dev/usb/usbdivar.h +++ b/sys/dev/usb/usbdivar.h @@ -231,6 +231,7 @@ struct usbd_interface { int ui_altindex; struct usbd_endpoint *ui_endpoints; void *ui_priv; + kmutex_t ui_pipelock; LIST_HEAD(, usbd_pipe) ui_pipes; };