lkml.org 
[lkml]   [2012]   [May]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
Subject[ 52/91] USB: Remove races in devio.c
3.4-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
@@ -333,17 +333,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;
}

@@ -398,6 +395,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
@@ -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 *

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_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);
@@ -1410,12 +1415,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;
}




\
 
 \ /
  Last update: 2012-05-27 05:01    [from the cache]
©2003-2014 Jasper Spaans. Advertise on this site