Messages in this thread Patch in this message | | | Date | Sun, 27 May 2012 09:26:48 +0900 | From | Greg KH <> | Subject | [ 35/55] USB: Remove races in devio.c |
| |
3.0-stable review patch. If anyone has any objections, please let me know.
------------------
From: Huajun Li <huajun.li.lee@gmail.com>
commit 4e09dcf20f7b5358615514c2ec8584b248ab8874 upstream.
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> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
--- drivers/usb/core/devio.c | 33 +++++++++++++++++++++++++-------- 1 file changed, 25 insertions(+), 8 deletions(-)
--- a/drivers/usb/core/devio.c +++ b/drivers/usb/core/devio.c @@ -292,17 +292,14 @@ static struct async *async_getcompleted( 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; } @@ -357,6 +354,7 @@ static void cancel_bulk_urbs(struct dev_ __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 @@ -379,8 +377,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; } @@ -433,6 +434,7 @@ static void async_completed(struct urb * static void destroy_async(struct dev_state *ps, struct list_head *list) { + struct urb *urb; struct async *as; unsigned long flags; @@ -440,10 +442,13 @@ static void destroy_async(struct dev_sta 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); @@ -1352,12 +1357,24 @@ static int proc_submiturb(struct dev_sta 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; }
| |