Skip to content

Commit

Permalink
USB: Remove races in devio.c
Browse files Browse the repository at this point in the history
There exist races in devio.c, below is one case,
and there are similar races in destroy_async()
and proc_unlinkurb().  Remove these races.

 cancel_bulk_urbs()        async_completed()
-------------------                -----------------------
 spin_unlock(&ps->lock);

                           list_move_tail(&as->asynclist,
		                    &ps->async_completed);

                           wake_up(&ps->wait);

                           Lead to free_async() be triggered,
                           then urb and 'as' will be freed.

 usb_unlink_urb(as->urb);
 ===> refer to the freed 'as'

Signed-off-by: Huajun Li <huajun.li.lee@gmail.com>
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: Oncaphillis <oncaphillis@snafu.de>
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  • Loading branch information
Huajun Li authored and Greg Kroah-Hartman committed May 18, 2012
1 parent 8377c94 commit 4e09dcf
Showing 1 changed file with 25 additions and 8 deletions.
33 changes: 25 additions & 8 deletions drivers/usb/core/devio.c
Original file line number Diff line number Diff line change
Expand Up @@ -333,17 +333,14 @@ static struct async *async_getcompleted(struct dev_state *ps)
static struct async *async_getpending(struct dev_state *ps,
void __user *userurb)
{
unsigned long flags;
struct async *as;

spin_lock_irqsave(&ps->lock, flags);
list_for_each_entry(as, &ps->async_pending, asynclist)
if (as->userurb == userurb) {
list_del_init(&as->asynclist);
spin_unlock_irqrestore(&ps->lock, flags);
return as;
}
spin_unlock_irqrestore(&ps->lock, flags);

return NULL;
}

Expand Down Expand Up @@ -398,6 +395,7 @@ static void cancel_bulk_urbs(struct dev_state *ps, unsigned bulk_addr)
__releases(ps->lock)
__acquires(ps->lock)
{
struct urb *urb;
struct async *as;

/* Mark all the pending URBs that match bulk_addr, up to but not
Expand All @@ -420,8 +418,11 @@ __acquires(ps->lock)
list_for_each_entry(as, &ps->async_pending, asynclist) {
if (as->bulk_status == AS_UNLINK) {
as->bulk_status = 0; /* Only once */
urb = as->urb;
usb_get_urb(urb);
spin_unlock(&ps->lock); /* Allow completions */
usb_unlink_urb(as->urb);
usb_unlink_urb(urb);
usb_put_urb(urb);
spin_lock(&ps->lock);
goto rescan;
}
Expand Down Expand Up @@ -472,17 +473,21 @@ static void async_completed(struct urb *urb)

static void destroy_async(struct dev_state *ps, struct list_head *list)
{
struct urb *urb;
struct async *as;
unsigned long flags;

spin_lock_irqsave(&ps->lock, flags);
while (!list_empty(list)) {
as = list_entry(list->next, struct async, asynclist);
list_del_init(&as->asynclist);
urb = as->urb;
usb_get_urb(urb);

/* drop the spinlock so the completion handler can run */
spin_unlock_irqrestore(&ps->lock, flags);
usb_kill_urb(as->urb);
usb_kill_urb(urb);
usb_put_urb(urb);
spin_lock_irqsave(&ps->lock, flags);
}
spin_unlock_irqrestore(&ps->lock, flags);
Expand Down Expand Up @@ -1399,12 +1404,24 @@ static int proc_submiturb(struct dev_state *ps, void __user *arg)

static int proc_unlinkurb(struct dev_state *ps, void __user *arg)
{
struct urb *urb;
struct async *as;
unsigned long flags;

spin_lock_irqsave(&ps->lock, flags);
as = async_getpending(ps, arg);
if (!as)
if (!as) {
spin_unlock_irqrestore(&ps->lock, flags);
return -EINVAL;
usb_kill_urb(as->urb);
}

urb = as->urb;
usb_get_urb(urb);
spin_unlock_irqrestore(&ps->lock, flags);

usb_kill_urb(urb);
usb_put_urb(urb);

return 0;
}

Expand Down

0 comments on commit 4e09dcf

Please sign in to comment.