Re: [Re: Linux 2.6.26-rc2] Write protect on on

James Bottomley
2008-06-20T22:09:39Z

On Fri, 2008-06-20 at 17:46 -0400, Alan Stern wrote:
> On Fri, 20 Jun 2008, James Bottomley wrote:
>
> > > Not having received any comments on that earlier patch, I wrote a new
> > > version. Actually it's a pair of patches, and they have to be applied
> > > in order. They don't look as ugly as the old one and they have a
> > > decent chance of being accepted.
> >
> > Actually, just looking at this, what you're really trying to do is
> > enforce an underrun detection, which is a concept built in to the
> > command structure but not necessarily well implemented by all drivers.
> >
> > However, I had a tick on this one to go back and look at it.
> >
> > Your initial contention was that the garbage value was "left over data"
> > in the sense command. However, I don't see how we're getting this into
> > the buffer; scsi_mode_sense() clears the buffer up to the length it's
> > expecting before it executes the request. How is this getting garbage
> > data if nothing's returned ... surely it should still be all zeros?
>
> It's not true that nothing's returned. The device returns N bytes of
> garbage data (I forget just now what N was) and sets the residue equal
> to N -- meaning that none of the data was meaningful.
>
> USB mass-storage is perhaps a little strange for people accustomed to
> regular SCSI. Quoting from the relevant spec:
>
> For Data-In the device shall report in the dCSWDataResidue the
> difference between the amount of data expected as stated in the
> dCBWDataTransferLength and the actual amount of relevant data
> sent by the device.
>
> The key word here is "relevant". The device is allowed to send
> non-relevant data and then tell the host to ignore it. Later on the
> spec says:
>
> If the device intends to send less data than the host indicated, then:
> The device shall send the intended data.
> The device may send fill data to pad up to a total of dCBWDataTransferLength.
> If the device actually transfers less data than the host indicated, then:
> The device may end the transfer with a short packet.
> The device shall STALL the Bulk-In pipe.
> The device shall set bCSWStatus to 00h or 01h.
> The device shall set dCSWDataResidue to the difference between dCBWDataTransferLength
> and the actual amount of relevant data sent.
>
> In this case the fill data is getting treated as real data. Does this
> clarify the situation?

Yes, thanks. It's a bit nasty from a security point of view, since the
leaking data apparently belonged to a different command. Wouldn't a
better fix (and a more secure one) be to clear from the end of the valid
data to the end of the buffer?

James