netdev - Re: [PATCHv4] usbnet: Resubmit interrupt URB once if halted

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.1104210937360.1939-100000@iolanthe.rowland.org>
Date:	Thu, 21 Apr 2011 09:43:46 -0400 (EDT)
From:	Alan Stern <stern@...land.harvard.edu>
To:	Paul Stewart <pstew@...omium.org>
cc:	netdev@...r.kernel.org, <linux-usb@...r.kernel.org>,
	<davem@...emloft.net>, <greg@...ah.com>
Subject: Re: [PATCHv4] usbnet: Resubmit interrupt URB once if halted

On Wed, 20 Apr 2011, Paul Stewart wrote:

> On Wed, Apr 20, 2011 at 2:08 PM, Alan Stern <stern@...land.harvard.edu> wrote:
> > On Tue, 19 Apr 2011, Paul Stewart wrote:
> >
> >> Set a flag if the interrupt URB completes with ENOENT as this
> >> occurs legitimately during system suspend.  When the usbnet_bh
> >> is called after resume, test this flag and try once to resubmit
> >> the interrupt URB.
> >
> > No doubt there's a good reason for doing things this way, but it isn't
> > clear.  Why wait until usbnet_bh() is called after resume?  Why not
> > resubmit the interrupt URB _during_ usbnet_resume()?
> 
> Actually, I was doing this in the bh because of feedback I had gained
> early in this process about not doing submit_urb in the resume().

Do you have a URL for that feedback?  In general, there's no reason not 
to resubmit URBs during a resume callback; lots of drivers do it.  But 
usbnet may have some special requirements of its own that I'm not aware 
of.

>  If
> that issue doesn't exist, that makes my work a lot easier.  In testing
> I found that just setting this to happen in the bh might be problematic
> due to firing too early, so this is good news.
> 
> > This would seem
> > to be the logical approach, seeing as how usbnet_suspend() kills the
> > interrupt URB.
> 
> Aha!  But you'll see from the current version of my patch that we don't
> actually ever kill the interrupt URB.  It gets killed all on its own (by the
> hcd?) and handed back to us in intr_complete().  This last bit about the
> complete function being called was lost on me for a while which is why
> in a previous iteration of the patch I was trying to kill the urb in suspend().

Why not kill the interrupt URB while suspending?  It's the proper thing
to do.  Otherwise you run the risk that an event might happen at just
the wrong time, causing the interrupt URB to complete normally, but
_after_ the driver has finished suspending.  There's a good chance the 
driver would not process the event correctly.

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