linux-kernel - Re: [ 38/48] SCSI & usb-storage: add try_rc_10

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: <1341261568.3319.7.camel@dabdike>
Date:	Mon, 2 Jul 2012 20:39:28 +0000
From:	James Bottomley <jbottomley@...allels.com>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
CC:	Hans de Goede <hdegoede@...hat.com>,
	Ben Hutchings <ben@...adent.org.uk>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"stable@...r.kernel.org" <stable@...r.kernel.org>,
	"akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
	"alan@...rguk.ukuu.org.uk" <alan@...rguk.ukuu.org.uk>,
	Alan Stern <stern@...land.harvard.edu>,
	Matthew Dharm <mdharm-usb@...-eyed-alien.net>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	linux-scsi <linux-scsi@...r.kernel.org>
Subject: Re: [ 38/48] SCSI & usb-storage: add try_rc_10_first flag

On Mon, 2012-07-02 at 11:52 -0700, Linus Torvalds wrote:
> On Mon, Jul 2, 2012 at 12:10 AM, Hans de Goede <hdegoede@...hat.com> wrote:
> >
> > So since 3.2 already tries READ_CAPACITY(10) first, this patch is
> > not needed. It does not harm either, and if the scsi level reporting
> > change ever finds its way into 3.2 stable (which it should not
> > IMHO), then this patch will be needed.
> 
> Yeah, it should definitely not make it into stable. In fact, I'd
> rather remove it from mainline too. The whole idea to do
> READ_CAPACITY_16 by default was moronic.
> 
> James? Why does the SCSI subsystem always do these idiotic defaults?
> We *know* that devices are buggy. Sure, USB storage tends to be
> buggier than most, but the whole idea of "let's use the new way of
> reporting that gets almost no testing" is just wrong.

It's not idiotic.  It's, unfortunately, required by the standard to
check for protection capable devices.  It's been this way for three
years or so.  Martin Petersen did extensive testing of devices when we
changed it and doing RC16 first is hedged around by claiming support not
only for SCSI_3 but also for SBC_2 in your INQUIRY data.

The problem seems to be that when all the testing was done, USB was
still always returning SCSI_2 in the inquiry data, but this got changed
to allow pass through of the actual vendor information.

> I don't think the USB layer should have needed to work around this. I
> don't think there should be a "try_10b" flag at all. I think the SCSI
> code should just *never* do the 16b version unless it has done the 10b
> version first and determined that the size doesn't fit.

If we did that, we'd regress on all the enterprise class drives with DIF
and DIX.

> And the code already had that logic got the read_10b case. The whole
> logic to do 16b by default was crap. It *used* to at least only do it
> for disks that were marked as having "protection", now it wants to do
> it for anything that should be able to handle it. Which is bogus. The
> whole "let's assume devices get it right" mindset is broken.
> 
> The mindset should be "let's assume devices are buggy, and be careful".

It still is, that's why we did extensive testing three years ago before
changing it.

If you have a solution that doesn't break all the enterprise devices but
works better for USB, I'm listening.  I think the best thing might be to
whitelist the scsi revision pass through in USB, so only the UAS devices
which really, really claim to know what they're doing advertise advanced
features.

James