diff options
author | Huajun Li <huajun.li.lee@gmail.com> | 2012-05-18 08:12:51 -0400 |
---|---|---|
committer | Greg Kroah-Hartman <gregkh@linuxfoundation.org> | 2012-05-18 19:37:55 -0400 |
commit | 4e09dcf20f7b5358615514c2ec8584b248ab8874 (patch) | |
tree | 52847eb294c7a08a10d27d8bf35844fb8cd0cf8f | |
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>
-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 c4a1af8a954b..e0f107948eba 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) | |||
333 | static struct async *async_getpending(struct dev_state *ps, | 333 | static struct async *async_getpending(struct dev_state *ps, |
334 | void __user *userurb) | 334 | void __user *userurb) |
335 | { | 335 | { |
336 | unsigned long flags; | ||
337 | struct async *as; | 336 | struct async *as; |
338 | 337 | ||
339 | spin_lock_irqsave(&ps->lock, flags); | ||
340 | list_for_each_entry(as, &ps->async_pending, asynclist) | 338 | list_for_each_entry(as, &ps->async_pending, asynclist) |
341 | if (as->userurb == userurb) { | 339 | if (as->userurb == userurb) { |
342 | list_del_init(&as->asynclist); | 340 | list_del_init(&as->asynclist); |
343 | spin_unlock_irqrestore(&ps->lock, flags); | ||
344 | return as; | 341 | return as; |
345 | } | 342 | } |
346 | spin_unlock_irqrestore(&ps->lock, flags); | 343 | |
347 | return NULL; | 344 | return NULL; |
348 | } | 345 | } |
349 | 346 | ||
@@ -398,6 +395,7 @@ static void cancel_bulk_urbs(struct dev_state *ps, unsigned bulk_addr) | |||
398 | __releases(ps->lock) | 395 | __releases(ps->lock) |
399 | __acquires(ps->lock) | 396 | __acquires(ps->lock) |
400 | { | 397 | { |
398 | struct urb *urb; | ||
401 | struct async *as; | 399 | struct async *as; |
402 | 400 | ||
403 | /* Mark all the pending URBs that match bulk_addr, up to but not | 401 | /* Mark all the pending URBs that match bulk_addr, up to but not |
@@ -420,8 +418,11 @@ __acquires(ps->lock) | |||
420 | list_for_each_entry(as, &ps->async_pending, asynclist) { | 418 | list_for_each_entry(as, &ps->async_pending, asynclist) { |
421 | if (as->bulk_status == AS_UNLINK) { | 419 | if (as->bulk_status == AS_UNLINK) { |
422 | as->bulk_status = 0; /* Only once */ | 420 | as->bulk_status = 0; /* Only once */ |
421 | urb = as->urb; | ||
422 | usb_get_urb(urb); | ||
423 | spin_unlock(&ps->lock); /* Allow completions */ | 423 | spin_unlock(&ps->lock); /* Allow completions */ |
424 | usb_unlink_urb(as->urb); | 424 | usb_unlink_urb(urb); |
425 | usb_put_urb(urb); | ||
425 | spin_lock(&ps->lock); | 426 | spin_lock(&ps->lock); |
426 | goto rescan; | 427 | goto rescan; |
427 | } | 428 | } |
@@ -472,6 +473,7 @@ static void async_completed(struct urb *urb) | |||
472 | 473 | ||
473 | static void destroy_async(struct dev_state *ps, struct list_head *list) | 474 | static void destroy_async(struct dev_state *ps, struct list_head *list) |
474 | { | 475 | { |
476 | struct urb *urb; | ||
475 | struct async *as; | 477 | struct async *as; |
476 | unsigned long flags; | 478 | unsigned long flags; |
477 | 479 | ||
@@ -479,10 +481,13 @@ static void destroy_async(struct dev_state *ps, struct list_head *list) | |||
479 | while (!list_empty(list)) { | 481 | while (!list_empty(list)) { |
480 | as = list_entry(list->next, struct async, asynclist); | 482 | as = list_entry(list->next, struct async, asynclist); |
481 | list_del_init(&as->asynclist); | 483 | list_del_init(&as->asynclist); |
484 | urb = as->urb; | ||
485 | usb_get_urb(urb); | ||
482 | 486 | ||
483 | /* drop the spinlock so the completion handler can run */ | 487 | /* drop the spinlock so the completion handler can run */ |
484 | spin_unlock_irqrestore(&ps->lock, flags); | 488 | spin_unlock_irqrestore(&ps->lock, flags); |
485 | usb_kill_urb(as->urb); | 489 | usb_kill_urb(urb); |
490 | usb_put_urb(urb); | ||
486 | spin_lock_irqsave(&ps->lock, flags); | 491 | spin_lock_irqsave(&ps->lock, flags); |
487 | } | 492 | } |
488 | spin_unlock_irqrestore(&ps->lock, flags); | 493 | spin_unlock_irqrestore(&ps->lock, flags); |
@@ -1399,12 +1404,24 @@ static int proc_submiturb(struct dev_state *ps, void __user *arg) | |||
1399 | 1404 | ||
1400 | static int proc_unlinkurb(struct dev_state *ps, void __user *arg) | 1405 | static int proc_unlinkurb(struct dev_state *ps, void __user *arg) |
1401 | { | 1406 | { |
1407 | struct urb *urb; | ||
1402 | struct async *as; | 1408 | struct async *as; |
1409 | unsigned long flags; | ||
1403 | 1410 | ||
1411 | spin_lock_irqsave(&ps->lock, flags); | ||
1404 | as = async_getpending(ps, arg); | 1412 | as = async_getpending(ps, arg); |
1405 | if (!as) | 1413 | if (!as) { |
1414 | spin_unlock_irqrestore(&ps->lock, flags); | ||
1406 | return -EINVAL; | 1415 | return -EINVAL; |
1407 | usb_kill_urb(as->urb); | 1416 | } |
1417 | |||
1418 | urb = as->urb; | ||
1419 | usb_get_urb(urb); | ||
1420 | spin_unlock_irqrestore(&ps->lock, flags); | ||
1421 | |||
1422 | usb_kill_urb(urb); | ||
1423 | usb_put_urb(urb); | ||
1424 | |||
1408 | return 0; | 1425 | return 0; |
1409 | } | 1426 | } |
1410 | 1427 | ||