netdev - Re: [PATCH 4/7] usbnet: cdc_mbim: don't recover device if suspend fails in system sleep

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]

Date:	Tue, 5 Mar 2013 11:54:21 -0500 (EST)
From:	Alan Stern <stern@...land.harvard.edu>
To:	Bjørn Mork <bjorn@...k.no>
cc:	Ming Lei <ming.lei@...onical.com>,
	"David S. Miller" <davem@...emloft.net>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Jiri Kosina <jkosina@...e.cz>, Oliver Neukum <oneukum@...e.de>,
	<netdev@...r.kernel.org>, <linux-usb@...r.kernel.org>,
	<linux-input@...r.kernel.org>
Subject: Re: [PATCH 4/7] usbnet: cdc_mbim: don't recover device if suspend
 fails in system sleep

On Tue, 5 Mar 2013, Bjørn Mork wrote:

> Ming Lei <ming.lei@...onical.com> writes:
> 
> > Yes, USB core will flush any outstanding URBs, but the driver still need
> > to deal with suspend failure carefully, for example, suppose usb_resume()
> > is called in suspend failure path, and the submitted URBs are killed
> > by USB core later. But after the device is wakeup, and the resume() will
> > do nothing since the suspend count is leaked. So it is what the patches
> > are fixing, and it is better to not depend on the default flushing URBs of
> > USB core.
> 
> I am starting to wonder why the USB core has combined system suspend and
> runtime suspend if we are going to end up with every driver testing
> PMSG_IS_AUTO(message) and selecting a completely different code path.

Mainly for historical reasons.  System suspend existed long before 
runtime suspend did.  When runtime suspend was added, it piggybacked 
off the existing code.  Furthermore, originally there was no 
requirement that system suspend always succeed; that was added later.

Also, the code paths are not completely different.  They differ mainly 
in their error handling.  But when you think about it, how serious an 
error can you encounter when you try to _stop_ using a device?

> You are right that we will end up with problems if usbnet_resume is
> called for a device usbnet hasn't suspended.  But I'd still claim that
> is a bug in the USB core, which is the one that decided to ignore the
> suspend error and still call resume.
> 
> I guess proper error handling here require the USB core to see the
> interface driver as dead if it fails to suspend on system suspend, and
> do forced rebinding on resume.

You are welcome to submit a patch to do this.  It shouldn't be hard; we
already have a flag indicating that an interface needs to be unbound at
reprobed at resume time.  You can update the kerneldoc in addition; as
you noticed, it currently does not describe the actual code completely
accurately.

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