]> git.karo-electronics.de Git - karo-tx-linux.git/commitdiff
USB: Remove races in devio.c
authorHuajun Li <huajun.li.lee@gmail.com>
Fri, 18 May 2012 12:12:51 +0000 (20:12 +0800)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Fri, 1 Jun 2012 07:12:57 +0000 (15:12 +0800)
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

index 0ca54e22d3191f673c07ff50d78d027dd4a6c278..ca3c303eed81a70db92fb6b5c2420d9468fa4446 100644 (file)
@@ -292,17 +292,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;
 }
 
@@ -357,6 +354,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
@@ -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 *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_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);
@@ -1352,12 +1357,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;
 }