On Wed, Aug 11, 2010 at 12:02:57PM -0400, James Bottomley wrote: > On Wed, 2010-08-11 at 08:46 -0700, Greg KH wrote: > > On Wed, Aug 11, 2010 at 07:46:11AM -0400, James Bottomley wrote: > > > On Tue, 2010-08-10 at 15:15 -0700, Greg KH wrote: > > > > On Tue, Aug 10, 2010 at 02:56:47PM -0700, Greg KH wrote: > > > > > On Tue, Aug 10, 2010 at 02:29:20PM -0700, akpm@xxxxxxxxxxxxxxxxxxxx wrote: > > > > > > From: Hans de Goede <hdegoede@xxxxxxxxxx> > > > > > > > > > > > > Some USB devices emulate a usb-mass-storage attached (scsi) cdrom device, > > > > > > usually this fake cdrom contains the windows software for the device. > > > > > > While working on supporting Appotech ax3003 based photoframes, which do > > > > > > this I discovered that they will go of into lala land when ever they see a > > > > > > READ_DISC_INFO scsi command. > > > > > > > > > > > > Thus this patch adds a scsi_device flag (which can then be set by the > > > > > > usb-storage driver through an unsual-devs entry), to indicate this, and > > > > > > makes the sr driver honor this flag. > > > > > > > > > > > > I know this sucks, but as discussed on linux-scsi list there is no other > > > > > > way to make this device work properly. > > > > > > > > > > > > Looking at usb traces made under windows, windows never sends a > > > > > > READ_DISC_INFO during normal interactions with a usb cdrom device. So as > > > > > > this cdrom emulation thingie becomes more common we might see more of this > > > > > > problem. > > > > > > > > > > > > Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx> > > > > > > Cc: James Bottomley <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx> > > > > > > Cc: Greg KH <greg@xxxxxxxxx> > > > > > > Cc: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> > > > > > > Cc: Matthew Dharm <mdharm-usb@xxxxxxxxxxxxxxxxxx> > > > > > > Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> > > > > > > --- > > > > > > > > > > > > drivers/scsi/sr.c | 8 +++++++- > > > > > > include/scsi/scsi_device.h | 1 + > > > > > > > > > > I can't take this one (and the other scsi-only one), unless James acks > > > > > it. If so, I'll be glad to do so. > > > > > > > > > > James? > > > > > > > > Hm, how about the other way around. I'll go queue them up in my tree > > > > tomorrow unless you NACK them, which seems to be a bit easier for > > > > everyone involved given that you are at LinuxCon this week. > > > > > > Well, what's the status from the USB side? Discussion (not entirely > > > surprisingly) when quiet just after the merge window opened. > > > > > > I've already said I think usb-storage should be doing command rejection > > > here not simply trying to reorganise SCSI so the commands the devices > > > don't like don't get issued because that way any user space offender > > > gets fixed too (and SCSI doesn't need patching). > > > > The filtering/rejection issue was discussed, and I agree that the USB > > code shouldn't be the one responsible for doing this type of thing, the > > scsi core shouldn't be sending out those types of messages in the first > > place. > > Well, it already is the usb_stor thread already has a detection and > filtering component. This particular problem isn't that certain SCSI > sequences in init cause an issue, it's that instead of returning ILLEGAL > COMMAND, the device crashes. I can only fix the init layer. If > something higher up (hello hal) sends these commands, the device will > crash anyway. > > > > So my position is that it's the wrong thing to do, but if USB won't do > > > the right thing, I'll reluctantly ack ... have we reached that point? > > > > Heh, what do you think the "right thing" is here, filtering? Shouldn't > > the "right thing" just be to not send these commands like this patch set > > provides? > > So I have to filter in SCSI? That's not really logical (nor what the > patch does). The point is that if the commands get rejected properly in > the first place, all levels of the stack will "just work". If you alter > SCSI not to send the command, fine we don't crash on init, but we're > still vulnerable to another entity sending the command and crashing the > device at a random time. > > The blacklist information is that this device crashes on that command. > Since we don't have to work around it, the best model is for some layer > to intercept the command and reply "I don't support this" on behalf of > the device. Since the layer where the identity information for this is > held is USB, it makes sense that this is just an addition to the USB > storage filters. Ok, I see the argument is valid for both sides. What do others think? Mathew, any thoughts? thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
