linux-kernel - Re: [PATCH] scsi/sd: fix suspend with USB-connected Android phone (one line)

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 12 May 2011 16:59:24 -0400
From:	Charles Hannum <root@...ck.net>
To:	James Bottomley <James.Bottomley@...senpartnership.com>
Cc:	"Rafael J. Wysocki" <rjw@...k.pl>, linux-kernel@...r.kernel.org,
	Greg Kroah-Hartman <gregkh@...e.de>,
	Alan Stern <stern@...land.harvard.edu>,
	linux-scsi <linux-scsi@...r.kernel.org>,
	linux-usb@...r.kernel.org
Subject: Re: [PATCH] scsi/sd: fix suspend with USB-connected Android phone
 (one line)

On Thu, May 12, 2011 at 16:36, James Bottomley
<James.Bottomley@...senpartnership.com> wrote:
> On Thu, 2011-05-12 at 22:03 +0200, Rafael J. Wysocki wrote:
>> Hi,
>>
>> Added some CCs.
>>
>> On Thursday, May 12, 2011, Charles Hannum wrote:
>> > Short version: My laptop doesn't suspend when my Android phone is
>> > connected and has been “ejected”.
>> >
>> > Long version:
>> >
>> > Android phones connect as USB mass storage devices.  After the “Turn
>> > on USB storage” button has been clicked, there are a few different
>> > ways to detach the “disk”:
>> >
>> > 1) pull the cable
>> > 2) click “Turn off USB storage”
>> > 3) “eject” the device
>> >
>> > In cases 2 & 3, the USB device is still attached to the system, but
>> > will now return MEDIUM NOT PRESENT for many commands, including
>> > SYNCHRONIZE CACHE—basically it acts like any device with removable
>> > media.  However, the act of the “media” being removed does not
>> > invalidate sdkp->WCE; therefore sd_shutdown() and sd_suspend() still
>> > call sd_sync_cache(), which *fails* because it gets a MEDIUM NOT
>> > PRESENT sense code.  In the sd_suspend() case, this causes the entire
>> > suspend to fail, and the laptop rewakes immediately.
>
> So this was the subject of some debate when suspend of sd was first
> introduced.  The synopsis of that debate is that by suspending, we're
> about the power down the system, and anything in the cache will be lost,
> possibly causing corruption, so failure to synchronise the cache
> *should* abort suspend.

If the device has been “ejected”, there is by definition no data in
the cache that needs to be synchronized—or, if there is, it's because
we failed to do a SYNCHRONIZE CACHE in the eject path.  At this point
the device believes there's no medium present, and the driver even
agrees, so it's perfectly reasonable for the device to return MEDIUM
NOT PRESENT.  The problem here is a generic failure of the suspend
code in the case of removable media that has been removed—there is a
piece of state left behind in the driver that causes it to misbehave.

Nothing I have mentioned (other than enumerating the cable-pulling
case), nor the change I proposed, has anything whatsoever to do with
the user inappropriately yanking an active device.

>> > There are a few different ways to fix this; e.g. one could
>> > specifically test media_not_present() if a sense code is returned in
>> > sd_sync_cache().
>
> Isn't this the best approach?  For removable medium, the onus is on the
> user not to eject with the cache unsynced.  If the user ignores that, we
> should recognise the problem and take a caveat emptor approach.

I was attempting to make the case where the user behaved completely
appropriately work right, not worrying about the other cases.  It
seems a bit silly to even call sd_sync_cache() if we already know the
medium has been removed; furthermore, it means the behavior is
different between when the device is first plugged in (WCE is still 0)
and when it's been used and then ejected (WCE remains 1 until a new
medium is inserted); this inconsistency seems poor.

> As a side note: having write through caches on removable media is a
> really silly idea because of the above problem ...

In the Android case, I see no reason why it's a problem, and the
device acts appropriately under the circumstances.  And given tray
locking, I'm not sure why it's a problem even for spinning media.  But
regardless, these are real devices, and that ship has long since
sailed.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/