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. James -- 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
