2005-01-17 20:34:36

by Michal Schmidt

[permalink] [raw]
Subject: [PATCH] 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


Attachments:
verify_command.patch (490.00 B)

2005-01-18 22:43:39

by Peter Osterlund

[permalink] [raw]
Subject: Re: [PATCH] fix verify_command to allow burning more than 1 DVD

Michal Schmidt <[email protected]> writes:

> 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
> --- 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
> @@ -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(-)

diff -puN drivers/block/scsi_ioctl.c~scsi-filter drivers/block/scsi_ioctl.c
--- linux/drivers/block/scsi_ioctl.c~scsi-filter 2005-01-18 23:38:37.966026728 +0100
+++ linux-petero/drivers/block/scsi_ioctl.c 2005-01-18 23:38:37.970026120 +0100
@@ -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]);
}

_

--
Peter Osterlund - [email protected]
http://web.telia.com/~u89404340

2005-01-19 00:44:47

by Michal Schmidt

[permalink] [raw]
Subject: Re: [PATCH] fix verify_command to allow burning more than 1 DVD

Peter Osterlund wrote:
> Michal Schmidt <[email protected]> writes:
>>--- 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
>>@@ -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?

> 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
> --- linux/drivers/block/scsi_ioctl.c~scsi-filter 2005-01-18 23:38:37.966026728 +0100
> +++ linux-petero/drivers/block/scsi_ioctl.c 2005-01-18 23:38:37.970026120 +0100
> @@ -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]);
> }
>
> _
>

2005-01-19 07:26:11

by Jens Axboe

[permalink] [raw]
Subject: Re: [PATCH] fix verify_command to allow burning more than 1 DVD

On Wed, Jan 19 2005, Michal Schmidt wrote:
> Peter Osterlund wrote:
> >Michal Schmidt <[email protected]> writes:
> >>--- 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
> >>@@ -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?

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

--
Jens Axboe

2005-01-19 20:02:06

by Dominique Simon

[permalink] [raw]
Subject: Re: [PATCH] fix verify_command to allow burning more than 1 DVD

Am Montag, 17. Januar 2005 21:34 schrieb Michal Schmidt:
> The bug is in the kernel in the function verify_command.

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

Ciao