From d7b0865588adf7662dfcf055d2f5b4b6b69e0d0a Mon Sep 17 00:00:00 2001 From: hselasky Date: Fri, 24 Jan 2014 08:10:08 +0000 Subject: [PATCH] MFC r260808 and r260814: - Close a minor deadlock. - Fix a possible memory use after free and leak situation associated with USB device detach when using character device handles. This also includes LibUSB. It turns out that "usb_close()" cannot always get a reference to clean up its USB transfers and such, if called during the kernel USB device detach. git-svn-id: svn://svn.freebsd.org/base/stable/10@261110 ccf9f872-aa2e-dd11-9fc8-001c23d0bc1f --- sys/dev/usb/usb_dev.c | 33 +++++++++++++++++---------------- sys/dev/usb/usb_device.c | 24 ++++++++++++++---------- 2 files changed, 31 insertions(+), 26 deletions(-) diff --git a/sys/dev/usb/usb_dev.c b/sys/dev/usb/usb_dev.c index 9e3cef522..6a3080f1f 100644 --- a/sys/dev/usb/usb_dev.c +++ b/sys/dev/usb/usb_dev.c @@ -207,6 +207,11 @@ usb_ref_device(struct usb_cdev_privdata *cpd, DPRINTFN(2, "no device at %u\n", cpd->dev_index); goto error; } + if (cpd->udev->state == USB_STATE_DETACHED && + (need_uref != 2)) { + DPRINTFN(2, "device is detached\n"); + goto error; + } if (cpd->udev->refcount == USB_DEV_REF_MAX) { DPRINTFN(2, "no dev ref\n"); goto error; @@ -597,6 +602,13 @@ usb_fifo_free(struct usb_fifo *f) mtx_unlock(f->priv_mtx); mtx_lock(&usb_ref_lock); + /* + * Check if the "f->refcount" variable reached zero + * during the unlocked time before entering wait: + */ + if (f->refcount == 0) + break; + /* wait for sync */ cv_wait(&f->cv_drain, &usb_ref_lock); } @@ -915,23 +927,12 @@ usb_close(void *arg) DPRINTFN(2, "cpd=%p\n", cpd); - err = usb_ref_device(cpd, &refs, 0); - if (err) + err = usb_ref_device(cpd, &refs, + 2 /* uref and allow detached state */); + if (err) { + DPRINTFN(0, "Cannot grab USB reference when " + "closing USB file handle\n"); goto done; - - /* - * If this function is not called directly from the root HUB - * thread, there is usually a need to lock the enumeration - * lock. Check this. - */ - if (!usbd_enum_is_locked(cpd->udev)) { - - DPRINTFN(2, "Locking enumeration\n"); - - /* reference device */ - err = usb_usb_ref_device(cpd, &refs); - if (err) - goto done; } if (cpd->fflags & FREAD) { usb_fifo_close(refs.rxfifo, cpd->fflags); diff --git a/sys/dev/usb/usb_device.c b/sys/dev/usb/usb_device.c index 3af48d8f7..76d493c9c 100644 --- a/sys/dev/usb/usb_device.c +++ b/sys/dev/usb/usb_device.c @@ -2070,6 +2070,8 @@ usb_free_device(struct usb_device *udev, uint8_t flag) DPRINTFN(4, "udev=%p port=%d\n", udev, udev->port_no); bus = udev->bus; + + /* set DETACHED state to prevent any further references */ usb_set_device_state(udev, USB_STATE_DETACHED); #if USB_HAVE_DEVCTL @@ -2085,16 +2087,7 @@ usb_free_device(struct usb_device *udev, uint8_t flag) usb_free_symlink(udev->ugen_symlink); udev->ugen_symlink = NULL; } -#endif - /* - * Unregister our device first which will prevent any further - * references: - */ - usb_bus_port_set_device(bus, udev->parent_hub ? - udev->parent_hub->hub->ports + udev->port_index : NULL, - NULL, USB_ROOT_HUB_ADDR); -#if USB_HAVE_UGEN /* wait for all pending references to go away: */ mtx_lock(&usb_ref_lock); udev->refcount--; @@ -2114,6 +2107,11 @@ usb_free_device(struct usb_device *udev, uint8_t flag) /* the following will get the device unconfigured in software */ usb_unconfigure(udev, USB_UNCFG_FLAG_FREE_EP0); + /* final device unregister after all character devices are closed */ + usb_bus_port_set_device(bus, udev->parent_hub ? + udev->parent_hub->hub->ports + udev->port_index : NULL, + NULL, USB_ROOT_HUB_ADDR); + /* unsetup any leftover default USB transfers */ usbd_transfer_unsetup(udev->ctrl_xfer, USB_CTRL_XFER_MAX); @@ -2647,8 +2645,14 @@ usb_set_device_state(struct usb_device *udev, enum usb_dev_state state) DPRINTF("udev %p state %s -> %s\n", udev, usb_statestr(udev->state), usb_statestr(state)); - udev->state = state; +#if USB_HAVE_UGEN + mtx_lock(&usb_ref_lock); +#endif + udev->state = state; +#if USB_HAVE_UGEN + mtx_unlock(&usb_ref_lock); +#endif if (udev->bus->methods->device_state_change != NULL) (udev->bus->methods->device_state_change) (udev); } -- 2.45.0