[ 52/91] USB: Remove races in devio.c

From: Greg KH
Date: Sat May 26 2012 - 21:18:04 EST


3.4-stable review patch. If anyone has any objections, please let me know.

------------------

From: Huajun Li <huajun.li.lee@xxxxxxxxx>

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@xxxxxxxxx>
Cc: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
Cc: Oncaphillis <oncaphillis@xxxxxxxx>
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>

---
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;
}



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/