1
0
Fork 0

USB: Remove races in devio.c

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>
hifive-unleashed-5.1
Huajun Li 2012-05-18 20:12:51 +08:00 committed by Greg Kroah-Hartman
parent 8377c94f62
commit 4e09dcf20f
1 changed files with 25 additions and 8 deletions

View File

@ -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;
}
@ -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
@ -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;
}
@ -472,6 +473,7 @@ 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;
@ -479,10 +481,13 @@ static void destroy_async(struct dev_state *ps, struct list_head *list)
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);
@ -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;
}