On Sat, Jul 29 2006, Linus Torvalds wrote: > > > On Sat, 29 Jul 2006, Jens Axboe wrote: > > > > I'd greatly prefer just ripping the entire command access table out, it > > was a mistake to begin with and still just a horrible solution. > > Well, I don't think the _notion_ of a command access table is the problem > per se. > > I just think that "sg.c" is badly done. > > We do it _correctly_ in the sane paths (ie we really have the "same table" > in "verify_command()" in block/scsi_ioctl.c), and the thing is, when done > correctly, the command access table is a perfectly fine idea. > > And the thing is, that "verify_command()" is absolutely _required_. As > root, you can send any command you want, but we really _do_ want normal > users to be able to send a sane subset, without allowing them to do other > things just because they have write access to the medium. If you feel that the command screening is required, then I'd suggest we start adding some flexibility to it. As Doug pointed out, it's very MMC centric right now. And you have have the problem of wanting to allow some commands for certain devices, some for others, but not have perfect overlap. Vendor specific commands is another problem, some of them are required for functionality on newer Plextor (not making this up or got it from Joerg, it's a fact) drives. We can never add vendor specific commands right now though, as they are device specific. One possible approach is this: http://git.kernel.dk/?p=linux-2.6-block.git;a=commitdiff;h=63bfd0060d0feb5f3ba161c2c1e6e8082fe2f68d exposing the command table to user space through a filter in the queue directory of that request queue. What do you think of that approach? > > In fact, I think we should decide soon what to do about it. At the > > storage summit, there was general consensus on just killing it as well. > > I don't think there's any point to killing it. > > The fact is, anybody who works with a known storage device simply SHOULD > NOT USE THE sg.c INTERFACES. Only totally insane people (ie Jörg > Schilling) think that it's even remotely sane. If you know it's a storage > device, you should use the proper block interfaces, and instad of doing > > cdrecord -dev=ATA:1,0,0 > > the person should really have done > > cdrecord -dev=/dev/hdc > > or something sane, and gone through the proper channels, instead of going > through the nightmare that is Schillings "brain"). > > In fact, I would seriously suggest that distributions should disallow the > insane interfaces to cdrecord. > > "sg.c" makes sense for SCSI laser range fingers and other strange stuff. > It does _not_ make sense for CD-ROM access. > > Btw, it's entirely possible that you might hit the same problem with > /dev/hdc too. But at that point it should be 100% obvious that > > - only root can ever be allowed to generate commands that the kernel has > no clue what they are doing. NO WAY can we allow a user to generate > postentially hardware-changing special commands just because he can > access the CD-ROM (ie how would the kernel know that it's not a command > that says "rewrite the firmware with something that always reads goatse > off the disk"?) > > - if cdrecord tries to do some device-specific setup, and is upset that > it can't do it because the user isn't root, then it's obviously a > cdrecord bug. Which wouldn't surprise me at all - can we _please_ not > rely on code that has been written by a certified nut-case? > > And in either case, I don't think this is a kernel bug, unless it also > happens as root, of course. Forget sg.c vs scsi_ioctl.c, that is a separate issue that bsg should unify in the future. The command table affects them all though, and the fact that we have differing access limitation in scsi_ioctl vs sg right now is a little sad. But a separate issue. -- Jens Axboe - : 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
