staging: usbip: avoid deadlock in vhci_device_unlink_cleanup()

Almost all of usbip assumes that the_controller->lock is acquired
before vdev->priv_lock. The exception is in
vhci_device_unlink_cleanup(), where locks are acquired in the
reverse order. This leads to occasional deadlocks.

Fixing this is a bit fiddly, as the_controller->lock can't be held
when calling usb_hcd_unlink_urb_from_ep() in the middle of the list
traversal. As I can't rule out concurrent callers to this function
(perhaps it is safe?), the code here becomes slightly uglier - when
locks are dropped in the middle so the list may have emptied itself
(not even list_for_each_entry_safe is safe here).

Signed-off-by: Bernard Blackham <b-linuxgit@largestprime.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/staging/usbip/vhci_hcd.c b/drivers/staging/usbip/vhci_hcd.c
index a5b028d074b87b1112ac263b5659ac2010c18688..fd2b21a6c20bc68c041bfdae58cf6549a3e5d720 100644 (file)
--- a/drivers/staging/usbip/vhci_hcd.c
+++ b/drivers/staging/usbip/vhci_hcd.c
@@ -749,6 +749,7 @@ static void vhci_device_unlink_cleanup(struct vhci_device *vdev)
 {
        struct vhci_unlink *unlink, *tmp;
 
+       spin_lock(&the_controller->lock);
        spin_lock(&vdev->priv_lock);
 
        list_for_each_entry_safe(unlink, tmp, &vdev->unlink_tx, list) {
@@ -757,9 +758,12 @@ static void vhci_device_unlink_cleanup(struct vhci_device *vdev)
                kfree(unlink);
        }
 
-       list_for_each_entry_safe(unlink, tmp, &vdev->unlink_rx, list) {
+       while (!list_empty(&vdev->unlink_rx)) {
                struct urb *urb;
 
+               unlink = list_first_entry(&vdev->unlink_rx, struct vhci_unlink,
+                       list);
+
                /* give back URB of unanswered unlink request */
                pr_info("unlink cleanup rx %lu\n", unlink->unlink_seqnum);
 
@@ -774,18 +778,24 @@ static void vhci_device_unlink_cleanup(struct vhci_device *vdev)
 
                urb->status = -ENODEV;
 
-               spin_lock(&the_controller->lock);
                usb_hcd_unlink_urb_from_ep(vhci_to_hcd(the_controller), urb);
+
+               list_del(&unlink->list);
+
+               spin_unlock(&vdev->priv_lock);
                spin_unlock(&the_controller->lock);
 
                usb_hcd_giveback_urb(vhci_to_hcd(the_controller), urb,
                                     urb->status);
 
-               list_del(&unlink->list);
+               spin_lock(&the_controller->lock);
+               spin_lock(&vdev->priv_lock);
+
                kfree(unlink);
        }
 
        spin_unlock(&vdev->priv_lock);
+       spin_unlock(&the_controller->lock);
 }
 
 /*