Skip to content

Commit

Permalink
wusbcore: clean up list locking in urb enqueue
Browse files Browse the repository at this point in the history
wa_urb_enqueue_run locks and unlocks its list lock as it traverses the
list of queued transfers.  This was done to prevent deadlocking due to
acquiring locks in reverse order in different places.  The problem is that
releasing the lock during the list traversal could allow the dequeue
routine to corrupt the list while it is being iterated over.  This patch
moves all list entries to a temp list while holding the list lock, then
traverses the temp list with no lock held.

Signed-off-by: Thomas Pugliese <thomas.pugliese@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  • Loading branch information
Thomas Pugliese authored and Greg Kroah-Hartman committed Aug 12, 2013
1 parent 467d296 commit e9a088f
Showing 1 changed file with 15 additions and 10 deletions.
25 changes: 15 additions & 10 deletions drivers/usb/wusbcore/wa-xfer.c
Original file line number Diff line number Diff line change
Expand Up @@ -928,7 +928,7 @@ static void wa_xfer_delayed_run(struct wa_rpipe *rpipe)
spin_lock_irqsave(&rpipe->seg_lock, flags);
while (atomic_read(&rpipe->segs_available) > 0
&& !list_empty(&rpipe->seg_list)) {
seg = list_entry(rpipe->seg_list.next, struct wa_seg,
seg = list_first_entry(&(rpipe->seg_list), struct wa_seg,
list_node);
list_del(&seg->list_node);
xfer = seg->xfer;
Expand Down Expand Up @@ -1093,30 +1093,35 @@ static void wa_urb_enqueue_b(struct wa_xfer *xfer)
*
* We need to be careful here, as dequeue() could be called in the
* middle. That's why we do the whole thing under the
* wa->xfer_list_lock. If dequeue() jumps in, it first locks urb->lock
* wa->xfer_list_lock. If dequeue() jumps in, it first locks xfer->lock
* and then checks the list -- so as we would be acquiring in inverse
* order, we just drop the lock once we have the xfer and reacquire it
* later.
* order, we move the delayed list to a separate list while locked and then
* submit them without the list lock held.
*/
void wa_urb_enqueue_run(struct work_struct *ws)
{
struct wahc *wa = container_of(ws, struct wahc, xfer_work);
struct wa_xfer *xfer, *next;
struct urb *urb;
LIST_HEAD(tmp_list);

/* Create a copy of the wa->xfer_delayed_list while holding the lock */
spin_lock_irq(&wa->xfer_list_lock);
list_for_each_entry_safe(xfer, next, &wa->xfer_delayed_list,
list_node) {
list_cut_position(&tmp_list, &wa->xfer_delayed_list,
wa->xfer_delayed_list.prev);
spin_unlock_irq(&wa->xfer_list_lock);

/*
* enqueue from temp list without list lock held since wa_urb_enqueue_b
* can take xfer->lock as well as lock mutexes.
*/
list_for_each_entry_safe(xfer, next, &tmp_list, list_node) {
list_del_init(&xfer->list_node);
spin_unlock_irq(&wa->xfer_list_lock);

urb = xfer->urb;
wa_urb_enqueue_b(xfer);
usb_put_urb(urb); /* taken when queuing */

spin_lock_irq(&wa->xfer_list_lock);
}
spin_unlock_irq(&wa->xfer_list_lock);
}
EXPORT_SYMBOL_GPL(wa_urb_enqueue_run);

Expand Down

0 comments on commit e9a088f

Please sign in to comment.