netdev - Re: [patch] usbnet: fix 100% CPU use on suspended device

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <Pine.LNX.4.44L0.1007261033090.1550-100000@iolanthe.rowland.org>
Date:	Mon, 26 Jul 2010 10:36:46 -0400 (EDT)
From:	Alan Stern <stern@...land.harvard.edu>
To:	Elly Jones <ellyjones@...gle.com>
cc:	David Miller <davem@...emloft.net>, <netdev@...r.kernel.org>,
	<linux-usb@...r.kernel.org>
Subject: Re: [patch] usbnet: fix 100% CPU use on suspended device

On Mon, 26 Jul 2010, Elly Jones wrote:

> On Mon, Jul 26, 2010 at 12:57 AM, David Miller <davem@...emloft.net> wrote:
> > From: Elly Jones <ellyjones@...gle.com>
> > Date: Wed, 21 Jul 2010 14:51:48 -0400
> >
> >> Subject: [patch] usbnet: fix 100% CPU use on suspended device
> >> From: Elly Jones <ellyjones@...gle.com>
> >>
> >> This patch causes the usbnet module not to attempt to submit URBs to the device
> >> if the device is not ready to accept them. This fixes a misbehavior trigged by
> >> the Qualcomm Gobi driver (released under GPL through their Code Aurora
> >> initiative) which causes the usbnet core to consume 100% of CPU attempting and
> >> failing to submit URBs. This patch is against Linus's 2.6 repo commit
> >> a9f7f2e74ae0e6a801a2433dc8e9124d73da0cb4.
> >> Signed-off-by: Elizabeth Jones <ellyjones@...gle.com>
> >
> > If the Qualcomm Gobi driver is the only one where this happens, maybe the
> > real problem is in that driver.
> 
> The member in question (dev->udev->can_submit) is documented as 'URBs
> may be submitted'. The existing code doesn't honor that flag. It is
> somewhat puzzling that (so far) only the Gobi driver seems to use that
> flag, but I don't think the bug lies in their driver here.

That flag is not intended for use by drivers but by the USB core.  (It
gets cleared only when the device is suspended.)  usbnet and Gobi
shouldn't need to look at it.

> > I'm not applying this until a USB person looks more deeply into this.
> 
> Alright. Can you suggest a particular USB person to bother?
> 
> >> ---
> >> diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
> >> index 81c76ad..df7e72e 100644
> >> --- a/drivers/net/usb/usbnet.c
> >> +++ b/drivers/net/usb/usbnet.c
> >> @@ -1172,6 +1172,7 @@ static void usbnet_bh (unsigned long param)
> >>       // or are we maybe short a few urbs?
> >>       } else if (netif_running (dev->net) &&
> >>                  netif_device_present (dev->net) &&
> >> +                dev->udev->can_submit &&
> >>                  !timer_pending (&dev->delay) &&
> >>                  !test_bit (EVENT_RX_HALT, &dev->flags)) {
> >>               int     temp = dev->rxq.qlen;

This isn't right.  The problem should be fixed some other way.  Under
what circumstances are URBs submitted incorrectly?

Alan Stern

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