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]

Message-ID: <87fw0aqqel.fsf@nemi.mork.no>
Date:	Tue, 05 Mar 2013 08:03:46 +0100
From:	Bjørn Mork <bjorn@...k.no>
To:	Ming Lei <ming.lei@...onical.com>
Cc:	"David S. Miller" <davem@...emloft.net>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Jiri Kosina <jkosina@...e.cz>,
	Alan Stern <stern@...land.harvard.edu>,
	Oliver Neukum <oneukum@...e.de>, 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

Ming Lei <ming.lei@...onical.com> writes:

> Hi,
>
> This patch adds comments on interface driver suspend callback
> to emphasize that the failure return value is ignored by
> USB core in system sleep context, so do not try to recover
> device for this case, otherwise the URB traffic scheduled
> in recovery of failure path may cross system sleep, and may
> cause problems.

Well, an unexpected error did happen so problems are to be expected,
yes.

> Also fixes the USB serial, HID and several usbnet drivers
> which may recover device in suspend failure path of system sleep.

I believe all of these are wrong unless you have any real bug which is
fixed by this.

All these drivers suspend in multiple steps, where each step can
fail. If a later step fails then they revert any previously successful
step before returning the failure, thereby ensuring that the
device/driver state when suspend returns is consistently either
suspended or resumed.

The error recovery they do in suspend is not about preventing suspend at
all.  It is about ensuring that that the driver and device is in a
consistent state, which is "resumed" if suspend fails.

Your patch set make the drivers return from suspend in some intermediate
state, where the device and/or driver is neither suspended nor resumed.
This is wrong.  You still did not necessarily kill all URBs, but you
killed some of them.  What is resume() going to do then?

I am going to NAK the cdc_mbim and qmi_wwan pacthes unless you can
convince me that we need to add a "partly-suspended" state for the
system suspend error case.  In which case the patch will need to include
the corresponding resume fix for the "partly-suspended" state.

Bjørn
--
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