Commit 236742de authored by Bernard Blackham's avatar Bernard Blackham Committed by Greg Kroah-Hartman

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: default avatarBernard Blackham <b-linuxgit@largestprime.net>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent 73295fe1
...@@ -749,6 +749,7 @@ static void vhci_device_unlink_cleanup(struct vhci_device *vdev) ...@@ -749,6 +749,7 @@ static void vhci_device_unlink_cleanup(struct vhci_device *vdev)
{ {
struct vhci_unlink *unlink, *tmp; struct vhci_unlink *unlink, *tmp;
spin_lock(&the_controller->lock);
spin_lock(&vdev->priv_lock); spin_lock(&vdev->priv_lock);
list_for_each_entry_safe(unlink, tmp, &vdev->unlink_tx, list) { 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) ...@@ -757,9 +758,12 @@ static void vhci_device_unlink_cleanup(struct vhci_device *vdev)
kfree(unlink); kfree(unlink);
} }
list_for_each_entry_safe(unlink, tmp, &vdev->unlink_rx, list) { while (!list_empty(&vdev->unlink_rx)) {
struct urb *urb; struct urb *urb;
unlink = list_first_entry(&vdev->unlink_rx, struct vhci_unlink,
list);
/* give back URB of unanswered unlink request */ /* give back URB of unanswered unlink request */
pr_info("unlink cleanup rx %lu\n", unlink->unlink_seqnum); pr_info("unlink cleanup rx %lu\n", unlink->unlink_seqnum);
...@@ -774,18 +778,24 @@ static void vhci_device_unlink_cleanup(struct vhci_device *vdev) ...@@ -774,18 +778,24 @@ static void vhci_device_unlink_cleanup(struct vhci_device *vdev)
urb->status = -ENODEV; urb->status = -ENODEV;
spin_lock(&the_controller->lock);
usb_hcd_unlink_urb_from_ep(vhci_to_hcd(the_controller), urb); 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); spin_unlock(&the_controller->lock);
usb_hcd_giveback_urb(vhci_to_hcd(the_controller), urb, usb_hcd_giveback_urb(vhci_to_hcd(the_controller), urb,
urb->status); urb->status);
list_del(&unlink->list); spin_lock(&the_controller->lock);
spin_lock(&vdev->priv_lock);
kfree(unlink); kfree(unlink);
} }
spin_unlock(&vdev->priv_lock); spin_unlock(&vdev->priv_lock);
spin_unlock(&the_controller->lock);
} }
/* /*
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment