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 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
