SG_IO and security

PATCH Alan Cox 13 participants 33 messages Aug 12, 2004 – Aug 16, 2004

Subsystems (4)

SCSI Block layer SCSI tape driver Uniform cdrom driver

lore

Share & tools

lore

Alan Cox

author

2004-08-12 13:19 UTC #0 lore raw

Since the entire thread seems to have died again unresolved I'd suggest the following patch should get into 2.6.8 so that anyone with read access to any block device cannot issue arbitary scsi commands to it (like writes or firmware erase)

block/scsi_ioctl.c renamed from block/scsi_ioctl.c~ +2 -0

@@ -115,6 +115,8 @@
 	char sense[SCSI_SENSE_BUFFERSIZE];
 	unsigned char cmd[BLK_MAX_CDB];
 
+	if (!capable(CAP_SYS_RAWIO))
+		return -EPERM;
 	if (hdr->interface_id != 'S')
 		return -EINVAL;
 	if (hdr->cmd_len > BLK_MAX_CDB)

Linus Torvalds

2004-08-12 16:39 UTC #1 lore raw

Quoted 5 lines · Alan Cox

Since the entire thread seems to have died again unresolved I'd suggest the following patch should get into 2.6.8 so that anyone with read access to any block device cannot issue arbitary scsi commands to it (like writes or firmware erase)

Hmm.. This still allows the old "junk" commands (SCSI_IOCTL_SEND_COMMAND).

I'll add that too.

Linus

Linus Torvalds

2004-08-12 16:45 UTC #2 lore raw

Quoted 2 lines · Linus Torvalds

Hmm.. This still allows the old "junk" commands (SCSI_IOCTL_SEND_COMMAND).

Btw, I think the _right_ thing to check is the write access of the file descriptor. If you have write access to a block device, you can delete the data, so you might as well be able to do the raw commands. And that would allow things like "disk" groups etc to work and burn CD's.

However, right now we don't even pass down the "struct file" to this function. We probably should. Anybody willing to go through the callers? (just a few callers, but things like cdrom_command() doesn't even have the file, so it has to be recursive).

Linus

Jeff Garzik

2004-08-12 16:55 UTC #3 lore raw

Quoted 7 lines · Linus Torvalds

Quoted 1 line · Linus Torvalds
Hmm.. This still allows the old "junk" commands (SCSI_IOCTL_SEND_COMMAND).
Btw, I think the _right_ thing to check is the write access of the file descriptor. If you have write access to a block device, you can delete the data, so you might as well be able to do the raw commands. And that would allow things like "disk" groups etc to work and burn CD's.

Define raw commands. I certainly don't want non-root users to be able to issue FORMAT UNIT on my hard drive.

Jeff

Jeff Garzik

2004-08-12 17:02 UTC #4 lore raw

Quoted 9 lines · Jeff Garzik

Quoted 7 lines · Linus Torvalds
Quoted 1 line · Linus Torvalds
Hmm.. This still allows the old "junk" commands (SCSI_IOCTL_SEND_COMMAND).

Quoted 5 lines

Quoted 1 line
Btw, I think the _right_ thing to check is the write access of the file descriptor. If you have write access to a block device, you can delete the data, so you might as well be able to do the raw commands. And that would allow things like "disk" groups etc to work and burn CD's.
Define raw commands. I certainly don't want non-root users to be able to issue FORMAT UNIT on my hard drive.

oh, to the block device, not the file.

nevermind.

Jeff

Linus Torvalds

2004-08-12 17:02 UTC #5 lore raw

In reply to Jeff Garzik

Quoted 8 lines · Jeff Garzik

Quoted 6 lines · Linus Torvalds
Quoted 1 line · Linus Torvalds
Hmm.. This still allows the old "junk" commands (SCSI_IOCTL_SEND_COMMAND).

Quoted 7 lines

Quoted 4 lines
Btw, I think the _right_ thing to check is the write access of the file descriptor. If you have write access to a block device, you can delete the data, so you might as well be able to do the raw commands. And that would allow things like "disk" groups etc to work and burn CD's.
Define raw commands. I certainly don't want non-root users to be able to issue FORMAT UNIT on my hard drive.

Ehh? The same ones you allow to write all over the raw device?

Let's see now:

	brw-rw----    1 root     disk       3,   0 Jan 30  2003 /dev/hda

would you put people you don't trust with your disk in the "disk" group?

Right. If you trust somebody enough that you give him write access to the disk, then you might as well trust him enough to do commands on it.

Conversely, if you don't trust him enough to do things like that, you shouldn't give him write access in the first place.

It's a hell of a lot easier to destroy a disk with

dd if=/dev/zero of=/dev/xxx bs=8k

than it is to do it with some special malicious command.

And yes, there's clearly a difference, but in general I'd say it is the _data_ on the disk that is worth something to you. The disk itself? Do you really fundamentally care?

Linus

Jeff Garzik

2004-08-12 17:14 UTC #6 lore raw

Quoted 7 lines · Linus Torvalds

Let's see now:
	brw-rw----    1 root     disk       3,   0 Jan 30  2003 /dev/hda
would you put people you don't trust with your disk in the "disk" group?
Right. If you trust somebody enough that you give him write access to the disk, then you might as well trust him enough to do commands on it.

Yeah, I agree. I was thinking write access to files on a hard drive, not write access to the blkdev itself.

Jeff

Kai Makisara

2004-08-12 19:22 UTC #7 lore raw

In reply to Linus Torvalds

Quoted 19 lines · Linus Torvalds

Quoted 8 lines · Jeff Garzik
Quoted 6 lines · Linus Torvalds
Quoted 1 line · Linus Torvalds
Hmm.. This still allows the old "junk" commands (SCSI_IOCTL_SEND_COMMAND).
Quoted 7 lines
Quoted 4 lines
Btw, I think the _right_ thing to check is the write access of the file descriptor. If you have write access to a block device, you can delete the data, so you might as well be able to do the raw commands. And that would allow things like "disk" groups etc to work and burn CD's.
Define raw commands. I certainly don't want non-root users to be able to issue FORMAT UNIT on my hard drive.

Quoted 8 lines

Ehh? The same ones you allow to write all over the raw device?
Let's see now:
	brw-rw----    1 root     disk       3,   0 Jan 30  2003 /dev/hda
would you put people you don't trust with your disk in the "disk" group?

This protects disks in practice but SG_IO is currently supported by other devices, at least SCSI tapes. It is reasonable in some organizations to give r/w access to ordinary users so that they can read/write tapes. I would be worried if this would enable the users, for instance, to mess up the mode page contents of the drive or change the firmware.

Signature · 1 line

Kai

Peter Jones

2004-08-13 19:28 UTC #8 lore raw

Quoted 5 lines · Kai Makisara

Quoted 4 lines · Linus Torvalds
Let's see now:
      brw-rw----    1 root     disk       3,   0 Jan 30  2003 /dev/hda

Quoted 7 lines

Quoted 2 lines
would you put people you don't trust with your disk in the "disk" group?
This protects disks in practice but SG_IO is currently supported by other devices, at least SCSI tapes. It is reasonable in some organizations to give r/w access to ordinary users so that they can read/write tapes. I would be worried if this would enable the users, for instance, to mess up the mode page contents of the drive or change the firmware.

Sure, but for that we need command based filtering.

This is at least a step in the right direction.

Jeff Garzik

2004-08-13 19:42 UTC #9 lore raw

Quoted 12 lines · Peter Jones

Quoted 9 lines · Kai Makisara
Quoted 6 lines · Linus Torvalds
Let's see now:
     brw-rw----    1 root     disk       3,   0 Jan 30  2003 /dev/hda
would you put people you don't trust with your disk in the "disk" group?

Quoted 8 lines

Quoted 5 lines
This protects disks in practice but SG_IO is currently supported by other devices, at least SCSI tapes. It is reasonable in some organizations to give r/w access to ordinary users so that they can read/write tapes. I would be worried if this would enable the users, for instance, to mess up the mode page contents of the drive or change the firmware.
Sure, but for that we need command based filtering.

We have that now (sigh). See attached patch, which is in BK...

A similar approach could be applied to tape as well.

Though in general I think command-based filtering is not scalable... at the very least I would prefer a list loaded from userspace at boot.

Jeff

# ChangeSet
# 2004/08/12 17:51:15-07:00 [email protected]
# Allow non-root users certain raw commands if they are deemed safe.
#
# We allow more commands if the disk was opened read-write.
#

drivers/block/scsi_ioctl.c +85 ~~-11~~

diff -Nru a/drivers/block/scsi_ioctl.c b/drivers/block/scsi_ioctl.c
@@ -105,8 +105,80 @@
 	return put_user(1, p);
 }
 
-static int sg_io(request_queue_t *q, struct gendisk *bd_disk,
-		 struct sg_io_hdr *hdr)
+#define CMD_READ_SAFE	0x01
+#define CMD_WRITE_SAFE	0x02
+#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 const 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_LONG),
+		safe_for_read(INQUIRY),
+		safe_for_read(MODE_SENSE),
+		safe_for_read(MODE_SENSE_10),
+		safe_for_read(START_STOP),
+
+		/* 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),
+
+		/* CD/DVD data reading */
+		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),
+
+		/* 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_BUFFER),
+		safe_for_write(WRITE_LONG),
+	};
+	unsigned char type = cmd_type[cmd[0]];
+
+	/* Anybody who can open the device can do a read-safe command */
+	if (type & CMD_READ_SAFE)
+		return 0;
+
+	/* Write-safe commands just require a writable open.. */
+	if (type & CMD_WRITE_SAFE) {
+		if (file->f_mode & FMODE_WRITE)
+			return 0;
+	}
+
+	/* And root can do any command.. */
+	if (capable(CAP_SYS_RAWIO))
+		return 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)
 {
 	unsigned long start_time;
 	int reading, writing;

@@ -115,14 +187,14 @@
 	char sense[SCSI_SENSE_BUFFERSIZE];
 	unsigned char cmd[BLK_MAX_CDB];
 
-	if (!capable(CAP_SYS_RAWIO))
-		return -EPERM;
 	if (hdr->interface_id != 'S')
 		return -EINVAL;
 	if (hdr->cmd_len > BLK_MAX_CDB)
 		return -EINVAL;
 	if (copy_from_user(cmd, hdr->cmdp, hdr->cmd_len))
 		return -EFAULT;
+	if (verify_command(file, cmd))
+		return -EPERM;
 
 	/*
 	 * we'll do that later

@@ -228,15 +300,13 @@
 #define READ_DEFECT_DATA_TIMEOUT	(60 * HZ )
 #define OMAX_SB_LEN 16          /* For backward compatibility */
 
-static int sg_scsi_ioctl(request_queue_t *q, struct gendisk *bd_disk,
-			 Scsi_Ioctl_Command __user *sic)
+static int sg_scsi_ioctl(struct file *file, request_queue_t *q,
+			 struct gendisk *bd_disk, Scsi_Ioctl_Command __user *sic)
 {
 	struct request *rq;
 	int err, in_len, out_len, bytes, opcode, cmdlen;
 	char *buffer = NULL, sense[SCSI_SENSE_BUFFERSIZE];
 
-	if (!capable(CAP_SYS_RAWIO))
-		return -EPERM;
 	/*
 	 * get in an out lengths, verify they don't exceed a page worth of data
 	 */

@@ -273,6 +343,10 @@
 	if (copy_from_user(buffer, sic->data + cmdlen, in_len))
 		goto error;
 
+	err = verify_command(file, rq->cmd);
+	if (err)
+		goto error;
+
 	switch (opcode) {
 		case SEND_DIAGNOSTIC:
 		case FORMAT_UNIT:

@@ -370,7 +444,7 @@
 			err = -EFAULT;
 			if (copy_from_user(&hdr, arg, sizeof(hdr)))
 				break;
-			err = sg_io(q, bd_disk, &hdr);
+			err = sg_io(file, q, bd_disk, &hdr);
 			if (err == -EFAULT)
 				break;

@@ -418,7 +492,7 @@
 			hdr.cmdp = ((struct cdrom_generic_command __user*) arg)->cmd;
 			hdr.cmd_len = sizeof(cgc.cmd);
 
-			err = sg_io(q, bd_disk, &hdr);
+			err = sg_io(file, q, bd_disk, &hdr);
 			if (err == -EFAULT)
 				break;

@@ -441,7 +515,7 @@
 			if (!arg)
 				break;
 
-			err = sg_scsi_ioctl(q, bd_disk, arg);
+			err = sg_scsi_ioctl(file, q, bd_disk, arg);
 			break;
 		case CDROMCLOSETRAY:
 			close = 1;

Kai Makisara

2004-08-14 07:23 UTC #10 lore raw

Quoted 20 lines · Jeff Garzik

Quoted 19 lines · Peter Jones
Quoted 14 lines · Kai Makisara
Quoted 6 lines · Linus Torvalds
Let's see now:
     brw-rw----    1 root     disk       3,   0 Jan 30  2003 /dev/hda
would you put people you don't trust with your disk in the "disk" group?
This protects disks in practice but SG_IO is currently supported by other devices, at least SCSI tapes. It is reasonable in some organizations to give r/w access to ordinary users so that they can read/write tapes. I would be worried if this would enable the users, for instance, to mess up the mode page contents of the drive or change the firmware.

Quoted 4 lines

Quoted 1 line
Sure, but for that we need command based filtering.
We have that now (sigh). See attached patch, which is in BK...

This patch looks OK except that it includes the command WRITE BUFFER. It is used to flash the firmware for many devices. Even the SCSI standard (draft) specifies that mode 06h is "Download microcode and save". This command _should be removed_ from the list of allowed commands.

Quoted 2 lines

A similar approach could be applied to tape as well.

As far as I can read the code, the filtering applies to all devices. Except WRITE BUFFER, the commands are acceptable for tapes considering the opening mode or undefined for tapes. The undefined commands may cause a device with bad firmware to lock the SCSI bus and in this way to cause DoS. However, this applies also to commands defined for a type of device but not implemented because they are optional.

I am ready to accept this risk of DoS (provided that allowing the filtered commands is really useful for somebody) but this is a risk we must recognize.

Quoted 3 lines

Though in general I think command-based filtering is not scalable... at the very least I would prefer a list loaded from userspace at boot.

I think always requiring CAP_RAWIO would be the approach of least surprise.

Signature · 1 line

Kai

Alan Cox

author

2004-08-14 16:36 UTC #11 lore raw

Quoted 5 lines · Kai Makisara

Quoted 3 lines
Though in general I think command-based filtering is not scalable... at the very least I would prefer a list loaded from userspace at boot.
I think always requiring CAP_RAWIO would be the approach of least surprise.

Sounds like an excuse for Al to get yet another file system into the kernel so you can edit filter rules 8)

Alan

Bill Davidsen

2004-08-16 22:24 UTC #12 lore raw

In reply to Jeff Garzik

Quoted 22 lines · Jeff Garzik

Quoted 20 lines · Peter Jones
Quoted 14 lines · Kai Makisara
Quoted 6 lines · Linus Torvalds
Let's see now:
     brw-rw----    1 root     disk       3,   0 Jan 30  2003 /dev/hda
would you put people you don't trust with your disk in the "disk" group?
This protects disks in practice but SG_IO is currently supported by other devices, at least SCSI tapes. It is reasonable in some organizations to give r/w access to ordinary users so that they can read/write tapes. I would be worried if this would enable the users, for instance, to mess up the mode page contents of the drive or change the firmware.

Quoted 8 lines

Quoted 1 line
Sure, but for that we need command based filtering.
We have that now (sigh). See attached patch, which is in BK...
A similar approach could be applied to tape as well.
Though in general I think command-based filtering is not scalable... at the very least I would prefer a list loaded from userspace at boot.

It would seem that the list is unlikely to change much, since it would presumably be limited to the standard SCSI commands, and require RAWIO for vendor commands. Do you see any like change I'm missing?

Signature · 3 lines

   -bill davidsen ([email protected])
"The secret to procrastination is to put things off until the
 last possible moment - but no longer"  -me

Bill Davidsen

2004-08-16 22:00 UTC #13 lore raw

In reply to Linus Torvalds

Quoted 28 lines · Linus Torvalds

Quoted 11 lines · Jeff Garzik
Quoted 7 lines · Linus Torvalds
Quoted 1 line · Linus Torvalds
Hmm.. This still allows the old "junk" commands (SCSI_IOCTL_SEND_COMMAND).
Btw, I think the _right_ thing to check is the write access of the file descriptor. If you have write access to a block device, you can delete the data, so you might as well be able to do the raw commands. And that would allow things like "disk" groups etc to work and burn CD's.
Define raw commands. I certainly don't want non-root users to be able to issue FORMAT UNIT on my hard drive.
Ehh? The same ones you allow to write all over the raw device?
Let's see now:
	brw-rw----    1 root     disk       3,   0 Jan 30  2003 /dev/hda
would you put people you don't trust with your disk in the "disk" group?
Right. If you trust somebody enough that you give him write access to the disk, then you might as well trust him enough to do commands on it.
Conversely, if you don't trust him enough to do things like that, you shouldn't give him write access in the first place.

Quoted 7 lines

It's a hell of a lot easier to destroy a disk with
dd if=/dev/zero of=/dev/xxx bs=8k
than it is to do it with some special malicious command.
And yes, there's clearly a difference, but in general I'd say it is the _data_ on the disk that is worth something to you. The disk itself? Do you really fundamentally care?

I will offer two cases which is not wildly improbable. User complains the CD burner will {burn faster, burn brand X media, write HD mode} if the firmware is upgraded. User has write to burn CDs, decides to flash the firmware herself, turns CD into paperweight. Or possibly user tries to install CD firmware on a disk drive.

Case two, user is DBA, has write on raw partitions for Oracle, can mangle the whole device, and through some stupidity does.

Signature · 3 lines

   -bill davidsen ([email protected])
"The secret to procrastination is to put things off until the
 last possible moment - but no longer"  -me

Arjan van de Ven

2004-08-12 17:07 UTC #14 lore raw

In reply to Jeff Garzik

Quoted 2 lines · Jeff Garzik

Define raw commands. I certainly don't want non-root users to be able to issue FORMAT UNIT on my hard drive.

then don't give them write access ;)

Jens Axboe

2004-08-12 17:35 UTC #15 lore raw

In reply to Linus Torvalds

Quoted 11 lines · Linus Torvalds

Quoted 2 lines · Linus Torvalds
Hmm.. This still allows the old "junk" commands (SCSI_IOCTL_SEND_COMMAND).
Btw, I think the _right_ thing to check is the write access of the file descriptor. If you have write access to a block device, you can delete the data, so you might as well be able to do the raw commands. And that would allow things like "disk" groups etc to work and burn CD's.

Quoted 4 lines

However, right now we don't even pass down the "struct file" to this function. We probably should. Anybody willing to go through the callers? (just a few callers, but things like cdrom_command() doesn't even have the file, so it has to be recursive).

Precisely, I guess that's a 2.6.9 job then. CDROM_SEND_PACKET works directly on top of SG_IO now, so should just work as long as sg_io() is translated.

I have no problem going over this, if somebody beats me to it, fine. I'll be gone on vacation next week.

Signature · 1 line

Jens Axboe

Jens Axboe

2004-08-12 18:29 UTC #16 lore raw

Quoted 17 lines · Jens Axboe

Quoted 11 lines · Linus Torvalds
Quoted 2 lines · Linus Torvalds
Hmm.. This still allows the old "junk" commands (SCSI_IOCTL_SEND_COMMAND).
Btw, I think the _right_ thing to check is the write access of the file descriptor. If you have write access to a block device, you can delete the data, so you might as well be able to do the raw commands. And that would allow things like "disk" groups etc to work and burn CD's.
Quoted 4 lines
However, right now we don't even pass down the "struct file" to this function. We probably should. Anybody willing to go through the callers? (just a few callers, but things like cdrom_command() doesn't even have the file, so it has to be recursive).

Quoted 6 lines

Precisely, I guess that's a 2.6.9 job then. CDROM_SEND_PACKET works directly on top of SG_IO now, so should just work as long as sg_io() is translated.
I have no problem going over this, if somebody beats me to it, fine. I'll be gone on vacation next week.

Here's a much better and more complete patch, imho, and doing it was no more than 20 minutes. It struck me while doing this that you could possibly get around just checking the data direction given in the sg_io_hdr. The advantage of that would be that you don't need a command table at all, the disadvantage is that it would still be possible for a given user with read-only access to a device to possibly confuse the device and/or cause bus resets. So it probably wont work. I added it as an extra check, though.

The table is as complete as cdrom.h is, plus the 6 and 16 byte variants from scsi.h. There are still commands missing, they can be added along the way though. Patch is compile tested.

===== drivers/block/scsi_ioctl.c 1.50 vs edited =====

drivers/block/scsi_ioctl.c +61 ~~-10~~

@@ -105,8 +105,56 @@
 	return put_user(1, p);
 }
 
+static int sg_allowed_cmd(unsigned char opcode, int may_write)
+{
+	if (capable(CAP_SYS_RAWIO))
+		return 1;
+	if (may_write)
+		return 1;
+
+	switch (opcode) {
+		case GPCMD_GET_CONFIGURATION:
+		case GPCMD_GET_EVENT_STATUS_NOTIFICATION:
+		case GPCMD_GET_PERFORMANCE:
+		case GPCMD_INQUIRY:
+		case GPCMD_MECHANISM_STATUS:
+		case MODE_SENSE:
+		case LOG_SENSE:
+		case GPCMD_MODE_SENSE_10:
+		case GPCMD_PLAY_AUDIO_10:
+		case GPCMD_PLAY_AUDIO_MSF:
+		case GPCMD_PLAY_AUDIO_TI:
+		case GPCMD_PLAY_CD:
+		case READ_6:
+		case GPCMD_READ_10:
+		case GPCMD_READ_12:
+		case READ_16:
+		case GPCMD_READ_CDVD_CAPACITY:
+		case GPCMD_READ_CD:
+		case GPCMD_READ_CD_MSF:
+		case GPCMD_READ_DISC_INFO:
+		case GPCMD_READ_DVD_STRUCTURE:
+		case GPCMD_READ_HEADER:
+		case GPCMD_READ_TRACK_RZONE_INFO:
+		case GPCMD_READ_SUBCHANNEL:
+		case GPCMD_READ_TOC_PMA_ATIP:
+		case GPCMD_REPORT_KEY:
+		case GPCMD_REQUEST_SENSE:
+		case GPCMD_SCAN:
+		case GPCMD_SEEK:
+		case GPCMD_START_STOP_UNIT:
+		case GPCMD_STOP_PLAY_SCAN:
+		case GPCMD_TEST_UNIT_READY:
+		case GPCMD_VERIFY_10:
+		case READ_BUFFER:
+			return 1;
+		default:
+			return 0;
+	}
+}
+
 static int sg_io(request_queue_t *q, struct gendisk *bd_disk,
-		 struct sg_io_hdr *hdr)
+		 struct sg_io_hdr *hdr, int may_write)
 {
 	unsigned long start_time;
 	int reading, writing;

@@ -115,14 +163,14 @@
 	char sense[SCSI_SENSE_BUFFERSIZE];
 	unsigned char cmd[BLK_MAX_CDB];
 
-	if (!capable(CAP_SYS_RAWIO))
-		return -EPERM;
 	if (hdr->interface_id != 'S')
 		return -EINVAL;
 	if (hdr->cmd_len > BLK_MAX_CDB)
 		return -EINVAL;
 	if (copy_from_user(cmd, hdr->cmdp, hdr->cmd_len))
 		return -EFAULT;
+	if (!sg_allowed_cmd(cmd[0], may_write))
+		return -EPERM;
 
 	/*
 	 * we'll do that later

@@ -149,6 +197,9 @@
 			break;
 		}
 
+		if (writing && !may_write)
+			return -EPERM;
+
 		rq = blk_rq_map_user(q, writing ? WRITE : READ, hdr->dxferp,
 				     hdr->dxfer_len);

@@ -229,14 +280,12 @@
 #define OMAX_SB_LEN 16          /* For backward compatibility */
 
 static int sg_scsi_ioctl(request_queue_t *q, struct gendisk *bd_disk,
-			 Scsi_Ioctl_Command __user *sic)
+			 Scsi_Ioctl_Command __user *sic, int may_write)
 {
 	struct request *rq;
 	int err, in_len, out_len, bytes, opcode, cmdlen;
 	char *buffer = NULL, sense[SCSI_SENSE_BUFFERSIZE];
 
-	if (!capable(CAP_SYS_RAWIO))
-		return -EPERM;
 	/*
 	 * get in an out lengths, verify they don't exceed a page worth of data
 	 */

@@ -323,8 +372,10 @@
 	return err;
 }
 
-int scsi_cmd_ioctl(struct gendisk *bd_disk, unsigned int cmd, void __user *arg)
+int scsi_cmd_ioctl(struct gendisk *bd_disk, struct file *file, unsigned int cmd,
+		   void __user *arg)
 {
+	int may_write = file->f_flags & FMODE_WRITE;
 	request_queue_t *q;
 	struct request *rq;
 	int close = 0, err;

@@ -370,7 +421,7 @@
 			err = -EFAULT;
 			if (copy_from_user(&hdr, arg, sizeof(hdr)))
 				break;
-			err = sg_io(q, bd_disk, &hdr);
+			err = sg_io(q, bd_disk, &hdr, may_write);
 			if (err == -EFAULT)
 				break;

@@ -418,7 +469,7 @@
 			hdr.cmdp = ((struct cdrom_generic_command __user*) arg)->cmd;
 			hdr.cmd_len = sizeof(cgc.cmd);
 
-			err = sg_io(q, bd_disk, &hdr);
+			err = sg_io(q, bd_disk, &hdr, may_write);
 			if (err == -EFAULT)
 				break;

@@ -441,7 +492,7 @@
 			if (!arg)
 				break;
 
-			err = sg_scsi_ioctl(q, bd_disk, arg);
+			err = sg_scsi_ioctl(q, bd_disk, arg, may_write);
 			break;
 		case CDROMCLOSETRAY:
 			close = 1;

===== drivers/block/paride/pcd.c 1.38 vs edited =====

drivers/block/paride/pcd.c +1 -1

@@ -259,7 +259,7 @@
 				unsigned cmd, unsigned long arg)
 {
 	struct pcd_unit *cd = inode->i_bdev->bd_disk->private_data;
-	return cdrom_ioctl(&cd->info, inode, cmd, arg);
+	return cdrom_ioctl(&cd->info, inode, file, cmd, arg);
 }
 
 static int pcd_block_media_changed(struct gendisk *disk)

===== drivers/cdrom/cdrom.c 1.65 vs edited =====

drivers/cdrom/cdrom.c +3 -2

@@ -2073,13 +2073,14 @@
  * mmc_ioct().
  */
 int cdrom_ioctl(struct cdrom_device_info *cdi, struct inode *ip,
-		unsigned int cmd, unsigned long arg)
+		struct file *file, unsigned int cmd, unsigned long arg)
 {
 	struct cdrom_device_ops *cdo = cdi->ops;
+	struct gendisk *disk = ip->i_bdev->bd_disk;
 	int ret;
 
 	/* Try the generic SCSI command ioctl's first.. */
-	ret = scsi_cmd_ioctl(ip->i_bdev->bd_disk, cmd, (void __user *)arg);
+	ret = scsi_cmd_ioctl(disk, file, cmd, (void __user *) arg);
 	if (ret != -ENOTTY)
 		return ret;

===== drivers/cdrom/cdu31a.c 1.42 vs edited =====

drivers/cdrom/cdu31a.c +1 -1

@@ -3179,7 +3179,7 @@
 static int scd_block_ioctl(struct inode *inode, struct file *file,
 				unsigned cmd, unsigned long arg)
 {
-	return cdrom_ioctl(&scd_info, inode, cmd, arg);
+	return cdrom_ioctl(&scd_info, inode, file, cmd, arg);
 }
 
 static int scd_block_media_changed(struct gendisk *disk)

===== drivers/cdrom/cm206.c 1.37 vs edited =====

drivers/cdrom/cm206.c +1 -1

@@ -1363,7 +1363,7 @@
 static int cm206_block_ioctl(struct inode *inode, struct file *file,
 				unsigned cmd, unsigned long arg)
 {
-	return cdrom_ioctl(&cm206_info, inode, cmd, arg);
+	return cdrom_ioctl(&cm206_info, inode, file, cmd, arg);
 }
 
 static int cm206_block_media_changed(struct gendisk *disk)

===== drivers/cdrom/mcd.c 1.35 vs edited =====

drivers/cdrom/mcd.c +1 -1

@@ -227,7 +227,7 @@
 static int mcd_block_ioctl(struct inode *inode, struct file *file,
 				unsigned cmd, unsigned long arg)
 {
-	return cdrom_ioctl(&mcd_info, inode, cmd, arg);
+	return cdrom_ioctl(&mcd_info, inode, file, cmd, arg);
 }
 
 static int mcd_block_media_changed(struct gendisk *disk)

===== drivers/cdrom/mcdx.c 1.37 vs edited =====

drivers/cdrom/mcdx.c +1 -1

@@ -233,7 +233,7 @@
 				unsigned cmd, unsigned long arg)
 {
 	struct s_drive_stuff *p = inode->i_bdev->bd_disk->private_data;
-	return cdrom_ioctl(&p->info, inode, cmd, arg);
+	return cdrom_ioctl(&p->info, inode, file, cmd, arg);
 }
 
 static int mcdx_block_media_changed(struct gendisk *disk)

===== drivers/cdrom/sbpcd.c 1.52 vs edited =====

drivers/cdrom/sbpcd.c +1 -1

@@ -5372,7 +5372,7 @@
 				unsigned cmd, unsigned long arg)
 {
 	struct sbpcd_drive *p = inode->i_bdev->bd_disk->private_data;
-	return cdrom_ioctl(p->sbpcd_infop, inode, cmd, arg);
+	return cdrom_ioctl(p->sbpcd_infop, inode, file, cmd, arg);
 }
 
 static int sbpcd_block_media_changed(struct gendisk *disk)

===== drivers/cdrom/viocd.c 1.6 vs edited =====

drivers/cdrom/viocd.c +1 -1

@@ -199,7 +199,7 @@
 		unsigned cmd, unsigned long arg)
 {
 	struct disk_info *di = inode->i_bdev->bd_disk->private_data;
-	return cdrom_ioctl(&di->viocd_info, inode, cmd, arg);
+	return cdrom_ioctl(&di->viocd_info, inode, file, cmd, arg);
 }
 
 static int viocd_blk_media_changed(struct gendisk *disk)

===== drivers/ide/ide-cd.c 1.88 vs edited =====

drivers/ide/ide-cd.c +2 -2

@@ -3395,10 +3395,10 @@
 {
 	struct block_device *bdev = inode->i_bdev;
 	ide_drive_t *drive = bdev->bd_disk->private_data;
-	int err = generic_ide_ioctl(bdev, cmd, arg);
+	int err = generic_ide_ioctl(bdev, file, cmd, arg);
 	if (err == -EINVAL) {
 		struct cdrom_info *info = drive->driver_data;
-		err = cdrom_ioctl(&info->devinfo, inode, cmd, arg);
+		err = cdrom_ioctl(&info->devinfo, inode, file, cmd, arg);
 	}
 	return err;
 }

===== drivers/ide/ide-disk.c 1.87 vs edited =====

drivers/ide/ide-disk.c +1 -1

@@ -1668,7 +1668,7 @@
 			unsigned int cmd, unsigned long arg)
 {
 	struct block_device *bdev = inode->i_bdev;
-	return generic_ide_ioctl(bdev, cmd, arg);
+	return generic_ide_ioctl(bdev, file, cmd, arg);
 }
 
 static int idedisk_media_changed(struct gendisk *disk)

===== drivers/ide/ide-floppy.c 1.39 vs edited =====

drivers/ide/ide-floppy.c +1 -1

@@ -1946,7 +1946,7 @@
 	ide_drive_t *drive = bdev->bd_disk->private_data;
 	idefloppy_floppy_t *floppy = drive->driver_data;
 	void __user *argp = (void __user *)arg;
-	int err = generic_ide_ioctl(bdev, cmd, arg);
+	int err = generic_ide_ioctl(bdev, file, cmd, arg);
 	int prevent = (arg) ? 1 : 0;
 	idefloppy_pc_t pc;
 	if (err != -EINVAL)

===== drivers/ide/ide-tape.c 1.42 vs edited =====

drivers/ide/ide-tape.c +1 -1

@@ -4807,7 +4807,7 @@
 {
 	struct block_device *bdev = inode->i_bdev;
 	ide_drive_t *drive = bdev->bd_disk->private_data;
-	int err = generic_ide_ioctl(bdev, cmd, arg);
+	int err = generic_ide_ioctl(bdev, file, cmd, arg);
 	if (err == -EINVAL)
 		err = idetape_blkdev_ioctl(drive, cmd, arg);
 	return err;

===== drivers/ide/ide.c 1.151 vs edited =====

drivers/ide/ide.c +9 -3

@@ -1453,8 +1453,8 @@
 	return ide_do_drive_cmd(drive, &rq, ide_head_wait);
 }
 
-int generic_ide_ioctl(struct block_device *bdev, unsigned int cmd,
-			unsigned long arg)
+int generic_ide_ioctl(struct block_device *bdev, struct file *file,
+			unsigned int cmd, unsigned long arg)
 {
 	ide_drive_t *drive = bdev->bd_disk->private_data;
 	ide_settings_t *setting;

@@ -1603,9 +1603,15 @@
 			return 0;
 		}
 
+		/*
+		 * should just be moved on top and passed first for all
+		 * cmds (needs to be for ide-tape/floppy to work anyways),
+		 * would need to check that there are no overlapping ioctls
+		 * between scsi and ide first
+		 */
 		case CDROMEJECT:
 		case CDROMCLOSETRAY:
-			return scsi_cmd_ioctl(bdev->bd_disk, cmd, p);
+			return scsi_cmd_ioctl(bdev->bd_disk, file, cmd, p);
 
 		case HDIO_GET_BUSSTATE:
 			if (!capable(CAP_SYS_ADMIN))

===== drivers/scsi/ide-scsi.c 1.42 vs edited =====

drivers/scsi/ide-scsi.c +1 -1

@@ -735,7 +735,7 @@
 			unsigned int cmd, unsigned long arg)
 {
 	struct block_device *bdev = inode->i_bdev;
-	return generic_ide_ioctl(bdev, cmd, arg);
+	return generic_ide_ioctl(bdev, file, cmd, arg);
 }
 
 static struct block_device_operations idescsi_ops = {

===== drivers/scsi/sd.c 1.154 vs edited =====

drivers/scsi/sd.c +1 -1

@@ -594,7 +594,7 @@
 		case SCSI_IOCTL_GET_BUS_NUMBER:
 			return scsi_ioctl(sdp, cmd, p);
 		default:
-			error = scsi_cmd_ioctl(disk, cmd, p);
+			error = scsi_cmd_ioctl(disk, filp, cmd, p);
 			if (error != -ENOTTY)
 				return error;
 	}

===== drivers/scsi/sr.c 1.113 vs edited =====

drivers/scsi/sr.c +1 -1

@@ -504,7 +504,7 @@
                 case SCSI_IOCTL_GET_BUS_NUMBER:
                         return scsi_ioctl(sdev, cmd, (void __user *)arg);
 	}
-	return cdrom_ioctl(&cd->cdi, inode, cmd, arg);
+	return cdrom_ioctl(&cd->cdi, inode, file, cmd, arg);
 }
 
 static int sr_block_media_changed(struct gendisk *disk)

===== drivers/scsi/st.c 1.92 vs edited =====

drivers/scsi/st.c +1 -1

@@ -3408,7 +3408,7 @@
 		case SCSI_IOCTL_GET_BUS_NUMBER:
 			break;
 		default:
-			i = scsi_cmd_ioctl(STp->disk, cmd_in, p);
+			i = scsi_cmd_ioctl(STp->disk, file, cmd_in, p);
 			if (i != -ENOTTY)
 				return i;
 			break;

===== include/linux/blkdev.h 1.149 vs edited =====

include/linux/blkdev.h +1 -1

@@ -517,7 +517,7 @@
 extern void blk_recount_segments(request_queue_t *, struct bio *);
 extern int blk_phys_contig_segment(request_queue_t *q, struct bio *, struct bio *);
 extern int blk_hw_contig_segment(request_queue_t *q, struct bio *, struct bio *);
-extern int scsi_cmd_ioctl(struct gendisk *, unsigned int, void __user *);
+extern int scsi_cmd_ioctl(struct gendisk *, struct file *, unsigned int, void __user *);
 extern void blk_start_queue(request_queue_t *q);
 extern void blk_stop_queue(request_queue_t *q);
 extern void __blk_stop_queue(request_queue_t *q);

===== include/linux/cdrom.h 1.20 vs edited =====

include/linux/cdrom.h +1 -1

@@ -985,7 +985,7 @@
 			struct file *fp);
 extern int cdrom_release(struct cdrom_device_info *cdi, struct file *fp);
 extern int cdrom_ioctl(struct cdrom_device_info *cdi, struct inode *ip,
-		unsigned int cmd, unsigned long arg);
+		struct file *file, unsigned int cmd, unsigned long arg);
 extern int cdrom_media_changed(struct cdrom_device_info *);
 
 extern int register_cdrom(struct cdrom_device_info *cdi);

===== include/linux/ide.h 1.127 vs edited =====

include/linux/ide.h +1 -1

@@ -1194,7 +1194,7 @@
 
 #define DRIVER(drive)		((drive)->driver)
 
-extern int generic_ide_ioctl(struct block_device *, unsigned, unsigned long);
+extern int generic_ide_ioctl(struct block_device *, struct file *, unsigned, unsigned long);
 
 /*
  * ide_hwifs[] is the master data structure used to keep track

Signature · 1 line

Jens Axboe

Jeff Garzik

2004-08-12 18:37 UTC #17 lore raw

drivers/block/paride/pcd.c line 262

Quoted 10 lines · Jens Axboe

close = 1;
===== drivers/block/paride/pcd.c 1.38 vs edited =====

drivers/block/paride/pcd.c +1 -1

@@ -259,7 +259,7 @@
 				unsigned cmd, unsigned long arg)
 {
 	struct pcd_unit *cd = inode->i_bdev->bd_disk->private_data;
-	return cdrom_ioctl(&cd->info, inode, cmd, arg);
+	return cdrom_ioctl(&cd->info, inode, file, cmd, arg);

If you have the struct file, can't you eliminate the inode argument?

Jeff

Jens Axboe

2004-08-12 18:43 UTC #18 lore raw

Quoted 14 lines · Jeff Garzik

drivers/block/paride/pcd.c line 262
Quoted 10 lines · Jens Axboe
close = 1;
===== drivers/block/paride/pcd.c 1.38 vs edited =====
drivers/block/paride/pcd.c +1 -1
@@ -259,7 +259,7 @@
 				unsigned cmd, unsigned long arg)
 {
 	struct pcd_unit *cd = inode->i_bdev->bd_disk->private_data;
-	return cdrom_ioctl(&cd->info, inode, cmd, arg);
+	return cdrom_ioctl(&cd->info, inode, file, cmd, arg);
If you have the struct file, can't you eliminate the inode argument?

How so?

Signature · 1 line

Jens Axboe

Christoph Hellwig

2004-08-12 18:46 UTC #19 lore raw

Quoted 3 lines · Jens Axboe

Quoted 1 line
If you have the struct file, can't you eliminate the inode argument?
How so?

file->f_dentry->d_inode

Jens Axboe

2004-08-12 18:48 UTC #20 lore raw

Quoted 6 lines · Christoph Hellwig

Quoted 3 lines · Jens Axboe
Quoted 1 line
If you have the struct file, can't you eliminate the inode argument?
How so?
file->f_dentry->d_inode

Ah, ok. IMHO that should be generally done for the file_op->ioctl then. But could easily be killed here of course.

Signature · 1 line

Jens Axboe

Alan Cox

author

2004-08-12 21:24 UTC #21 lore raw

In reply to Jens Axboe

drivers/block/scsi_ioctl.c line 113

Quoted 6 lines · Jens Axboe

drivers/block/scsi_ioctl.c +6 -0

+static int sg_allowed_cmd(unsigned char opcode, int may_write)
+{
+	if (capable(CAP_SYS_RAWIO))
+		return 1;
+	if (may_write)
+		return 1;

I agree with passing the data down, unfortunately anyone with a raw device access they can open for write can still physically anihiliate the hardware. That causes real problems for anyone allocating partitions for databases like Oracle, giving direct user access to devices for virtualization like UML, giving direct user access to a M/O drive.

It also doesn't solve the read/write outside of partition problem.

Alan

Alan Cox

2004-08-12 21:23 UTC #22 lore raw

In reply to Linus Torvalds

Quoted 3 lines · Linus Torvalds

Quoted 2 lines · Linus Torvalds
Hmm.. This still allows the old "junk" commands (SCSI_IOCTL_SEND_COMMAND).

That uses sg_io() so gets caught as well unless I screwed up following the code paths.

Quoted 4 lines

Btw, I think the _right_ thing to check is the write access of the file descriptor. If you have write access to a block device, you can delete the data, so you might as well be able to do the raw commands. And that would allow things like "disk" groups etc to work and burn CD's.

With the current code I can destroy all your hard disks given read access to the drive. With checks on writable I can destroy all your hard disks/cdroms as appropriate with write access.

Destroy here means "dead, defunct, pushing up the daisies, go order a new one kind of dead".

In essence the interface (and the SCSI/ATA/.. layers below) don't seperate media and device. This also kicks in for partitioning since write access to /dev/hda1 giving me SG_IO scsi access doesn't enforce partitioning.

We end up needing some notion of what commands should be allowed to any user and what commands should be allowed solely to superusers. That leads to a second question for you which is one I had an argument about Jens on.

Do we

a) Have code that essentially says "if read on base device can do ...., if write can do ... , else capable(...)"

b) ioctls/other command functionality for the stuff users should be allowed to do.

Option (a) means parsing command blocks which are pretty regular and parseable.

Alan

Eric Lammerts

2004-08-12 22:57 UTC #23 lore raw

Quoted 4 lines · Alan Cox

In essence the interface (and the SCSI/ATA/.. layers below) don't seperate media and device. This also kicks in for partitioning since write access to /dev/hda1 giving me SG_IO scsi access doesn't enforce partitioning.

But who needs SG_IO on partition devices?
Why not simply disallow it completely for non-root users?

Eric

Linus Torvalds

2004-08-13 00:10 UTC #24 lore raw

In reply to Alan Cox

Quoted 5 lines · Alan Cox

Quoted 2 lines
Quoted 2 lines
Hmm.. This still allows the old "junk" commands (SCSI_IOCTL_SEND_COMMAND).
That uses sg_io() so gets caught as well unless I screwed up following the code paths.

No, while the cdrom_ioctl thing does use sg_io, the really old and horrible sg_scsi_ioctl thing does it's own commands by hand.

I don't know why. I get the feeling that it _should_ use sg_io().

Quoted 2 lines

With the current code I can destroy all your hard disks given read access to the drive.

Oh, I clearly agree with you that something had to be done. Which is why I did an extended version of your patch already.

Quoted 10 lines

Do we
a) Have code that essentially says "if read on base device can do ...., if write can do ... , else capable(...)"
b) ioctls/other command functionality for the stuff users should be allowed to do.
Option (a) means parsing command blocks which are pretty regular and parseable.

Yes. I think we should do:
- harmless read-only commands that we know, we allow for read opens
- harmless write-only commands that we know, we allow for write opens
- commands we don't know, or commands that aren't harmless, we require RAWIO-capability for.

That seems fairly trivial to implement, and is clearly the user-friendly thing to do. The quick patch is just too damn unfriendly.

Linus

Jens Axboe

2004-08-13 06:59 UTC #25 lore raw

Quoted 6 lines · Linus Torvalds

Quoted 3 lines · Alan Cox
Quoted 2 lines
Quoted 2 lines
Hmm.. This still allows the old "junk" commands (SCSI_IOCTL_SEND_COMMAND).

Quoted 7 lines

Quoted 2 lines
That uses sg_io() so gets caught as well unless I screwed up following the code paths.
No, while the cdrom_ioctl thing does use sg_io, the really old and horrible sg_scsi_ioctl thing does it's own commands by hand.
I don't know why. I get the feeling that it _should_ use sg_io().

While that does make sense, it would be more code to fold them together than what is currently there. SCSI_IOCTL_SEND_COMMAND is really horrible, the person inventing that API should be subject to daily public ridicule.

Signature · 1 line

Jens Axboe

[email protected]

2004-08-13 07:22 UTC #26 lore raw

Quoted 4 lines · Jens Axboe

While that does make sense, it would be more code to fold them together than what is currently there. SCSI_IOCTL_SEND_COMMAND is really horrible, the person inventing that API should be subject to daily public ridicule.

Introduced in 0.96a, between Apr 9 1992 and Apr 24 1992; part of initial SCSI merge, so probably existed earlier than that (ultrastor claims to be 1991, aha1542 appears to have started in January 1992). Drew Eckhardt, most likely...

Arjan van de Ven

2004-08-13 07:43 UTC #27 lore raw

In reply to Jens Axboe

Quoted 5 lines

While that does make sense, it would be more code to fold them together than what is currently there. SCSI_IOCTL_SEND_COMMAND is really horrible, the person inventing that API should be subject to daily public ridicule.

sounds like deprecation of this interface should start (I suspect this one will need ample notice of that so the sooner we do .... )

Jens Axboe

2004-08-13 07:47 UTC #28 lore raw

Quoted 7 lines · Arjan van de Ven

Quoted 5 lines
While that does make sense, it would be more code to fold them together than what is currently there. SCSI_IOCTL_SEND_COMMAND is really horrible, the person inventing that API should be subject to daily public ridicule.
sounds like deprecation of this interface should start (I suspect this

Very much so, it should have been deprecated for the last 5 years :)

Quoted 1 line

one will need ample notice of that so the sooner we do .... )

I have no idea how many apps use this ioctl, does anyone have a rough list?

Signature · 1 line

Jens Axboe

Jeff Garzik

2004-08-13 19:23 UTC #29 lore raw

Quoted 10 lines · Jens Axboe

Quoted 6 lines · Arjan van de Ven
Quoted 4 lines
While that does make sense, it would be more code to fold them together than what is currently there. SCSI_IOCTL_SEND_COMMAND is really horrible, the person inventing that API should be subject to daily public ridicule.
sounds like deprecation of this interface should start (I suspect this

Quoted 8 lines

Very much so, it should have been deprecated for the last 5 years :)
Quoted 1 line
one will need ample notice of that so the sooner we do .... )
I have no idea how many apps use this ioctl, does anyone have a rough list?

Add a rate-limited "this feature is deprecated" feature and find out...

Jeff

Linus Torvalds

2004-08-13 19:38 UTC #30 lore raw

Quoted 7 lines · Jeff Garzik

2B
Quoted 3 lines · Jens Axboe
I have no idea how many apps use this ioctl, does anyone have a rough list?
Add a rate-limited "this feature is deprecated" feature and find out...

Googling for it does show it as being documented and apparently used by a few programs, at least...

Linus

Jeff Garzik

2004-08-13 19:53 UTC #31 lore raw

Quoted 6 lines · Linus Torvalds

Quoted 3 lines · Jeff Garzik
2B
Jens Axboe wrote:

Quoted 7 lines

Quoted 4 lines
Quoted 2 lines
I have no idea how many apps use this ioctl, does anyone have a rough list?
Add a rate-limited "this feature is deprecated" feature and find out...
Googling for it does show it as being documented and apparently used by a few programs, at least...

Personally I know it's in use, but for some programs it's a fallback if SG_IO or a more modern method doesn't work...

Since we have (AFAICS) five interfaces (scsi-send-command, sg v1, sg v2, sg v3, and SG_IO) I would rather just go ahead and

static int printed_warning;
if (!printed_warning++)

	printk(KERN_WARNING "SCSI_SEND_COMMAND deprecated\n");

Also, inevitably some of the programs using this will be ancient disk tools (some written by disk vendors long gone)...

Jeff

Florian Weimer

2004-08-13 19:58 UTC #32 lore raw

In reply to Linus Torvalds

* Linus Torvalds:

Quoted 8 lines · Linus Torvalds

Quoted 2 lines
Hmm.. This still allows the old "junk" commands (SCSI_IOCTL_SEND_COMMAND).
Btw, I think the _right_ thing to check is the write access of the file descriptor. If you have write access to a block device, you can delete the data, so you might as well be able to do the raw commands. And that would allow things like "disk" groups etc to work and burn CD's.

But wouldn't the ability to burn CDs imply that a user can also corrupted the firmware of the drive, unless other countermeasures are in place?