netdev - Re: [PATCH 0/7] USB: 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 22:03:06 +0800
From:	Ming Lei <ming.lei@...onical.com>
To:	Oliver Neukum <oneukum@...e.de>
Cc:	Bjørn Mork <bjorn@...k.no>,
	"David S. Miller" <davem@...emloft.net>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Jiri Kosina <jkosina@...e.cz>,
	Alan Stern <stern@...land.harvard.edu>, netdev@...r.kernel.org,
	linux-usb@...r.kernel.org, linux-input@...r.kernel.org
Subject: Re: [PATCH 0/7] USB: don't recover device if suspend fails in system sleep

On Tue, Mar 5, 2013 at 9:28 PM, Oliver Neukum <oneukum@...e.de> wrote:
> On Tuesday 05 March 2013 21:08:09 Ming Lei wrote:
>> On Tue, Mar 5, 2013 at 8:50 PM, Oliver Neukum <oneukum@...e.de> wrote:
>
>> > In other words, if we don't handle errors, there must be no errors,
>> > otherwise it doesn't matter what we do in the error case. We'd leave
>> > the problem to generic layers.
>>
>> Generic layers can't handle the driver's specific failure.
>
> We depend on stopping the HC forcing all devices into suspend.
> We know this is problematic. For example some disk enclosures
> need to flush cache. Fortunately for us this is done in the SCSI
> layer.

I mean only the driver can know its specific suspend failure
in its suspend callback, so it is reasonable to deal with it in its
resume() callback, suppose the policy to ignore suspend failure
isn't changed.

>
>> If driver records its suspend failure state in suspend(), resume()
>> should and can deal with it without much difficulty.
>
> Yes, but why bother? Either we can safely suspend in any state or
> we must not ignore errors.

In fact, for many drivers, its suspend() in system sleep just return 0,
for example, cdc_wcm/cdc_acm/hub, etc, so these drivers simply
ignore any failures from suspend() during system sleep.

For other drivers, if suspend() don't return success in system sleep,
it shouldn't try to recover the device, and its resume has to handle
the failure. Or do you have better approach? Suppose we keep
ignoring the suspend failure.

At least, I think URBs can't be submitted to device in its system
suspend failure path, which is generally one part of suspend
recovery for many usb drivers. Doing such recovery may confuse
resume() very much, and it is really wrong.

That is the inconsistency of handling system suspend between
usb drivers and usb core.

>
>> > Furthermore there is a small chance that although the device tree
>> > is walked, teh system suspend fails for another later reason that
>> > is not ignored. In that case the drivers need to do error recovery,
>> > albeit in resume().
>>
>> Yes,  resume() need to handle the USB system suspend failure
>> either in normal resume or error recovery, both are basically same.
>
> In theory yes, in practice usually power is cut.

reset_resume() or rebind() can handle the power cut case, for example,
the suspend failure state can be ignored in reset_resume(), or rebind()
just forget previous suspend failure.  Correct me if it is wrong.

Thanks,
--
Ming Lei
--
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