Re: usbnet: Recursive Locking bug ?

[Oliver Neukum <oneukum-l3A5Bk7waGM@public.gmane.org>]

Am Dienstag, 7. Dezember 2010, 15:08:43 schrieb Neil Jones:
> I think this is due to unlink_urbs():

Definitely.

> static int unlink_urbs (struct usbnet *dev, struct sk_buff_head *q)
> {
>         unsigned long           flags;
>         struct sk_buff          *skb, *skbnext;
>         int                     count = 0;
> 
>         spin_lock_irqsave (&q->lock, flags);
>         skb_queue_walk_safe(q, skb, skbnext) {
>                 struct skb_data         *entry;
>                 struct urb              *urb;
>                 int                     retval;
> 
>                 entry = (struct skb_data *) skb->cb;
>                 urb = entry->urb;
> 
>                 // during some PM-driven resume scenarios,
>                 // these (async) unlinks complete immediately
>                 retval = usb_unlink_urb (urb);
>                 if (retval != -EINPROGRESS && retval != 0)
>                         netdev_dbg(dev->net, "unlink urb err, %d\n", retval);
>                 else
>                         count++;
>         }
>         spin_unlock_irqrestore (&q->lock, flags);
>         return count;
> }
> 
> I dont think this should hold the list lock when calling usb_unlink_urb (urb).
> The completion for the URBs takes the lock as required in its bottom half:

But what makes sure that the URB unlinked in unlink_urbs() stays a valid pointer?
The bottom half may run and free the URB.

	Regards
		Oliver
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html