diff options
author | Huajun Li <huajun.li.lee@gmail.com> | 2012-05-18 20:12:51 +0800 |
---|---|---|
committer | Greg Kroah-Hartman <gregkh@linuxfoundation.org> | 2012-05-18 16:37:55 -0700 |
commit | 4e09dcf20f7b5358615514c2ec8584b248ab8874 (patch) | |
tree | 52847eb294c7a08a10d27d8bf35844fb8cd0cf8f /drivers/usb/core | |
parent | 8377c94f627f7943da9a7eefdb21fd2e9e7ec629 (diff) |
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>
Diffstat (limited to 'drivers/usb/core')
-rw-r--r-- | drivers/usb/core/devio.c | 33 |
1 files changed, 25 insertions, 8 deletions
diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c index c4a1af8a954..e0f107948eb 100644 --- a/drivers/usb/core/devio.c +++ b/drivers/usb/core/devio.c @@ -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; } |