Skip to contentSkip to conversation

fix verify_command to allow burning more than 1 DVD

PATCH In review Michal Schmidt 4 participants 5 messages Jan 17, 2005 – Jan 19, 2005 Block layer

lore
Share & tools
lore

Michal Schmidt

author
#0 lore raw

PATCH 1/1 fix verify_command to allow burning more than 1 DVD

Hello,

I use K3B with growisofs to burn DVDs. After boot I can burn a DVD as a normal user. But only the first one. When I want to burn another one, K3B complains that it is unable to prevent media removal. Then only root can burn DVDs.
The bug is in the kernel in the function verify_command.
When a process opens the DVD recorder with O_RDONLY and issues a command which is marked safe_for_write, this function is supposed to just return -EPERM and do nothing more. However, there is a bug that causes the command to be marked as CMD_WARNED. From now on no non-privileged process is able to issue this command even if it correctly opens the device with O_RDWR - because the command is no longer marked as CMD_WRITE_SAFE. A patch is attached.

Michal

drivers/block/scsi_ioctl.c renamed from drivers/block/scsi_ioctl.c.orig +1 -3
@@ -197,9 +197,7 @@ static int verify_command(struct file *f
 	if (type & CMD_WRITE_SAFE) {
 		if (file->f_mode & FMODE_WRITE)
 			return 0;
-	}
-
-	if (!(type & CMD_WARNED)) {
+	} else if (!(type & CMD_WARNED)) {
 		cmd_type[cmd[0]] = CMD_WARNED;
 		printk(KERN_WARNING "scsi: unknown opcode 0x%02x\n", cmd[0]);
 	}
#1 lore raw
Quoted 18 lines · Michal Schmidt

I use K3B with growisofs to burn DVDs. After boot I can burn a DVD as a normal user. But only the first one. When I want to burn another one, K3B complains that it is unable to prevent media removal. Then only root can burn DVDs.
The bug is in the kernel in the function verify_command. When a process opens the DVD recorder with O_RDONLY and issues a command which is marked safe_for_write, this function is supposed to just return -EPERM and do nothing more. However, there is a bug that causes the command to be marked as CMD_WARNED. From now on no non-privileged process is able to issue this command even if it correctly opens the device with O_RDWR - because the command is no longer marked as CMD_WRITE_SAFE.
A patch is attached.

Michal

drivers/block/scsi_ioctl.c static int verify_command(struct file *f · line 203

Quoted 10 lines
drivers/block/scsi_ioctl.c renamed from drivers/block/scsi_ioctl.c.orig +1 -3
@@ -197,9 +197,7 @@ static int verify_command(struct file *f
 	if (type & CMD_WRITE_SAFE) {
 		if (file->f_mode & FMODE_WRITE)
 			return 0;
-	}
-
-	if (!(type & CMD_WARNED)) {
+	} else if (!(type & CMD_WARNED)) {
 		cmd_type[cmd[0]] = CMD_WARNED;
 		printk(KERN_WARNING "scsi: unknown opcode 0x%02x\n", cmd[0]);
 	}

That patch will not write the warning message in some cases. I think this patch is better:

---

 linux-petero/drivers/block/scsi_ioctl.c |    2 +-
 1 files changed, 1 insertion(+), 1 deletion(-)
drivers/block/scsi_ioctl.c renamed from drivers/block/scsi_ioctl.c~scsi-filter +1 -1
diff -puN drivers/block/scsi_ioctl.c~scsi-filter drivers/block/scsi_ioctl.c
@@ -200,7 +200,7 @@ static int verify_command(struct file *f
 	}
 
 	if (!(type & CMD_WARNED)) {
-		cmd_type[cmd[0]] = CMD_WARNED;
+		cmd_type[cmd[0]] |= CMD_WARNED;
 		printk(KERN_WARNING "scsi: unknown opcode 0x%02x\n", cmd[0]);
 	}

_

Signature · 2 lines
Peter Osterlund - [email protected]
http://web.telia.com/~u89404340
author
#2 lore raw
Quoted 17 lines · Peter Osterlund

drivers/block/scsi_ioctl.c static int verify_command(struct file *f · line 203

Quoted 13 lines · Michal Schmidt
drivers/block/scsi_ioctl.c renamed from drivers/block/scsi_ioctl.c.orig +1 -3
@@ -197,9 +197,7 @@ static int verify_command(struct file *f
 	if (type & CMD_WRITE_SAFE) {
 		if (file->f_mode & FMODE_WRITE)
 			return 0;
-	}
-
-	if (!(type & CMD_WARNED)) {
+	} else if (!(type & CMD_WARNED)) {
 		cmd_type[cmd[0]] = CMD_WARNED;
 		printk(KERN_WARNING "scsi: unknown opcode 0x%02x\n", cmd[0]);
 	}

That patch will not write the warning message in some cases.

Yes. In cases when the device is opened for reading and the command is known as safe_for_write. Do we really want to print this warning in that case?

Quoted 21 lines

I think this patch is better:

---

 linux-petero/drivers/block/scsi_ioctl.c |    2 +-
 1 files changed, 1 insertion(+), 1 deletion(-)

diff -puN drivers/block/scsi_ioctl.c~scsi-filter drivers/block/scsi_ioctl.c

drivers/block/scsi_ioctl.c renamed from drivers/block/scsi_ioctl.c~scsi-filter +1 -1
@@ -200,7 +200,7 @@ static int verify_command(struct file *f
 	}
 
 	if (!(type & CMD_WARNED)) {
-		cmd_type[cmd[0]] = CMD_WARNED;
+		cmd_type[cmd[0]] |= CMD_WARNED;
 		printk(KERN_WARNING "scsi: unknown opcode 0x%02x\n", cmd[0]);
 	}

_

#3 lore raw
Quoted 21 lines · Michal Schmidt
Quoted 11 lines · Peter Osterlund
Quoted 10 lines · Michal Schmidt

--- linux-2.6.11-mm1/drivers/block/scsi_ioctl.c.orig 2005-01-17
20:42:40.000000000 +0100
+++ linux-2.6.11-mm1/drivers/block/scsi_ioctl.c 2005-01-17
20:43:14.000000000 +0100

(quoted diff) +0 -2
@@ -197,9 +197,7 @@ static int verify_command(struct file *f
 	if (type & CMD_WRITE_SAFE) {
 		if (file->f_mode & FMODE_WRITE)
 			return 0;
-	}
-
Quoted 8 lines

Review static int verify_command(struct file *f · line 203

Quoted 5 lines
(quoted diff) +1 -1
-	if (!(type & CMD_WARNED)) {
+	} else if (!(type & CMD_WARNED)) {
 		cmd_type[cmd[0]] = CMD_WARNED;
 		printk(KERN_WARNING "scsi: unknown opcode 0x%02x\n", cmd[0]);
 	}

That patch will not write the warning message in some cases.

Quoted 3 lines

Yes. In cases when the device is opened for reading and the command is known as safe_for_write.
Do we really want to print this warning in that case?

No, the command should only be dumped if it is unknown and denied.

Signature · 1 line
Jens Axboe
#4 lore raw
In reply to Michal Schmidt

Am Montag, 17. Januar 2005 21:34 schrieb Michal Schmidt:

Quoted 1 line

The bug is in the kernel in the function verify_command.

Your patch works fine. Nasty bug that was...

Ciao