cd burning with plextor drives.

cd burning with plextor drives.

[Dave Jones]

AFAICT, this has been broken since the code to sanitise scsi commands
was added circa 2.6.8 or so.

It seems cdrecord tries to send some vendor specific commands to Plextor
drives, and fails miserably as can be seen here..
https://bugzilla.redhat.com/bugzilla/attachment.cgi?id=124267

I'm not that familiar with this code, but would adding exceptions
on a per-vendor basis in sg_allow_access() be the way forward here?

If not, what is the right answer ?

		Dave

-- 
http://www.codemonkey.org.uk

Re: cd burning with plextor drives.

[Jens Axboe]

On Sat, Jul 29 2006, Dave Jones wrote:
> AFAICT, this has been broken since the code to sanitise scsi commands
> was added circa 2.6.8 or so.
> 
> It seems cdrecord tries to send some vendor specific commands to Plextor
> drives, and fails miserably as can be seen here..
> https://bugzilla.redhat.com/bugzilla/attachment.cgi?id=124267
> 
> I'm not that familiar with this code, but would adding exceptions
> on a per-vendor basis in sg_allow_access() be the way forward here?
> 
> If not, what is the right answer ?

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.

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.

-- 
Jens Axboe

Re: cd burning with plextor drives.

[James Bottomley]

On Sat, 2006-07-29 at 13:12 +0200, Jens Axboe wrote:
> > I'm not that familiar with this code, but would adding exceptions
> > on a per-vendor basis in sg_allow_access() be the way forward here?
> > 
> > If not, what is the right answer ?
> 
> 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.
> 
> 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 concur.  If we're going to allow users access to burn CDs, it's
impossible to police them with certainty as this case indicates.  If we
allow vendor specific commands down, there are bound to be some that
format the drive or destroy the firmware ...

So I think ripping the table out and acknowledging we have no security
is better than giving the illusion of having it.

James

Re: cd burning with plextor drives.

[Christer Weinigel]

James Bottomley <James.Bottomley@SteelEye.com> writes:

> On Sat, 2006-07-29 at 13:12 +0200, Jens Axboe wrote:
> > > I'm not that familiar with this code, but would adding exceptions
> > > on a per-vendor basis in sg_allow_access() be the way forward here?
> > > 
> > > If not, what is the right answer ?
> > 
> > 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.
> > 
> > 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 concur.  If we're going to allow users access to burn CDs, it's
> impossible to police them with certainty as this case indicates.  If we
> allow vendor specific commands down, there are bound to be some that
> format the drive or destroy the firmware ...
> 
> So I think ripping the table out and acknowledging we have no security
> is better than giving the illusion of having it.

How about making cmd_type a per device variable and adding an ioctl to
set cmd_type?  Let cmd_type default to letting everything through.
That way a distribution can add filters if it wants to.

 /Christer

-- 
"Just how much can I get away with and still go to heaven?"

Freelance consultant specializing in device driver programming for Linux 
Christer Weinigel <christer@weinigel.se>  http://www.weinigel.se

Re: cd burning with plextor drives.

[Linus Torvalds]

On Sat, 29 Jul 2006, James Bottomley wrote:
> 
> I concur.  If we're going to allow users access to burn CDs, it's
> impossible to police them with certainty as this case indicates.

Not so. I can (and have) written tons of CD's as a normal user, with 
perfect security.

No, the kernel shouldn't allow device-specific commands. That goes without 
saying. Whether this is a sg.c problem, or a cdrecord problem is unclear, 
I suspect it's the latter. 

		Linus

Re: cd burning with plextor drives.

[James Bottomley]

On Sat, 2006-07-29 at 10:06 -0700, Linus Torvalds wrote:
> Not so. I can (and have) written tons of CD's as a normal user, with 
> perfect security.

But not for every CD burner ... that's the point.  The heuristics we
have now work for a large subset.  For the rest, we get a stream of "my
CD won't burn as a user like it's supposed to" bug reports.

In the old days, the gnome/kde stuff simply gave the user ownership of
the cd device and we allowed any command through and so all CDs worked,
if not very safely.  Suddenly with the new "are you root, if not I
consult my allowed tables" method we get this list of CDs that can't
burn as a user.

> No, the kernel shouldn't allow device-specific commands. That goes without 
> saying. Whether this is a sg.c problem, or a cdrecord problem is
> unclear, 
> I suspect it's the latter. 

There are certain CDs that just require vendor specific magic to
work ... even cdrecord has no choice but to do this.

In general, this allowed command list is solidifying policy in the
kernel, which is the problem.  If we merely put the ability to enforce
policy in the kernel (without actually having any by default and allow
the distros to set it via sysfs as we do now for the SCSI blacklist)
then we'll go back to the old days (of every CD just works) and if
there's a problem it will be a distro issue because they got their
policy wrong (they're the ones who can scan a computer, see a plextor
on /dev/sdc and allow certain vendor specific commands to /dev/sdc only)

James

Re: cd burning with plextor drives.

[Linus Torvalds]

On Sat, 29 Jul 2006, James Bottomley wrote:
> 
> But not for every CD burner ... that's the point.  The heuristics we
> have now work for a large subset.  For the rest, we get a stream of "my
> CD won't burn as a user like it's supposed to" bug reports.

That's not _our_ problem. That's likely the problem of "cdrecord" 
_thinking_ that it needs to do something smart, when it really really 
doesn't need to.

I have yet to hear anything that implies anything else than "cdrecord is 
just buggy".

> There are certain CDs that just require vendor specific magic to
> work ... even cdrecord has no choice but to do this.

IF that is actually true (and if you were told this by Joerg, I would 
double- and triple-check it from some other source), then in the end you 
do end up having to have a per-device translation table, and it probably 
shouldn't be in the kernel.

It probably _should_ be in a suid-root thing that validates the commands. 
People used to make cdrecord suid, but that thing really isn't written to 
be done that way. That doesn't mean that something else couldn't be done 
properly.

There's no way the kernel should do this. In a hotplug environment in 
particular (which is not _that_ common with CD-ROM's but certainly not 
unheard of), you don't want to have the kernel know about every quirk of 
every device anyway.

			Linus

Re: cd burning with plextor drives.

[Christer Weinigel]

Linus Torvalds <torvalds@osdl.org> writes:

> On Sat, 29 Jul 2006, James Bottomley wrote:
> > There are certain CDs that just require vendor specific magic to
> > work ... even cdrecord has no choice but to do this.
> 
> IF that is actually true (and if you were told this by Joerg, I would 
> double- and triple-check it from some other source), then in the end you 
> do end up having to have a per-device translation table, and it probably 
> shouldn't be in the kernel.

Joerg is correct about this.  I just went through the cdrecord sources
(from Fedora Core 5 with a DVD patch) and took a quick look at what
commands and mode pages were used by the different files.  I've
probably missed a few things, but this ought to give you and idea
about what is needed to burn a CD:

drv_philips.c -- Philips/Yamaha/Ricoh/Plasmon

    Commands:

    0xE2 -- Philips First Writeable Address
    0xE4 -- Philips Reserve Track
    0xE6 -- Philips Write Track
    0xE7 -- Philips Medium Load/Unload
    0xE9 -- Philips Fixation
    0xEC -- Philips Recover?
    0xEE -- Philips Read Session Info

    Mode pages:

    0x21 -- Philips Write Track Info?
    0x23 -- Philips speed select
    0x31 -- Yamaha speed select

scsi_mmc.c - MMC-3

    Commands:

    0x46 -- MMC Get Configuration

drv_7501.c -- Masushita CW-7501

    Commands:

    0x01 - Rezero Unit
    0xE2 -- Set Mode?
    0xE3 -- Fixate/Finalize
    0xE6 -- Write DAO
    0xE7 -- Reserve Track
    0xE9 -- Read Trackinfo

    Mode pages:

    0x20 -- Speed control
    0x21 -- MCN?
    0x22 -- ISRC
    0x23 -- Dummy / Write information
    0x24 -- CD-R Disk Information

scsi_cdr.c -- pre-MMC standard functions up to MMC-2

    Commands:
    
    0x00 -- Test Unit Ready
    0x01 -- Rezero Unit
    0x03 -- Request Sense
    0x04 -- Format Unit
    0x08 -- Read 6
    0x0B -- Seek 6
    0x0D -- Qic02 Sysgen SC4000?
    0x0a -- Write 6
    0x12 -- Inquiry
    0x15 -- Mode Select 10
    0x1B -- Start/Stop Unit
    0x1E -- Prevent/Allow Medium Removal
    0x1a -- Mode Sense 10
    0x25 -- Read Capacity
    0x28 -- Read 10
    0x2B -- Seek 10
    0x2a -- Write 10
    0x35 -- Flush Cache
    0x3B -- Write Buffer
    0x3C -- Read Buffer
    0x42 -- Read Subchannel
    0x43 -- Read TOC
    0x44 -- Read Header
    0x51 -- Read Disk Info
    0x52 -- Read Track Info
    0x53 -- Reserve Track
    0x54 -- Send OPC
    0x55 -- Mode Select 10
    0x59 -- Read Master Cue Sheet
    0x5A -- Mode Sense 10
    0x5B -- Close Track/Session
    0x5C -- Read Buffer Cap(acity?)
    0x5D -- Send Cue Sheet
    0xA1 -- Blank Unit
    0xA6 -- Medium Load/Unload
    0xAA -- Write XG5?
    0xAD -- Read DVD Structure
    0xB6 -- Set Streaming?
    0xBB -- Set CD Speed
    0xBF -- Send DVD Structure
    0xE5 -- Read Track Info Philips

    Except for 0x0D and the last bunch these commands are probably pretty
    generic for all CDROM drives.

drv_jvc.c -- JVR/TEAC

    Commands:

    0x2A -- Write 10
    0xB3 -- Set Limits
    0xC4 -- Read PMA
    0xC7 -- Read Disk Info
    0xE0 -- Buffer Inquiry
    0xE1 -- Write PMA
    0xE3 -- Freeze
    0xE4 -- Clear Subcode
    0xE6 -- Next Write Address
    0xEC -- Optimize Power
    0xEF -- Read Peak Buffer Capacity

    Commands 0xC1, 0xC3, 0xC6, 0xCE, 0xCF and 0xDF are also in the
    code but are never used.

    Mode Pages:

    0x00 -- Select Sector Size
    0x21 -- Dummy Selection
    0x31 -- Speed Selection

drv_mmc.c -- SCSI-3/mmc conforming drives

    Commands:

    0x3B -- Yamaha Write Buffer With Parameter
    0xBB -- Yamaha Force Speed
    0xE9 -- Plextor Drive Mode
    0xEB -- Plextor Get Speed List
    0xED -- Plextor Drive Mode 2
    0xF5 -- Plextor Read BPC

    Mode Pages:

    0x05 -- CD/DVD Write Parameters, Speed Select
    0x2A -- MMC mode page
    0x30 -- Ricoh Mode Page (burn-free and read speed control)
    0x31 -- Yamaha Tattoo Page

drv_sony.c -- Sony

    Commands:

    0xE0 -- Write Start
    0xE1 -- Write Continue
    0xE2 -- Discontinue
    0xEC -- Read Buffer Capacity
    0xF0 -- Close Track
    0xF1 -- Finalize
    0xF2 -- Flush
    0xF3 -- Reserve Track
    0xF5 -- Write Track
    0xF6 -- Recover
    0xF8 -- Set Write Parameter 

    Mode Pages:

    0x20 -- Mastering Information
    0x22 -- Disk Information
    0x23 -- Track Information
    0x31 -- Drive Speed

 /Christer

-- 
"Just how much can I get away with and still go to heaven?"

Freelance consultant specializing in device driver programming for Linux 
Christer Weinigel <christer@weinigel.se>  http://www.weinigel.se

Re: cd burning with plextor drives.

[Linus Torvalds]

On Sun, 30 Jul 2006, Christer Weinigel wrote:
> 
> Joerg is correct about this.  I just went through the cdrecord sources
> (from Fedora Core 5 with a DVD patch) and took a quick look at what
> commands and mode pages were used by the different files.  I've
> probably missed a few things, but this ought to give you and idea
> about what is needed to burn a CD:

Umm. I suspect you are either looking at the really old drives (like over 
a decade ago), when there were indeed lots of CD-burners that had very 
specific commands because the standard was new or nonexistent.

Or you're looking at some extended commands that cdrecord _knows_ about 
and can use, but that aren't necessarily standard or supported on all 
CD-ROM drives.

I really _would_ be pretty surprised if you can't do standard CD-ROM (and 
DVD) burning with the standard MMC commands with just about _any_ drive 
that is more recent than ten years old. 

One reason? I don't think Windows supports those out of the box either.

		Linus

Re: cd burning with plextor drives.

[Christian Iversen]

On Sunday 30 July 2006 21:57, Linus Torvalds wrote:
> On Sun, 30 Jul 2006, Christer Weinigel wrote:
> > Joerg is correct about this.  I just went through the cdrecord sources
> > (from Fedora Core 5 with a DVD patch) and took a quick look at what
> > commands and mode pages were used by the different files.  I've
> > probably missed a few things, but this ought to give you and idea
> > about what is needed to burn a CD:
>
> Umm. I suspect you are either looking at the really old drives (like over
> a decade ago), when there were indeed lots of CD-burners that had very
> specific commands because the standard was new or nonexistent.
>
> Or you're looking at some extended commands that cdrecord _knows_ about
> and can use, but that aren't necessarily standard or supported on all
> CD-ROM drives.
>
> I really _would_ be pretty surprised if you can't do standard CD-ROM (and
> DVD) burning with the standard MMC commands with just about _any_ drive
> that is more recent than ten years old.

That was my impression as well - perhaps this could be solved by making the 
MMC-commands valid for any CD-writer device, and making LightScribe, 
BurnProff, etc available only to root? or, by a special vendor->permission 
table like suggested previously? That would change the bug reports to 
simply "my drive can't do BurnProof unless I'm root", instead of "my drive 
can't burn unless I'm root". That's clearly more manageable, right?

-- 
Regards,
Christian Iversen

Re: cd burning with plextor drives.

[Christer Weinigel]

Linus Torvalds <torvalds@osdl.org> writes:

> On Sun, 30 Jul 2006, Christer Weinigel wrote:
> > 
> > Joerg is correct about this.  I just went through the cdrecord sources
> > (from Fedora Core 5 with a DVD patch) and took a quick look at what
> > commands and mode pages were used by the different files.  I've
> > probably missed a few things, but this ought to give you and idea
> > about what is needed to burn a CD:
> 
> Umm. I suspect you are either looking at the really old drives (like over 
> a decade ago), when there were indeed lots of CD-burners that had very 
> specific commands because the standard was new or nonexistent.
>
> Or you're looking at some extended commands that cdrecord _knows_ about 
> and can use, but that aren't necessarily standard or supported on all 
> CD-ROM drives.

Yes, a lot of those CD burners are ancient.  But I like the fact that
Linux supports a lot of ancient hardware. :-)

IMHO, those extenend commands shoule be available to unprivileged
userspace, because setting the burn speed is a rather basic operation.
A lot of people who do backups to CDROM burn CDs at 4x even if the
burner supports 24x, because CDs burned at lower speeds are often more
realiable than CDs burned at the higher speeds.

Still, cdrecord could warn and say "I couldn't set the drive speed,
but do you want me to continue anyway" instead of failing
unconditionally.

What I think we ought to to is to follow Christian Iversen's
suggestion and default to allowing the MMC command set for CD-players.
Then add some kind of syscall/sysctl to modify the list of allowed
commands such as the patch Peter Jones posted.  Did you look at it?

    http://www.uwsg.iu.edu/hypermail/linux/kernel/0409.1/2109.html

I've forward ported and compile tested Peter Jones' patch to 2.6.17.7,
if you want to try it out.  No guarantees that it works though.

I also think that the filer has to be extended to filter MODE_SELECT
/MODE_SELECT_10 too, since the burn-proof/speed settings usually are
selected via vendor mode pages (although not on the Plextors which
started this thread) but there are some other mode pages that should
definitely not be user accessible.  If you think Peter Jones' patch is
acceptable in principle, it could be extended to support mode pages
too.

Another extension may be to add a bitmap for "used commands/mode
pages" that records what commands have been used by an application.
That way a distribution maker could have a simple script that enables
all commands, runs the offending program, and then looks at the
commands used and creates a rc-script that enables those commands (and
preferably also gives the user the option to mail the program name,
list of commands and /proc/ide/hdc/identify to the distribution
maker).

  /Christer

No signed-off-by line because this is Peter Jones work originally,
I've just done a trivial forward port.  

Index: linux-2.6.17.7/block/Makefile
===================================================================
--- linux-2.6.17.7.orig/block/Makefile
+++ linux-2.6.17.7/block/Makefile
@@ -2,7 +2,7 @@
 # Makefile for the kernel block layer
 #
 
-obj-y	:= elevator.o ll_rw_blk.o ioctl.o genhd.o scsi_ioctl.o
+obj-y	:= elevator.o ll_rw_blk.o ioctl.o genhd.o scsi_ioctl.o rcf.o
 
 obj-$(CONFIG_IOSCHED_NOOP)	+= noop-iosched.o
 obj-$(CONFIG_IOSCHED_AS)	+= as-iosched.o
Index: linux-2.6.17.7/block/genhd.c
===================================================================
--- linux-2.6.17.7.orig/block/genhd.c
+++ linux-2.6.17.7/block/genhd.c
@@ -187,6 +187,7 @@ void add_disk(struct gendisk *disk)
 			    disk->minors, NULL, exact_match, exact_lock, disk);
 	register_disk(disk);
 	blk_register_queue(disk);
+	blk_register_filter(disk);
 }
 
 EXPORT_SYMBOL(add_disk);
@@ -194,6 +195,7 @@ EXPORT_SYMBOL(del_gendisk);	/* in partit
 
 void unlink_gendisk(struct gendisk *disk)
 {
+	blk_unregister_filter(disk);
 	blk_unregister_queue(disk);
 	blk_unregister_region(MKDEV(disk->major, disk->first_minor),
 			      disk->minors);
@@ -389,6 +391,12 @@ static ssize_t disk_stats_read(struct ge
 		jiffies_to_msecs(disk_stat_read(disk, io_ticks)),
 		jiffies_to_msecs(disk_stat_read(disk, time_in_queue)));
 }
+
+static ssize_t disk_driver_read(struct gendisk * disk, char *page)
+{
+	return sprintf(page, "%s\n", disk->disk_name);
+}
+
 static struct disk_attribute disk_attr_uevent = {
 	.attr = {.name = "uevent", .mode = S_IWUSR },
 	.store	= disk_uevent_store
@@ -413,6 +421,10 @@ static struct disk_attribute disk_attr_s
 	.attr = {.name = "stat", .mode = S_IRUGO },
 	.show	= disk_stats_read
 };
+static struct disk_attribute disk_attr_driver = {
+	.attr = {.name = "driver", .mode = S_IRUGO },
+	.show	= disk_driver_read
+};
 
 static struct attribute * default_attrs[] = {
 	&disk_attr_uevent.attr,
@@ -421,6 +433,7 @@ static struct attribute * default_attrs[
 	&disk_attr_removable.attr,
 	&disk_attr_size.attr,
 	&disk_attr_stat.attr,
+	&disk_attr_driver.attr,
 	NULL,
 };
 
Index: linux-2.6.17.7/block/rcf.c
===================================================================
--- /dev/null
+++ linux-2.6.17.7/block/rcf.c
@@ -0,0 +1,322 @@
+/*
+ * Copyright 2004 Peter M. Jones <pjones@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public Licens
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-
+ *
+ */
+
+#include <linux/list.h>
+#include <linux/genhd.h>
+#include <linux/spinlock.h>
+#include <linux/parser.h>
+#include <asm/bitops.h>
+
+#include <scsi/scsi.h>
+#include <linux/cdrom.h>
+
+int rcf_verify_command(struct file *file, unsigned char *cmd)
+{
+	struct rawio_cmd_filter *rcf;
+	struct inode *inode;
+
+	inode = file->f_dentry->d_inode;
+	if (!inode)
+		return -EINVAL;
+
+	rcf = &inode->i_bdev->bd_disk->filter;
+
+	/* root can do any command. */
+	if (capable(CAP_SYS_RAWIO))
+		return 0;
+
+	/* if there's no filter allocated, bail out. */
+	if (unlikely(!rcf))
+		return -EPERM;
+
+	/* Anybody who can open the device can do a read-safe command */
+	if (test_bit(cmd[0], rcf->read_ok))
+		return 0;
+
+	/* Write-safe commands require a writable open */
+	if (test_bit(cmd[0], rcf->write_ok))
+		if (file->f_mode & FMODE_WRITE)
+			return 0;
+
+	/* Or else the user has no perms */
+	return -EPERM;
+}
+
+EXPORT_SYMBOL(rcf_verify_command);
+
+/* and now, the sysfs stuff */
+static ssize_t rcf_readcmds_show(struct rawio_cmd_filter *rcf, char *page)
+{
+	char *npage = page;
+	int x;
+	
+	for (x=0; x < RCF_MAX_NR_CMDS; x++)
+		if (test_bit(x, rcf->read_ok)) {
+			sprintf(npage, "%02x", x);
+			npage += 2;
+			if (x < RCF_MAX_NR_CMDS-1)
+				sprintf(npage++, " ");
+		}
+	if (npage != page)
+		npage += sprintf(npage, "\n");
+	
+	return (npage - page);
+}
+
+static ssize_t rcf_writecmds_show(struct rawio_cmd_filter *rcf, char *page)
+{
+	char *npage = page;
+	int x;
+	
+	for (x=0; x < RCF_MAX_NR_CMDS; x++)
+		if (test_bit(x, rcf->write_ok)) {
+			sprintf(npage, "%02x", x);
+			npage += 2;
+			if (x < RCF_MAX_NR_CMDS-1)
+				sprintf(npage++, " ");
+		}
+	if (npage != page)
+		npage += sprintf(npage, "\n");
+	
+	return (npage - page);
+}
+
+#define RCF_ADD	0
+#define RCF_DEL	1
+
+static ssize_t rcf_store(struct rawio_cmd_filter *rcf, const char *page,
+			size_t count)
+{
+	ssize_t ret=0;
+	int ad,rw;
+	int cmd, status;
+
+	substring_t ss;
+
+	if (!strncmp(page+ret, "add", 3)) {
+		ret += 3;
+		ad = RCF_ADD;
+	} else if (!strncmp(page+ret, "del", 3)) {
+		ret += 3;
+		ad = RCF_DEL;
+	} else 
+		return -EINVAL;
+	if (page[ret++] == 0)
+		return -EINVAL;
+
+	if (!strncmp(page+ret, "read", 4)) {
+		ret += 4;
+		rw = READ;
+	} else if (!strncmp(page+ret, "write", 5)) {
+		ret += 5;
+		rw = WRITE;
+	} else
+		return -EINVAL;
+	if (page[ret++] == 0)
+		return -EINVAL;
+
+	ss.from = (char *)page+ret;
+	ss.to = ss.from + count - ret;
+	ret += count - ret;
+
+	status = match_hex(&ss, &cmd);
+	if (status)
+		return status;
+	if (cmd > 256)
+		return -EINVAL;
+
+	if (ad == RCF_ADD) {
+		if (rw == READ)
+			set_bit(cmd, rcf->read_ok);
+		else
+			set_bit(cmd, rcf->write_ok);
+	} else {
+		if (rw == READ)
+			clear_bit(cmd, rcf->read_ok);
+		else
+			clear_bit(cmd, rcf->write_ok);
+	}
+
+	return count;
+}
+
+struct rcf_sysfs_entry {
+	struct attribute attr;
+	ssize_t (*show)(struct rawio_cmd_filter *, char *);
+	ssize_t (*store)(struct rawio_cmd_filter *, const char *, size_t);
+};
+
+static struct rcf_sysfs_entry rcf_readcmds_entry = {
+	.attr = {.name = "ok_read_commands", .mode = S_IRUGO | S_IWUSR },
+	.show = rcf_readcmds_show,
+	.store = rcf_store,
+};
+
+static struct rcf_sysfs_entry rcf_writecmds_entry = {
+	.attr = {.name = "ok_write_commands", .mode = S_IRUGO | S_IWUSR },
+	.show = rcf_writecmds_show,
+	.store = rcf_store,
+};
+
+static struct attribute *default_attrs[] = {
+	&rcf_readcmds_entry.attr,
+	&rcf_writecmds_entry.attr,
+	NULL,
+};
+
+#define to_rcf(atr) container_of((atr), struct rcf_sysfs_entry, attr)
+
+static ssize_t
+rcf_attr_show(struct kobject *kobj, struct attribute *attr, char *page)
+{
+	struct rcf_sysfs_entry *entry = to_rcf(attr);
+	struct rawio_cmd_filter *rcf;
+
+	rcf = container_of(kobj, struct rawio_cmd_filter, kobj);
+	if (!entry->show)
+		return 0;
+	
+	return entry->show(rcf, page);
+}
+
+static ssize_t
+rcf_attr_store(struct kobject *kobj, struct attribute *attr,
+			const char *page, size_t length)
+{
+	struct rcf_sysfs_entry *entry = to_rcf(attr);
+	struct rawio_cmd_filter *rcf;
+
+	rcf = container_of(kobj, struct rawio_cmd_filter, kobj);
+	if (!entry->store)
+		return -EINVAL;
+	
+	return entry->store(rcf, page, length);
+}
+
+static struct sysfs_ops rcf_sysfs_ops = {
+	.show = rcf_attr_show,
+	.store = rcf_attr_store,
+};
+
+static struct kobj_type rcf_ktype = {
+	.sysfs_ops = &rcf_sysfs_ops,
+	.default_attrs = default_attrs,
+};
+
+static void rcf_set_defaults(struct rawio_cmd_filter *rcf)
+{
+	/* Basic read-only commands */
+	set_bit(TEST_UNIT_READY, rcf->read_ok);
+	set_bit(REQUEST_SENSE, rcf->read_ok);
+	set_bit(READ_6, rcf->read_ok);
+	set_bit(READ_10, rcf->read_ok);
+	set_bit(READ_12, rcf->read_ok);
+	set_bit(READ_16, rcf->read_ok);
+	set_bit(READ_BUFFER, rcf->read_ok);
+	set_bit(READ_LONG, rcf->read_ok);
+	set_bit(INQUIRY, rcf->read_ok);
+	set_bit(MODE_SENSE, rcf->read_ok);
+	set_bit(MODE_SENSE_10, rcf->read_ok);
+	set_bit(START_STOP, rcf->read_ok);
+	set_bit(GPCMD_VERIFY_10, rcf->read_ok);
+	set_bit(VERIFY_16, rcf->read_ok);
+	set_bit(READ_BUFFER, rcf->read_ok);
+	
+	/* Audio CD commands */
+	set_bit(GPCMD_PLAY_CD, rcf->read_ok);
+	set_bit(GPCMD_PLAY_AUDIO_10, rcf->read_ok);
+	set_bit(GPCMD_PLAY_AUDIO_MSF, rcf->read_ok);
+	set_bit(GPCMD_PLAY_AUDIO_TI, rcf->read_ok);
+	set_bit(GPCMD_PAUSE_RESUME, rcf->read_ok);
+	
+	/* CD/DVD data reading */
+	set_bit(GPCMD_READ_CD, rcf->read_ok);
+	set_bit(GPCMD_READ_CD_MSF, rcf->read_ok);
+	set_bit(GPCMD_READ_DISC_INFO, rcf->read_ok);
+	set_bit(GPCMD_READ_CDVD_CAPACITY, rcf->read_ok);
+	set_bit(GPCMD_READ_DVD_STRUCTURE, rcf->read_ok);
+	set_bit(GPCMD_READ_HEADER, rcf->read_ok);
+	set_bit(GPCMD_READ_TRACK_RZONE_INFO, rcf->read_ok);
+	set_bit(GPCMD_READ_SUBCHANNEL, rcf->read_ok);
+	set_bit(GPCMD_READ_TOC_PMA_ATIP, rcf->read_ok);
+	set_bit(GPCMD_REPORT_KEY, rcf->read_ok);
+	set_bit(GPCMD_SCAN, rcf->read_ok);
+	set_bit(GPCMD_GET_CONFIGURATION, rcf->read_ok);
+	set_bit(GPCMD_READ_FORMAT_CAPACITIES, rcf->read_ok);
+	set_bit(GPCMD_GET_EVENT_STATUS_NOTIFICATION, rcf->read_ok);
+	set_bit(GPCMD_GET_PERFORMANCE, rcf->read_ok);
+	set_bit(GPCMD_SEEK, rcf->read_ok);
+	set_bit(GPCMD_STOP_PLAY_SCAN, rcf->read_ok);
+	
+	/* Basic writing commands */
+	set_bit(WRITE_6, rcf->write_ok);
+	set_bit(WRITE_10, rcf->write_ok);
+	set_bit(WRITE_VERIFY, rcf->write_ok);
+	set_bit(WRITE_12, rcf->write_ok);
+	set_bit(WRITE_VERIFY_12, rcf->write_ok);
+	set_bit(WRITE_16, rcf->write_ok);
+	set_bit(WRITE_LONG, rcf->write_ok);
+	set_bit(ERASE, rcf->write_ok);
+	set_bit(GPCMD_MODE_SELECT_10, rcf->write_ok);
+	set_bit(MODE_SELECT, rcf->write_ok);
+	set_bit(GPCMD_BLANK, rcf->write_ok);
+	set_bit(GPCMD_CLOSE_TRACK, rcf->write_ok);
+	set_bit(GPCMD_FLUSH_CACHE, rcf->write_ok);
+	set_bit(GPCMD_FORMAT_UNIT, rcf->write_ok);
+	set_bit(GPCMD_REPAIR_RZONE_TRACK, rcf->write_ok);
+	set_bit(GPCMD_RESERVE_RZONE_TRACK, rcf->write_ok);
+	set_bit(GPCMD_SEND_DVD_STRUCTURE, rcf->write_ok);
+	set_bit(GPCMD_SEND_EVENT, rcf->write_ok);
+	set_bit(GPCMD_SEND_KEY, rcf->write_ok);
+	set_bit(GPCMD_SEND_OPC, rcf->write_ok);
+	set_bit(GPCMD_SEND_CUE_SHEET, rcf->write_ok);
+	set_bit(GPCMD_SET_SPEED, rcf->write_ok);
+	set_bit(GPCMD_PREVENT_ALLOW_MEDIUM_REMOVAL, rcf->write_ok);
+	set_bit(GPCMD_LOAD_UNLOAD, rcf->write_ok);
+	set_bit(GPCMD_SET_STREAMING, rcf->write_ok);
+}
+
+int blk_register_filter(struct gendisk *disk)
+{
+	int ret;
+	struct rawio_cmd_filter *rcf = &disk->filter;
+
+	rcf->kobj.parent = kobject_get(&disk->kobj);
+	if (!rcf->kobj.parent)
+		return -EBUSY;
+
+	snprintf(rcf->kobj.name, KOBJ_NAME_LEN, "%s", "filter");
+	rcf->kobj.ktype = &rcf_ktype;
+
+	ret = kobject_register(&rcf->kobj);
+	if (ret < 0)
+		return ret;
+
+	rcf_set_defaults(&disk->filter);
+	
+	return 0;
+}
+
+void blk_unregister_filter(struct gendisk *disk)
+{
+	struct rawio_cmd_filter *rcf = &disk->filter;
+
+	kobject_unregister(&rcf->kobj);
+	kobject_put(&disk->kobj);
+}
Index: linux-2.6.17.7/block/scsi_ioctl.c
===================================================================
--- linux-2.6.17.7.orig/block/scsi_ioctl.c
+++ linux-2.6.17.7/block/scsi_ioctl.c
@@ -106,120 +106,6 @@ static int sg_emulated_host(request_queu
 	return put_user(1, p);
 }
 
-#define CMD_READ_SAFE	0x01
-#define CMD_WRITE_SAFE	0x02
-#define CMD_WARNED	0x04
-#define safe_for_read(cmd)	[cmd] = CMD_READ_SAFE
-#define safe_for_write(cmd)	[cmd] = CMD_WRITE_SAFE
-
-static int verify_command(struct file *file, unsigned char *cmd)
-{
-	static unsigned char cmd_type[256] = {
-
-		/* Basic read-only commands */
-		safe_for_read(TEST_UNIT_READY),
-		safe_for_read(REQUEST_SENSE),
-		safe_for_read(READ_6),
-		safe_for_read(READ_10),
-		safe_for_read(READ_12),
-		safe_for_read(READ_16),
-		safe_for_read(READ_BUFFER),
-		safe_for_read(READ_DEFECT_DATA),
-		safe_for_read(READ_LONG),
-		safe_for_read(INQUIRY),
-		safe_for_read(MODE_SENSE),
-		safe_for_read(MODE_SENSE_10),
-		safe_for_read(LOG_SENSE),
-		safe_for_read(START_STOP),
-		safe_for_read(GPCMD_VERIFY_10),
-		safe_for_read(VERIFY_16),
-
-		/* Audio CD commands */
-		safe_for_read(GPCMD_PLAY_CD),
-		safe_for_read(GPCMD_PLAY_AUDIO_10),
-		safe_for_read(GPCMD_PLAY_AUDIO_MSF),
-		safe_for_read(GPCMD_PLAY_AUDIO_TI),
-		safe_for_read(GPCMD_PAUSE_RESUME),
-
-		/* CD/DVD data reading */
-		safe_for_read(GPCMD_READ_BUFFER_CAPACITY),
-		safe_for_read(GPCMD_READ_CD),
-		safe_for_read(GPCMD_READ_CD_MSF),
-		safe_for_read(GPCMD_READ_DISC_INFO),
-		safe_for_read(GPCMD_READ_CDVD_CAPACITY),
-		safe_for_read(GPCMD_READ_DVD_STRUCTURE),
-		safe_for_read(GPCMD_READ_HEADER),
-		safe_for_read(GPCMD_READ_TRACK_RZONE_INFO),
-		safe_for_read(GPCMD_READ_SUBCHANNEL),
-		safe_for_read(GPCMD_READ_TOC_PMA_ATIP),
-		safe_for_read(GPCMD_REPORT_KEY),
-		safe_for_read(GPCMD_SCAN),
-		safe_for_read(GPCMD_GET_CONFIGURATION),
-		safe_for_read(GPCMD_READ_FORMAT_CAPACITIES),
-		safe_for_read(GPCMD_GET_EVENT_STATUS_NOTIFICATION),
-		safe_for_read(GPCMD_GET_PERFORMANCE),
-		safe_for_read(GPCMD_SEEK),
-		safe_for_read(GPCMD_STOP_PLAY_SCAN),
-
-		/* Basic writing commands */
-		safe_for_write(WRITE_6),
-		safe_for_write(WRITE_10),
-		safe_for_write(WRITE_VERIFY),
-		safe_for_write(WRITE_12),
-		safe_for_write(WRITE_VERIFY_12),
-		safe_for_write(WRITE_16),
-		safe_for_write(WRITE_LONG),
-		safe_for_write(WRITE_LONG_2),
-		safe_for_write(ERASE),
-		safe_for_write(GPCMD_MODE_SELECT_10),
-		safe_for_write(MODE_SELECT),
-		safe_for_write(LOG_SELECT),
-		safe_for_write(GPCMD_BLANK),
-		safe_for_write(GPCMD_CLOSE_TRACK),
-		safe_for_write(GPCMD_FLUSH_CACHE),
-		safe_for_write(GPCMD_FORMAT_UNIT),
-		safe_for_write(GPCMD_REPAIR_RZONE_TRACK),
-		safe_for_write(GPCMD_RESERVE_RZONE_TRACK),
-		safe_for_write(GPCMD_SEND_DVD_STRUCTURE),
-		safe_for_write(GPCMD_SEND_EVENT),
-		safe_for_write(GPCMD_SEND_KEY),
-		safe_for_write(GPCMD_SEND_OPC),
-		safe_for_write(GPCMD_SEND_CUE_SHEET),
-		safe_for_write(GPCMD_SET_SPEED),
-		safe_for_write(GPCMD_PREVENT_ALLOW_MEDIUM_REMOVAL),
-		safe_for_write(GPCMD_LOAD_UNLOAD),
-		safe_for_write(GPCMD_SET_STREAMING),
-	};
-	unsigned char type = cmd_type[cmd[0]];
-	int has_write_perm = 0;
-
-	/* Anybody who can open the device can do a read-safe command */
-	if (type & CMD_READ_SAFE)
-		return 0;
-
-	/*
-	 * file can be NULL from ioctl_by_bdev()...
-	 */
-	if (file)
-		has_write_perm = file->f_mode & FMODE_WRITE;
-
-	/* Write-safe commands just require a writable open.. */
-	if ((type & CMD_WRITE_SAFE) && has_write_perm)
-		return 0;
-
-	/* And root can do any command.. */
-	if (capable(CAP_SYS_RAWIO))
-		return 0;
-
-	if (!type) {
-		cmd_type[cmd[0]] = CMD_WARNED;
-		printk(KERN_WARNING "scsi: unknown opcode 0x%02x\n", cmd[0]);
-	}
-
-	/* Otherwise fail it with an "Operation not permitted" */
-	return -EPERM;
-}
-
 static int sg_io(struct file *file, request_queue_t *q,
 		struct gendisk *bd_disk, struct sg_io_hdr *hdr)
 {
@@ -236,7 +122,7 @@ static int sg_io(struct file *file, requ
 		return -EINVAL;
 	if (copy_from_user(cmd, hdr->cmdp, hdr->cmd_len))
 		return -EFAULT;
-	if (verify_command(file, cmd))
+	if (rcf_verify_command(file, cmd))
 		return -EPERM;
 
 	if (hdr->dxfer_len > (q->max_hw_sectors << 9))
@@ -431,7 +317,7 @@ int sg_scsi_ioctl(struct file *file, str
 	if (in_len && copy_from_user(buffer, sic->data + cmdlen, in_len))
 		goto error;
 
-	err = verify_command(file, rq->cmd);
+	err = rcf_verify_command(file, rq->cmd);
 	if (err)
 		goto error;
 
Index: linux-2.6.17.7/include/linux/blkdev.h
===================================================================
--- linux-2.6.17.7.orig/include/linux/blkdev.h
+++ linux-2.6.17.7/include/linux/blkdev.h
@@ -599,6 +599,9 @@ struct sec_size {
 	unsigned block_size_bits;
 };
 
+extern int blk_register_filter(struct gendisk *disk);
+extern void blk_unregister_filter(struct gendisk *disk);
+extern int rcf_verify_command(struct file *file, unsigned char *cmd);
 extern int blk_register_queue(struct gendisk *disk);
 extern void blk_unregister_queue(struct gendisk *disk);
 extern void register_disk(struct gendisk *dev);
Index: linux-2.6.17.7/include/linux/genhd.h
===================================================================
--- linux-2.6.17.7.orig/include/linux/genhd.h
+++ linux-2.6.17.7/include/linux/genhd.h
@@ -97,6 +97,15 @@ struct disk_stats {
 	unsigned long io_ticks;
 	unsigned long time_in_queue;
 };
+
+#define RCF_MAX_NR_CMDS	256
+#define	RCF_CMD_BYTES	(RCF_MAX_NR_CMDS / (sizeof (unsigned long) * 8))
+
+struct rawio_cmd_filter {
+	unsigned long read_ok[RCF_CMD_BYTES];
+	unsigned long write_ok[RCF_CMD_BYTES];
+	struct kobject kobj;
+};
 	
 struct gendisk {
 	int major;			/* major number of driver */
@@ -108,6 +117,7 @@ struct gendisk {
 	int part_uevent_suppress;
 	struct block_device_operations *fops;
 	struct request_queue *queue;
+	struct rawio_cmd_filter filter;
 	void *private_data;
 	sector_t capacity;

-- 
"Just how much can I get away with and still go to heaven?"

Freelance consultant specializing in device driver programming for Linux 
Christer Weinigel <christer@weinigel.se>  http://www.weinigel.se

Re: cd burning with plextor drives.

[Linus Torvalds]

On Sun, 31 Jul 2006, Christer Weinigel wrote:
> 
> Yes, a lot of those CD burners are ancient.  But I like the fact that
> Linux supports a lot of ancient hardware. :-)

Sure. But I don't think we should necessarily design any new code for it. 
It's not like this has _ever_ worked differently from what it does now, so 
if you have one of the pre-MMC drives, the right answer right now (and 
always has been, I think) is that you need to be root to burn them.

I seriously doubt anybody _really_ cares.

That said, I don't think Peter Jones' patch is wrong.  I don't have any 
strong feelings about it - if people actually want this adn feels it 
improves the situation, why not? Although to actually make _sense_, we'd 
want to have distros that actually want to use the interface to 
auto-populate the command things or something.. In the absense of interest 
from distros, I don't think it is worth it.

		Linus

Re: cd burning with plextor drives.

[Dave Jones]

On Mon, Jul 31, 2006 at 01:32:53PM -0700, Linus Torvalds wrote:
 > 
 > 
 > On Sun, 31 Jul 2006, Christer Weinigel wrote:
 > > 
 > > Yes, a lot of those CD burners are ancient.  But I like the fact that
 > > Linux supports a lot of ancient hardware. :-)
 > 
 > Sure. But I don't think we should necessarily design any new code for it. 
 > It's not like this has _ever_ worked differently from what it does now, so 
 > if you have one of the pre-MMC drives, the right answer right now (and 
 > always has been, I think) is that you need to be root to burn them.
 > 
 > I seriously doubt anybody _really_ cares.
 > 
 > That said, I don't think Peter Jones' patch is wrong.  I don't have any 
 > strong feelings about it - if people actually want this adn feels it 
 > improves the situation, why not? Although to actually make _sense_, we'd 
 > want to have distros that actually want to use the interface to 
 > auto-populate the command things or something.. In the absense of interest 
 > from distros, I don't think it is worth it.

With my distro vendor hat on, I'm interested in anything that makes
these bug reports go away.  I don't particularly care on the mechanism,
but we need *something*.

Should his patch be accepted, Peter would likely be the poor soul who gets
to implement the userspace bits for Fedora anyway, and I doubt he would've
hacked up the patch if he hadn't intended to do so :)

		Dave

-- 
http://www.codemonkey.org.uk

Re: cd burning with plextor drives.

[Jens Axboe]

On Mon, Jul 31 2006, Linus Torvalds wrote:
> 
> 
> On Sun, 31 Jul 2006, Christer Weinigel wrote:
> > 
> > Yes, a lot of those CD burners are ancient.  But I like the fact that
> > Linux supports a lot of ancient hardware. :-)
> 
> Sure. But I don't think we should necessarily design any new code for it. 
> It's not like this has _ever_ worked differently from what it does now, so 
> if you have one of the pre-MMC drives, the right answer right now (and 
> always has been, I think) is that you need to be root to burn them.
> 
> I seriously doubt anybody _really_ cares.

Probably not for the ancient ones, except the odd person still using
them :-)

There are, however, new uses of vendor specific commands that you need
access to for fully utilizing your drive. As mentioned, Plextor is one
of these. And they do make current and very good burners, yet we cannot
add their opcodes to the table because they (sanely) used the vendor
specific range for that.

> That said, I don't think Peter Jones' patch is wrong.  I don't have any 
> strong feelings about it - if people actually want this adn feels it 
> improves the situation, why not? Although to actually make _sense_, we'd 
> want to have distros that actually want to use the interface to 
> auto-populate the command things or something.. In the absense of interest 
> from distros, I don't think it is worth it.

The default table will be the same as know, distros can leave it alone
for now. It at least enables the user to add the vendor opcodes if need
be.

I'll test and mangle the patch for 2.6.19.

-- 
Jens Axboe

Re: cd burning with plextor drives.

[Linus Torvalds]

On Mon, 31 Jul 2006, Jens Axboe wrote:
> 
> There are, however, new uses of vendor specific commands that you need
> access to for fully utilizing your drive. As mentioned, Plextor is one
> of these. And they do make current and very good burners, yet we cannot
> add their opcodes to the table because they (sanely) used the vendor
> specific range for that.

However, before we do that, can somebody actially just FIX that stupid 
cdrecord bug. It shouldn't say it can't write the drive, if it's just that 
it's not allowed to use one of the vendor-specific extensions.

As mentioned, I bet burning _does_ actually work.

		Linus

Re: cd burning with plextor drives.

[Jens Axboe]

On Mon, Jul 31 2006, Linus Torvalds wrote:
> 
> 
> On Mon, 31 Jul 2006, Jens Axboe wrote:
> > 
> > There are, however, new uses of vendor specific commands that you need
> > access to for fully utilizing your drive. As mentioned, Plextor is one
> > of these. And they do make current and very good burners, yet we cannot
> > add their opcodes to the table because they (sanely) used the vendor
> > specific range for that.
> 
> However, before we do that, can somebody actially just FIX that stupid 
> cdrecord bug. It shouldn't say it can't write the drive, if it's just that 
> it's not allowed to use one of the vendor-specific extensions.
> 
> As mentioned, I bet burning _does_ actually work.

Oh yeah, I think so too. It's for other features on the Plextor, like
disc quality scans. So it's not critical as such for burning, but it is
something that we do want to work.

I don't think you disagree that we need to support the vendor unique
range of opcodes. And I don't think you disagree that we cannot safely
put these in the global table. So that leaves one sane option - per
device cmd filter table. I've kept Peter's original patch somewhat
uptodate, I'll bring it to 2.6.18-rc3 and test+submit for 2.6.19.

-- 
Jens Axboe

Re: cd burning with plextor drives.

[Douglas Gilbert]

Linus Torvalds wrote:
> 
> On Sat, 29 Jul 2006, James Bottomley wrote:
>> I concur.  If we're going to allow users access to burn CDs, it's
>> impossible to police them with certainty as this case indicates.
> 
> Not so. I can (and have) written tons of CD's as a normal user, with 
> perfect security.
> 
> No, the kernel shouldn't allow device-specific commands. That goes without 
> saying. Whether this is a sg.c problem, or a cdrecord problem is unclear, 
> I suspect it's the latter. 

Command filtering has always been dubious. The sg driver
takes the approach of allowing through a small number of
"safe" (often mandatory) SCSI _Primary_ Commands (SPC). It
takes no special account of the Multimedia Commands (MMC),
cdrecord or the SCSI Block Commands (SBC). The sg driver
filter does lean a little towards SBC by allowing the
READ CAPACITY command to be accessed O_RDONLY.

The block layer SG_IO filter bends over backwards to
support MMC and hence cdrecord. So it supports one
device _type_ specific class. Due to vendor specific
commands (e.g. from plextor) it cannot keep cdrecord
completely happy.

For a comparison of the two filters see table 3 in:
http://www.torque.net/sg/sg_io.html
That table highlights another difference between the
two filters:
  - sg: allow some commands to be accessed O_RDONLY
    and let all commands to be accessed O_RDWR
  - block SG_IO: has three states: allow some commands
    to be accessed O_RDONLY, a larger set O_RDWR and
    the rest with CAP_SYS_RAW_IO

The latter approach is harder to keep correct. Two glaring
faults are REPORT LUNS (mandatory since SPC-3) and
READ CAPACITY(16); both are "safe" but need CAP_SYS_RAW_IO
capability (usually root permissions). This may not annoy
cdrecord but it would peeve other pass through users.


If a user has read write permissions on
a full device (not just a partition in it) why shouldn't
they be able to send any (SCSI/ATA/...) pass through
command to it? When the sg driver was used to burn cd
and dvds in lk 2.4 series the window manager needed to
arrange for the GUI owner to have write permissions
on cd and dvd writing devices. No root permissions or
CAP_SYS_RAW_IO capability was needed.

Doug Gilbert

Re: cd burning with plextor drives.

[Linus Torvalds]

On Sat, 29 Jul 2006, Douglas Gilbert wrote:
>
> Command filtering has always been dubious.

No it has not.

Command filtering falls under the _very_ non-dubious heading of "of 
_course_ we have to do it". There is absolutely zero doubt about it at 
all. 

You literally have two choices:
 - you can filter commands
 - you can disallow all command access for non-specific-capability users.

Those are the two choices. There really is no third choice. The only 
question is the details of _how_ you do the filtering and/or disallowing.

> If a user has read write permissions on
> a full device (not just a partition in it) why shouldn't
> they be able to send any (SCSI/ATA/...) pass through
> command to it?

They have read-write access to the PLATTER. 

The fact that you may have access to write data to a disk does _not_ mean 
that you must necessarily be able to set the password on the disk so that 
nobody else can ever read or write data to that disk without your 
permission.

Quite frankly, if you don't see that as an "obvious", and that I'm 100% 
right when I say that you have the above _two_ choices, and that your 
choice simply is not a choice at all, but total idiocy, then I don't know 
what to say.

Put another way: you will remove that command filtering in 
block/scsi_ioctl.c only in a kernel that I don't maintain, or by disabling 
it in some way that is so hidden that I won't notice. Because I'm not so 
stupid as to think that it's ok for normal users to set driver passwords 
or rewrite the disk firmware just because they have write permissions to 
the device. That's pretty damn final.

But you can try to _improve_ the filtering. We've certainly done that 
before. Quite frankly, I don't think there's a lot there that can be 
improved upon any more, but it's certainly an option that we could change 
that filtering to be (a) per-device and (b) allow root to explicitly 
change it on a per-machine and per-device setting, with the current 
filtering rules being just the "default rules".

Then you could encode any additional rules you want in a /sbin/hotplug 
script or something. But the filtering isn't going _anywhere_, and what 
you suggest is just totally and utterly insane.

			Linus

Re: cd burning with plextor drives.

[Douglas Gilbert]

Linus Torvalds wrote:
> 
> On Sat, 29 Jul 2006, Douglas Gilbert wrote:
>> Command filtering has always been dubious.
> 
> No it has not.
> 
> Command filtering falls under the _very_ non-dubious heading of "of 
> _course_ we have to do it". There is absolutely zero doubt about it at 
> all. 
> 
> You literally have two choices:
>  - you can filter commands
>  - you can disallow all command access for non-specific-capability users.
> 
> Those are the two choices. There really is no third choice. The only 
> question is the details of _how_ you do the filtering and/or disallowing.
> 
>> If a user has read write permissions on
>> a full device (not just a partition in it) why shouldn't
>> they be able to send any (SCSI/ATA/...) pass through
>> command to it?
> 
> They have read-write access to the PLATTER. 
> 
> The fact that you may have access to write data to a disk does _not_ mean 
> that you must necessarily be able to set the password on the disk so that 
> nobody else can ever read or write data to that disk without your 
> permission.

See below. Non-root users do not usually have read
or write permissions to a _disk_. We were talking
about a different class of devices: cd/dvd drives.

> Quite frankly, if you don't see that as an "obvious", and that I'm 100% 
> right when I say that you have the above _two_ choices, and that your 
> choice simply is not a choice at all, but total idiocy, then I don't know 
> what to say.

Well if you want to filter, I'll be happy to find
flaws, both over and under filtering. Are you sure
SCSI commands are being sent through the pass through?
How about ATA commands tunnelled in SCSI commands
(already supported by libata). There was recent talk
of tunnelling SAS SMP functions through SCSI (SSP)
to name another (ab)use.

> Put another way: you will remove that command filtering in 
> block/scsi_ioctl.c only in a kernel that I don't maintain, or by disabling 
> it in some way that is so hidden that I won't notice. Because I'm not so 
> stupid as to think that it's ok for normal users to set driver passwords 
> or rewrite the disk firmware just because they have write permissions to 
> the device. That's pretty damn final.

You weren't at the storage summit. As stated in other
posts to this thread it was concluded that command
filtering is a flawed strategy. With that consensus
that leaves the "disallow all command access for
non-specific-capability users" option.

> But you can try to _improve_ the filtering. We've certainly done that 
> before. Quite frankly, I don't think there's a lot there that can be 
> improved upon any more, but it's certainly an option that we could change 
> that filtering to be (a) per-device and (b) allow root to explicitly 
> change it on a per-machine and per-device setting, with the current 
> filtering rules being just the "default rules".

By current rules I suppose you mean the block SG_IO rules. Do you really
mean "per-device" as in /dev/hdc or "per-device-type" as in disk versus
cd/dvd drive? The current rules are skewed towards MMC so far that
they are broken in that some SBC (i.e. hard disk) modifying commands
can slip through in the guise of innocent MMC commands.

> Then you could encode any additional rules you want in a /sbin/hotplug 
> script or something. But the filtering isn't going _anywhere_, and what 
> you suggest is just totally and utterly insane.

Sounds like you are playing the man and not the ball.
Why didn't you jump on either of these posts:
http://marc.theaimsgroup.com/?l=linux-scsi&m=115417174425242&w=2
http://marc.theaimsgroup.com/?l=linux-scsi&m=115418046812126&w=2
Hmm?


As you are no doubt aware, disk permissions and cd/dvd drive
permissions are set up differently:

$ df -hT
Filesystem    Type    Size  Used Avail Use% Mounted on
/dev/hda2     ext3     19G   15G  3.4G  82% /

$ ls -l /dev/hda /dev/hda2
brw-r----- 1 root disk 3, 0 Jul 29 05:14 /dev/hda
brw-r----- 1 root disk 3, 2 Jul 29 09:14 /dev/hda2

This email has been saved on /dev/hda2 but I don't have
permissions on it. I can't send any pass through commands
to /dev/hda or /dev/hda2 as a non-root user (unless
my user is in group 'disk' and the command only needs
O_RDONLY which rules out the SCSI WRITE command).

Here is my cd/dvd combo drive:

$ ls -l /dev/hdc
brw------- 1 dougg disk 22, 0 Jul 29 09:14 /dev/hdc

The GUI (kde of FC5) was started as user 'dougg' but
I'm writing this email as user 'torque' so I can't
access that cd/dvd drive at the pass through level.


There is another class of commands that the sg driver does
require CAP_SYS_RAW_IO capability for. Those are the ones
that do collateral damage. A bus reset command comes to
mind (also ATA soft resets on PATA can take out the other
device on the cable). IMO collateral damage is something
the OS should take special care with.

Doug Gilbert

Re: cd burning with plextor drives.

[Linus Torvalds]

On Sat, 29 Jul 2006, Douglas Gilbert wrote:
> 
> See below. Non-root users do not usually have read
> or write permissions to a _disk_. We were talking
> about a different class of devices: cd/dvd drives.

What has that got to do with _anything_?

What do you think is the difference between a disk and a CD drive?

And do you really think that it's the _medium_ that looks at the commands? 
I don't think so. Regardless of whether it's a disk or a CD-ROM, it's the 
controller in the _drive_ that looks and acts on those commands.

And that CD drive (that you have write access to) also has things like 
firmware. And how do you think that firmware is updated? By magic? Or by 
SCSI commands sent to the drive?

(Yeah, I realize that it's also possible that the firmware update could 
happen by the drive just reading a disk, since the controller will do 
various things on its own anyway. I don't actually know if that is ever 
the case, or possibly even the _common_ case, but my wild guess would be 
that it's a lot more common to have a firmware update SCSI command than to 
have the drive able to update its own firmware).

Are you _really_ suggesting that people who happen to log in to the 
console should automatically also be allowed to rewrite the CD-ROM drive 
firmware, just because they are supposed to be able to write to the CD-ROM 
media in that drive? Or do other random things that the kernel really 
doesn't know what they do?

For all we know, every single vendor-specific command might be a "please 
self-descruct now" command. That would be a pretty stupid vendor, but hey, 
nothing is impossible.

If you really think that normal people should be able to send arbitrary 
SCSI commands to their CD-ROM unit, just because they should be able to 
write to the _platter_ that happens to have been inserted into that unit, 
then you really are crazy.

By updating the firmware on the CD-ROM drive, you can basically make it be 
nonfunctional.

Or do you know what all those vendor-specific commands do? In that case, 
we could just say "ok, for this particular device, we know this command is 
safe, so we can let it through".

> You weren't at the storage summit. As stated in other
> posts to this thread it was concluded that command
> filtering is a flawed strategy. With that consensus
> that leaves the "disallow all command access for
> non-specific-capability users" option.

Apparently it was good that I wasn't at that storage summit, because you 
guys clearly don't care about users or sanity, and I'd just have been 
frustrated.

We _do_ want to allow users to write their own disks without SUID programs 
if at all possible, and if for no other reason than the fact that people 
expect it to work, and so far every SUID program to do so has been a 
disaster (I'm not saying that it necessarily _always_ is a disaster, and 
I'm not saying it's wrong, but at least historically it has been).

And that BY DEFINITION means that we need to do filtering. No ifs, buts or 
maybes about it.

> By current rules I suppose you mean the block SG_IO rules. Do you really
> mean "per-device" as in /dev/hdc or "per-device-type" as in disk versus
> cd/dvd drive?

Per-device as in /dev/hdc. Every device node would need to have its own 
filtering rule.

> Why didn't you jump on either of these posts:

Because I don't read linux-scsi? I replied when I was Cc'd and noticed 
(and hey, I sometimes get too much personal email too, so that "and 
noticed" is actually important. The fact that I don't reply to everything 
doesn't mean that I read it, understood it, and agreed with it).

			Linus

Re: cd burning with plextor drives.

[Linus Torvalds]

On Sat, 29 Jul 2006, Linus Torvalds wrote:
>
> Are you _really_ suggesting that people who happen to log in to the 
> console should automatically also be allowed to rewrite the CD-ROM drive 
> firmware, just because they are supposed to be able to write to the CD-ROM 
> media in that drive? Or do other random things that the kernel really 
> doesn't know what they do?

Btw, in the name of full disclosure, I should probably admit that when I 
did some of the block/scsi_ioctl.c reorganizations and tried to push 
people to realize that it was much better to do "cdrecord dev=/dev/hda" 
than by going through sg.c and the ide-scsi emulation thing, the code not 
only only first missed the CAP_SYS_RAWIO check entirely, I also then made 
the decision that only root could do any SCSI command ioctl's, and that I 
thought it was reasonable.

I was disabused of that notion fairly quickly. People _really_ didn't like 
the "if (!capable(CAP_SYS_RAWIO)) return -EPERM;" approach, and 
"verify_command()" was written pretty quickly.

So I have been disabused of that notion myself, the same way I now hope to 
pass on that wisdom to the next generation of abusees ;)

Anyway, the kernel _has_ gone both ways in the past - not having any 
access checks at all, and only allowing root. We tried it, and neither 
really worked.

And no, to my knowledge, nobody _actually_ ever fried their CD-ROM by a 
bad user overwriting the firmware, but quite frankly, I don't want to risk 
it.

(As a historical oddity - there _was_ this bug in some drive that would 
corrupt the firmware by mistake, though - I think we _did_ actually fry 
some drives by sending it a "cache flush" command that it wasn't 
expecting, and that it turned into firmware reload thing or something. I 
think Alan Cox knows all the gruesome details, but that we could 
definitely blame on bad hardware..

I think that actually happened to be a CD-ROM drive, but it wasn't even a 
user-generated command at all, it was the kernel doing the cache-flush all 
on its sorry own.. Somebody who remembers the exact details can fill us 
in just for completeness)

		Linus

Re: cd burning with plextor drives.

[Arjan van de Ven]

> (As a historical oddity - there _was_ this bug in some drive that would 
> corrupt the firmware by mistake, though - I think we _did_ actually fry 
> some drives by sending it a "cache flush" command that it wasn't 
> expecting, and that it turned into firmware reload thing or something. I 
> think Alan Cox knows all the gruesome details, but that we could 
> definitely blame on bad hardware..

since you asked for it...

a certain distribution shipped a preproduction version of a packet
writing patch or it was some barrier thing, I forgot, which would end up
sending an *ATA* "cache flush" command to ATAPI drives that didn't claim
to understand that command range. A certain model of cdrom drive (LG?)
actually had that same command byte code as "flash firmware". 

Arguably it's a very stupid value to pick, but it's also not technically
a drive bug; the drive explicitly claimed to not understand the command
set in question, and that distro kernel sent it anyway. 


-- 
if you want to mail me at work (you don't), use arjan (at) linux.intel.com

Re: cd burning with plextor drives.

[Matthew Wilcox]

On Sun, Jul 30, 2006 at 12:13:06AM +0200, Arjan van de Ven wrote:
> 
> > (As a historical oddity - there _was_ this bug in some drive that would 
> > corrupt the firmware by mistake, though - I think we _did_ actually fry 
> > some drives by sending it a "cache flush" command that it wasn't 
> > expecting, and that it turned into firmware reload thing or something. I 
> > think Alan Cox knows all the gruesome details, but that we could 
> > definitely blame on bad hardware..
> 
> since you asked for it...
> 
> a certain distribution shipped a preproduction version of a packet
> writing patch or it was some barrier thing, I forgot, which would end up
> sending an *ATA* "cache flush" command to ATAPI drives that didn't claim
> to understand that command range. A certain model of cdrom drive (LG?)
> actually had that same command byte code as "flash firmware". 
> 
> Arguably it's a very stupid value to pick, but it's also not technically
> a drive bug; the drive explicitly claimed to not understand the command
> set in question, and that distro kernel sent it anyway. 

Actually, the standard does prohibit these command values from being
reused for different purposes.  http://lwn.net/Articles/55815/

Re: cd burning with plextor drives.

[Rogier Wolff]

On Sat, Jul 29, 2006 at 01:33:42PM -0700, Linus Torvalds wrote:
> (Yeah, I realize that it's also possible that the firmware update
> could happen by the drive just reading a disk, since the controller
> will do various things on its own anyway. I don't actually know if
> that is ever the case, or possibly even the _common_ case, but my
> wild guess would be that it's a lot more common to have a firmware
> update SCSI command than to have the drive able to update its own
> firmware).

Just for the record: we've upgraded CD burner firmware, and it works
by running some silly little program on DOS. No media involved.

We've updated/modified harddisk drive firmware. Same procedure: vendor
specific commands, and voila! (We're currently banging the hardware to
do this, possibly we might port to some passtrhough thingy later on)

I have vendor confirmation that turning on some consumer electronics
while holding a specific key combo will make the device boot (some
stage-2 boot) from the inserted CD instead of the firmware inside.
They use this to boot the "diagnostics" CD, but an "update firmware"
CD could obviously be made. 

	Roger. 

-- 
** R.E.Wolff@BitWizard.nl ** http://www.BitWizard.nl/ ** +31-15-2600998 **
*-- BitWizard writes Linux device drivers for any device you may have! --*
Q: It doesn't work. A: Look buddy, doesn't work is an ambiguous statement. 
Does it sit on the couch all day? Is it unemployed? Please be specific! 
Define 'it' and what it isn't doing. --------- Adapted from lxrbot FAQ

Re: cd burning with plextor drives.

[Douglas Gilbert]

Linus Torvalds wrote:
> 
> On Sat, 29 Jul 2006, Douglas Gilbert wrote:
>> See below. Non-root users do not usually have read
>> or write permissions to a _disk_. We were talking
>> about a different class of devices: cd/dvd drives.
> 
> What has that got to do with _anything_?
> 
> What do you think is the difference between a disk and a CD drive?
> 
> And do you really think that it's the _medium_ that looks at the commands? 
> I don't think so. Regardless of whether it's a disk or a CD-ROM, it's the 
> controller in the _drive_ that looks and acts on those commands.
> 
> And that CD drive (that you have write access to) also has things like 
> firmware. And how do you think that firmware is updated? By magic? Or by 
> SCSI commands sent to the drive?

<snip>

Hard disks and CD/DVD drives are quite different from
a command set point of view. Both have high level
read operations that map very cleanly to the block
layer. The write side is very different. Disks have
a high level write operation that is symmetric with
the read operation and the write can be called many times
on the same lba with provision for multi user access
(locking). Writes and reads can be interspersed in
rapid succession.

OTOH CD/DVD drive media are written once (leaving aside
the "-RW" hack) and that write is done at a much
lower level (command set wise) than the read. The
application writing the cd/dvd media is assumed to
have exclusive access for the duration of the write
operation (which may be some time). During a write
operation the rest of the cd/dvd media is unavailable
for reading. That low level write operation assumes
the application knows a lot more about the drive and
the media than the OS would ever want to know. Another
difference is that the cd/dvd media is not only
removable but that a user expects to be able to
read it 5 or 10 years after it was burnt on a drive
from another manufacturer.

To cope with the lower level write once operation of a
cd/dvd drive, we need applications like cdrecord and
nero and they need a pass through interface. Just like
you, I have been abused by Joerg Schilling but there
was some technical merit in what he was saying and
it is reflected in the SG_IO ioctl design. Actually
the idea of the SG_IO ioctl itself came from Joerg
Schilling.


I have pointed out that the block layer SG_IO ioctl is
broken in various ways. Here is an example chosen because
it will be very difficult to exploit. The MMC command
set must not use opcodes (the first byte of a SCSI cdb)
that are used by the SCSI Primary Commands (SPC) but it
can overlap with other device specific command sets.
A quite important device specific command set is SBC (for
block devices including hard disks). The GET PERFORMANCE
MMC command is set for O_RDONLY access by the block layer
SG_IO filter and O_RDWR by the sg driver. That command uses
opcode 0xac . From a MMC point of view that opcode is "safe".
Now checking my references I observe that for the optical
block memory device type (covered by SBC) that is an
ERASE(12) operation. Choose your poison!


Also be aware that we have been in a world of multiple command
sets (and multiple transports) for some time. Take ATAPI devices:
one can send many SPC and MMC (SCSI) commands to such a device.
One can also send certain (transport and identification related)
ATA commands. The most obvious example of the latter is the
IDENTIFY PACKET DEVICE (ATA) command. As of lk 2.6.15 one can
send that ATA command two ways:
  - via the long standing ATA specific ioctls (e.g. the
    HDIO_GET_IDENTITY ioctl **)
  - if the device is libata connected (or any other SAT
    compliant layer), via the ATA PASS THROUGH SCSI command

The IDENTIFY PACKET DEVICE (ATA) command is "safe" and
should be allowed O_RDONLY but not all ATA commands that can
be sent to ATAPI device (e.g. SET FEATURES) could be
considered "safe". Once we move lower into the transport
domain, things can become even murkier with ATAPI
possibly conveyed across SATA (S-ATAPI) which in turn may
be conveyed across STP in SAS. Sometimes transport
characteristics can be tweaked "in-band", that is, down the
same path that commands are sent.


The point I'm trying to make is that opcode based filtering
may have been sufficient in 1992 (circa SCSI-2) but 14
years later it most certainly ain't.


Doug Gilbert

** Last time I looked the HDIO_GET_IDENTITY ioctl returned
   cached information. IMO that is incorrect, it should go
   out to the command each time.

Re: cd burning with plextor drives.

[Linus Torvalds]

On Sun, 30 Jul 2006, Douglas Gilbert wrote:
> 
> The point I'm trying to make is that opcode based filtering
> may have been sufficient in 1992 (circa SCSI-2) but 14
> years later it most certainly ain't.

Nobody has ever contested that we might not make it smarter.

The point I was makign was that _not_ filtering is insane. The filters 
could possibly be made smarter, but REMOVING THEM IS NOT AN OPTION. It's 
also not an option to make it root-only, because making cdrecord suid root 
simply isn't acceptable as things are now.

And that _is_ what you and Jens were talking about.

And that is also what you claimed that the storage summit decided was the 
sane approach.

So can we _please_ agree that the storage summit was just being stupid and 
incompetent, and hadn't thought it through? That filtering isn't going 
away, but it migt be extended.

Oh, btw, I'm perfectly fine with removing the filtering from "sg.c", and 
making _that_ interface be root-only. Anything that doesn't use the 
block/scsi_ioctl.c version of the interface is not worth supporting any 
more, since it hasn't _ever_ worked reliably with IDE-CD's, and quite 
frankly, IDE-CD's is pretty much 95+% of the market.

		Linus

Re: cd burning with plextor drives.

[Christer Weinigel]

Linus Torvalds <torvalds@osdl.org> writes:

> But you can try to _improve_ the filtering. We've certainly done that 
> before. Quite frankly, I don't think there's a lot there that can be 
> improved upon any more, but it's certainly an option that we could change 
> that filtering to be (a) per-device and (b) allow root to explicitly 
> change it on a per-machine and per-device setting, with the current 
> filtering rules being just the "default rules".

The filtering can definitely be improved.  Today sg_io filters on SCSI
commands only, but it really ought to have a filter for mode pages[1]
too.  The burn proof setting for a drive could be in a vendor specific
mode page and cdrecord should be able to write to it.  Another page
controls the SCSI bus timings and shold definitely not be writable by
cdrecord.

Or we could just do as Jörg Schilling says: trust him and just make
cdrecord suid root.

  /Christer

[1] http://en.wikipedia.org/wiki/SCSI_mode_pages

-- 
"Just how much can I get away with and still go to heaven?"

Freelance consultant specializing in device driver programming for Linux 
Christer Weinigel <christer@weinigel.se>  http://www.weinigel.se
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: cd burning with plextor drives.

[Linus Torvalds]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 228 bytes --]



On Sat, 29 Jul 2006, Christer Weinigel wrote:
> 
> Or we could just do as Jörg Schilling says: trust him and just make
> cdrecord suid root.

Mwhahhahhaaah.. 

[ Wiping tears of laughter off my face ]

That was funny.

		Linus

Re: cd burning with plextor drives.

[Linus Torvalds]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2639 bytes --]



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.

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

		Linus

Re: cd burning with plextor drives.

[Dave Jones]

On Sat, Jul 29, 2006 at 10:04:46AM -0700, Linus Torvalds wrote:

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

Ah, that's what I was hunting for.  I must have poked through every
file in drivers/scsi/ looking for that ioctl. It never occured to me
to even look in block/

I've not looked too closely at exactly which method cdrecord was using
in the numerous bug reports we've had on this, but ..

 > Btw, it's entirely possible that you might hit the same problem with 
 > /dev/hdc too.

This may be the case.

 >  - 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"?)

I had visions of extending verify_command() to be of the form..

	if (devicevendor==PLEXTOR) {
		safe_for_write(ENABLE_BURN_PROOF);
		safe_for_write(ENABLE_FROBNICATOR);
	}
	etc..

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

I concur it shouldn't fail completely. But at the same time, if a user
splashed out his pennies to get a modern writer with shiny features, it's
a bit crappy of us to make him be root to use them.

 > - can we _please_ not 
 >    rely on code that has been written by a certified nut-case?

It's a mystery to me why nothing better has come along over the years.

		Dave

-- 
http://www.codemonkey.org.uk

Re: cd burning with plextor drives.

[Rogier Wolff]

On Sat, Jul 29, 2006 at 01:22:05PM -0400, Dave Jones wrote:
> I had visions of extending verify_command() to be of the form..
> 
> 	if (devicevendor==PLEXTOR) {
> 		safe_for_write(ENABLE_BURN_PROOF);
> 		safe_for_write(ENABLE_FROBNICATOR);
> 	}
> 	etc..

Almost there.... 

Instead it should walk a (device-specific (*)) table, that specifies
what is "safe". The table has masks to specify which bits are
important and what values are expected and allowed etc etc.

This table could be initialized "empty". 

Next a simple interface should allow root to modify the table. 


I expect that after doing this, the code above the suggestion could be
deleted, and that the currently-hardcoded policy could move into the
default table, instead of having the table start out empty. This might
be a performance tradeoff (i.e. even though it reduces code size, the
hardcoded version may be much quicker than walking the table every
time).

Now distributions/users/sysadmins can chose: 

A) leave as is: The ACME DVDwriter's new SuperDVDProof feature
   will only work as root once implemented in cdrecord. 

B) insert a table entry: "everything allowed". This will allow
   users to shoot themselves in the foot. 

C) Be smart about it, and provide a mechanism to detect ACME
   drives with the feature, select the appropriate config file
   and upload the corresponding table.

Everybody happy?

	Roger. 


(*) /dev/hdc, not "Plextor". 

-- 
** R.E.Wolff@BitWizard.nl ** http://www.BitWizard.nl/ ** +31-15-2600998 **
*-- BitWizard writes Linux device drivers for any device you may have! --*
Q: It doesn't work. A: Look buddy, doesn't work is an ambiguous statement. 
Does it sit on the couch all day? Is it unemployed? Please be specific! 
Define 'it' and what it isn't doing. --------- Adapted from lxrbot FAQ

Re: cd burning with plextor drives.

[Arjan van de Ven]

> This table could be initialized "empty". 
> 
> Next a simple interface should allow root to modify the table. 

this got coded quite a while ago
http://www.uwsg.iu.edu/hypermail/linux/kernel/0409.1/2109.html

Re: cd burning with plextor drives.

[Steve McIntyre]

Rogier Wolff writes:
>On Sat, Jul 29, 2006 at 01:22:05PM -0400, Dave Jones wrote:
>> I had visions of extending verify_command() to be of the form..
>> 
>> 	if (devicevendor==PLEXTOR) {
>> 		safe_for_write(ENABLE_BURN_PROOF);
>> 		safe_for_write(ENABLE_FROBNICATOR);
>> 	}
>> 	etc..
>
>Almost there.... 
>
>Instead it should walk a (device-specific (*)) table, that specifies
>what is "safe". The table has masks to specify which bits are
>important and what values are expected and allowed etc etc.
>
>This table could be initialized "empty". 
>
>Next a simple interface should allow root to modify the table. 

Yup. I proposed a proof-of-concept patch to allow this a while back -
let root define policy per-device:

http://marc.theaimsgroup.com/?l=linux-scsi&m=113214433405637&w=2

-- 
Steve McIntyre, Cambridge, UK.                                steve@einval.com
Into the distance, a ribbon of black
Stretched to the point of no turning back

Re: cd burning with plextor drives.

[Jens Axboe]

On Sat, Jul 29 2006, Dave Jones wrote:
>  >  - 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"?)
> 
> I had visions of extending verify_command() to be of the form..
> 
> 	if (devicevendor==PLEXTOR) {
> 		safe_for_write(ENABLE_BURN_PROOF);
> 		safe_for_write(ENABLE_FROBNICATOR);
> 	}
> 	etc..

God Dave, that's horrible and completely unmaintanable! The main problem
with the device table right now is that it's completely kernel
controlled, thus burdening everybody with this policy. Lets get it fixed
instead of adding more warts to it.

-- 
Jens Axboe

Re: cd burning with plextor drives.

[Jens Axboe]

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

-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html