netdev - Re: usbnet: Recursive Locking bug ?

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Tue, 7 Dec 2010 15:54:37 +0100
From:	Oliver Neukum <oneukum@...e.de>
To:	Neil Jones <neiljay@...il.com>,
	David Brownell <dbrownell@...rs.sourceforge.net>
Cc:	netdev@...r.kernel.org, linux-usb@...r.kernel.org
Subject: Re: usbnet: Recursive Locking bug ?

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 netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html