Re: [PATCH] allow root to modify raw scsi command permissions list

• Previous message: Bill Davidsen: "Re: [PATCH 1/3] Separate IRQ-stacks from 4K-stacks option"
• In reply to: Kai Makisara: "Re: [PATCH] allow root to modify raw scsi command permissions list"
• Next in thread: Kai Makisara: "Re: [PATCH] allow root to modify raw scsi command permissions list"
• Reply: Kai Makisara: "Re: [PATCH] allow root to modify raw scsi command permissions list"
• Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

To: Kai Makisara <Kai.Makisara@kolumbus.fi>
Date:	Wed, 15 Sep 2004 15:49:00 -0400

On Wed, 2004-09-15 at 22:14 +0300, Kai Makisara wrote:
> On Tue, 14 Sep 2004, Peter Jones wrote:
>
> > diff -urpN linux-2.5-export/drivers/block/Makefile pjones-2.5-export/drivers/block/Makefile
> > --- linux-2.5-export/drivers/block/Makefile 2004-09-10 11:54:01 -04:00
> > +++ pjones-2.5-export/drivers/block/Makefile 2004-09-10 12:11:50 -04:00
> > @@ -13,7 +13,7 @@
> > # kblockd threads
> > #
> [Patch snipped]
>
> Your patch allows updating the filters for each device. This is one aspect
> of the problem. Another problem is that the command filter appropriate for
> CD/DVD writers is _forced on all devices_. In your patch this is in the
> defaults. In the current scsi_ioctl filter it applies to everything.

My patch leaves the defaults as what are currently in the kernel.

> I have already commented on MODE SELECT. I still think it is dangerous.

It's only marked as being allowed if you have write access; /dev/hda
usually isn't a+w, last I checked.

I'd actually say that this isn't suitable for CD drives. To do CDDA
reads in many cases you need MODE_SELECT. As such, CD drives should
have MODE_SELECT in the list of things allowed to read. But the point
is that this gives control of the policy to the userland, where it
belongs.

> Looking at the command list reveals a couple of other problems:
> - The command GPCMD_READ_DISC_INFO is "safe_for_read". The command 0x51
> has also another use: XPWRITE(10), i.e., a write command. Clearly a
> problem.
> - The "safe_for_read" command GPCMD_REPORT_KEY, 0xa4, has aliases
> CHANGE ALIAS, SET DEVICE IDENTIFIER and SET TARGET PORT.
> - The "safe_for_write" command GPCMD_SEND_DVD_STRUCTURE has alias
> VOLUME SET OUT (raid configuration).

My intent with the patch isn't really to take any stance on which
commands should and shouldn't be allowed. In fact, it's quite the
opposite -- this allows initscripts/hotplug to install an appropriate
list of commands for each device.

> There are other aliases but they return information and don't look too
> dangerous. The command code is only 8 bits and there probably will be more
> aliasing in future.
>
> My conclusion is that the filter _necessary_ for burning CD/DVDs is _not
> safe for all devices_.

Sure. The point is that there's no reason the kernel should make that
its problem.

> How to solve this problem?

Let the distros pick the list.

> One idea I had was to add a sysfs-changable attribute accessible to the
> filter (disk or queue) that would have a few possible states: allow
> only root, allow filtered, allow all?

This is basically what my patch gives you. Here's a handy table:

owner mode explanation
root.disk 0600 allow only root
root.disk 0400 allow root read commands, and write if CAP_SYS_RAWIO
root.disk 0660 allow root and disk members to do everything
root.disk 0640 allow root everything, disk can do read commands.
root.disk 0644 allow root everything, everybody read commands.
root.disk 0664 allow root and disk everything, all else get read
etc

That's everything except "allow all", but that is easy:

for x in {0,1,2,3,4,5,6,7,8,9,a,b,c,d,e,f}\
{0,1,2,3,4,5,6,7,8,9,a,b,c,d,e,f} ; do
  echo "add read $x" > /sys/block/hda/filter/ok_read_commands
  echo "add write %x" > /sys/block/hda/filter/ok_write_commands
done

> The default would be to "allow only root". The cdrom registering could
> set this to "allow filtered". This would allow the current behaviour
> for CD/DVD drives but would be safe for others. Other devices could be
> selectively allowed passthrough access if necessary.

There's no point in making the cdrom registration have code for this --
just let userland do the policy stuff, like it should.

> This could also be done in your approach. One possibility would be to
> start with empty filter and call from CD/DVD registering a function that
> sets the filter you currently have as default. This would be both flexible
> and safe.

I don't want to do an empty filter, because that'll mean existing users
of SG_IO and such break for no real reason. The approach my patch takes
is not to change *any* of the existing policy, but to enable distros to
change it to their hearts' content. It's *dead* simple to make
initscripts and hotplug pick "correct" lists for each device.

-- 
        Peter
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

• Previous message: Bill Davidsen: "Re: [PATCH 1/3] Separate IRQ-stacks from 4K-stacks option"
• In reply to: Kai Makisara: "Re: [PATCH] allow root to modify raw scsi command permissions list"
• Next in thread: Kai Makisara: "Re: [PATCH] allow root to modify raw scsi command permissions list"
• Reply: Kai Makisara: "Re: [PATCH] allow root to modify raw scsi command permissions list"
• Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]