From 9f5b544589c28db6dc6755d081e71e0986bff8b6 Mon Sep 17 00:00:00 2001 From: hselasky Date: Thu, 27 Mar 2014 06:59:56 +0000 Subject: [PATCH] MFC r263423: Try to resolve a possible deadlock when detaching USB devices which create character devices. The deadlock can happen if an application is issuing IOCTLs which require USB refcounting, at the same time the USB device is detaching. There is already a counter in place in the USB device structure to detect this situation, but it was not always checked ahead of invoking functions that might destroy character devices, like detach, set configuration, set alternate interface or detach active kernel driver. git-svn-id: svn://svn.freebsd.org/base/stable/10@263799 ccf9f872-aa2e-dd11-9fc8-001c23d0bc1f --- sys/dev/usb/usb_dev.c | 19 +++++----- sys/dev/usb/usb_device.c | 78 +++++++++++++++++++++++++++++++++++---- sys/dev/usb/usb_process.c | 12 ++++++ sys/dev/usb/usb_process.h | 1 + 4 files changed, 92 insertions(+), 18 deletions(-) diff --git a/sys/dev/usb/usb_dev.c b/sys/dev/usb/usb_dev.c index c81b942be..e4471aee0 100644 --- a/sys/dev/usb/usb_dev.c +++ b/sys/dev/usb/usb_dev.c @@ -214,12 +214,13 @@ usb_ref_device(struct usb_cdev_privdata *cpd, DPRINTFN(2, "device is detached\n"); goto error; } - if (cpd->udev->refcount == USB_DEV_REF_MAX) { - DPRINTFN(2, "no dev ref\n"); - goto error; - } if (need_uref) { DPRINTFN(2, "ref udev - needed\n"); + + if (cpd->udev->refcount == USB_DEV_REF_MAX) { + DPRINTFN(2, "no dev ref\n"); + goto error; + } cpd->udev->refcount++; mtx_unlock(&usb_ref_lock); @@ -293,9 +294,8 @@ usb_ref_device(struct usb_cdev_privdata *cpd, usbd_enum_unlock(cpd->udev); if (crd->is_uref) { - if (--(cpd->udev->refcount) == 0) { - cv_signal(&cpd->udev->ref_cv); - } + cpd->udev->refcount--; + cv_broadcast(&cpd->udev->ref_cv); } mtx_unlock(&usb_ref_lock); DPRINTFN(2, "fail\n"); @@ -361,10 +361,9 @@ usb_unref_device(struct usb_cdev_privdata *cpd, crd->is_write = 0; } if (crd->is_uref) { - if (--(cpd->udev->refcount) == 0) { - cv_signal(&cpd->udev->ref_cv); - } crd->is_uref = 0; + cpd->udev->refcount--; + cv_broadcast(&cpd->udev->ref_cv); } mtx_unlock(&usb_ref_lock); } diff --git a/sys/dev/usb/usb_device.c b/sys/dev/usb/usb_device.c index 76d493c9c..33dfd03ad 100644 --- a/sys/dev/usb/usb_device.c +++ b/sys/dev/usb/usb_device.c @@ -451,6 +451,65 @@ usb_endpoint_foreach(struct usb_device *udev, struct usb_endpoint *ep) return (NULL); } +/*------------------------------------------------------------------------* + * usb_wait_pending_ref_locked + * + * This function will wait for any USB references to go away before + * returning and disable further USB device refcounting on the + * specified USB device. This function is used when detaching a USB + * device. + *------------------------------------------------------------------------*/ +static void +usb_wait_pending_ref_locked(struct usb_device *udev) +{ +#if USB_HAVE_UGEN + const uint16_t refcount = + usb_proc_is_called_from( + USB_BUS_EXPLORE_PROC(udev->bus)) ? 1 : 2; + + DPRINTF("Refcount = %d\n", (int)refcount); + + while (1) { + /* wait for any pending references to go away */ + mtx_lock(&usb_ref_lock); + if (udev->refcount == refcount) { + /* prevent further refs being taken */ + udev->refcount = USB_DEV_REF_MAX; + mtx_unlock(&usb_ref_lock); + break; + } + usbd_enum_unlock(udev); + cv_wait(&udev->ref_cv, &usb_ref_lock); + mtx_unlock(&usb_ref_lock); + (void) usbd_enum_lock(udev); + } +#endif +} + +/*------------------------------------------------------------------------* + * usb_ref_restore_locked + * + * This function will restore the reference count value after a call + * to "usb_wait_pending_ref_locked()". + *------------------------------------------------------------------------*/ +static void +usb_ref_restore_locked(struct usb_device *udev) +{ +#if USB_HAVE_UGEN + const uint16_t refcount = + usb_proc_is_called_from( + USB_BUS_EXPLORE_PROC(udev->bus)) ? 1 : 2; + + DPRINTF("Refcount = %d\n", (int)refcount); + + /* restore reference count and wakeup waiters, if any */ + mtx_lock(&usb_ref_lock); + udev->refcount = refcount; + cv_broadcast(&udev->ref_cv); + mtx_unlock(&usb_ref_lock); +#endif +} + /*------------------------------------------------------------------------* * usb_unconfigure * @@ -1121,6 +1180,9 @@ usb_detach_device(struct usb_device *udev, uint8_t iface_index, sx_assert(&udev->enum_sx, SA_LOCKED); + /* wait for pending refs to go away */ + usb_wait_pending_ref_locked(udev); + /* * First detach the child to give the child's detach routine a * chance to detach the sub-devices in the correct order. @@ -1147,6 +1209,8 @@ usb_detach_device(struct usb_device *udev, uint8_t iface_index, usb_detach_device_sub(udev, &iface->subdev, &iface->pnpinfo, flag); } + + usb_ref_restore_locked(udev); } /*------------------------------------------------------------------------* @@ -2088,14 +2152,6 @@ usb_free_device(struct usb_device *udev, uint8_t flag) udev->ugen_symlink = NULL; } - /* wait for all pending references to go away: */ - mtx_lock(&usb_ref_lock); - udev->refcount--; - while (udev->refcount != 0) { - cv_wait(&udev->ref_cv, &usb_ref_lock); - } - mtx_unlock(&usb_ref_lock); - usb_destroy_dev(udev->ctrl_dev); #endif @@ -2612,8 +2668,14 @@ usb_fifo_free_wrap(struct usb_device *udev, /* no need to free this FIFO */ continue; } + /* wait for pending refs to go away */ + usb_wait_pending_ref_locked(udev); + /* free this FIFO */ usb_fifo_free(f); + + /* restore refcount */ + usb_ref_restore_locked(udev); } } #endif diff --git a/sys/dev/usb/usb_process.c b/sys/dev/usb/usb_process.c index 7493e39c6..e70166ca4 100644 --- a/sys/dev/usb/usb_process.c +++ b/sys/dev/usb/usb_process.c @@ -501,3 +501,15 @@ usb_proc_rewakeup(struct usb_process *up) cv_signal(&up->up_cv); } } + +/*------------------------------------------------------------------------* + * usb_proc_is_called_from + * + * This function will return non-zero if called from inside the USB + * process passed as first argument. Else this function returns zero. + *------------------------------------------------------------------------*/ +int +usb_proc_is_called_from(struct usb_process *up) +{ + return (up->up_curtd == curthread); +} diff --git a/sys/dev/usb/usb_process.h b/sys/dev/usb/usb_process.h index 97262c41d..c12cdc455 100644 --- a/sys/dev/usb/usb_process.h +++ b/sys/dev/usb/usb_process.h @@ -81,6 +81,7 @@ void usb_proc_mwait(struct usb_process *up, void *pm0, void *pm1); void usb_proc_free(struct usb_process *up); void *usb_proc_msignal(struct usb_process *up, void *pm0, void *pm1); void usb_proc_rewakeup(struct usb_process *up); +int usb_proc_is_called_from(struct usb_process *up); void usb_proc_explore_mwait(struct usb_device *, void *, void *); void *usb_proc_explore_msignal(struct usb_device *, void *, void *); -- 2.45.0