All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jens Axboe <axboe@suse.de>
To: Kai Makisara <Kai.Makisara@kolumbus.fi>
Cc: Peter Jones <pjones@redhat.com>, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] allow root to modify raw scsi command permissions list
Date: Wed, 15 Sep 2004 21:50:28 +0200	[thread overview]
Message-ID: <20040915195024.GA3899@suse.de> (raw)
In-Reply-To: <Pine.LNX.4.58.0409152151190.1972@kai.makisara.local>

On Wed, Sep 15 2004, 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.
> 
> I have already commented on MODE SELECT. I still think it is dangerous. 
> 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).
> 
> 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_.
> 
> How to solve this problem? 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? 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.
> 
> 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 think that's a really nice idea. I've said right from the beginning
that the command tables cannot even be per-device type, at least with
CDROMs we have examples of commands with different meanings. But at
least with a device-type default list we're a little closer.

A good reminder of why the whole thing is a mess :-)

-- 
Jens Axboe


  parent reply	other threads:[~2004-09-15 19:50 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2004-09-14 14:51 [PATCH] allow root to modify raw scsi command permissions list Peter Jones
2004-09-14 22:43 ` Marc Ballarin
2004-09-15 17:08   ` Peter Jones
2004-09-15 19:14 ` Kai Makisara
2004-09-15 19:49   ` Peter Jones
2004-09-15 20:49     ` Kai Makisara
2004-09-15 19:50   ` Jens Axboe [this message]
2004-09-15 20:32     ` Peter Jones
2004-09-16  9:33       ` Marc Ballarin
2004-09-15 21:08 ` [PATCH-NEW] " Marc Ballarin
2004-09-15 21:38   ` Alan Cox
2004-09-15 23:03     ` Peter Jones
2004-09-15 22:23       ` Alan Cox
2004-09-15 23:33     ` Marc Ballarin
2004-09-15 23:36       ` Peter Jones
2004-09-16 12:21         ` Marc Ballarin
2004-09-16 15:10         ` Marc Ballarin
2004-09-16 17:36           ` Peter Jones

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20040915195024.GA3899@suse.de \
    --to=axboe@suse.de \
    --cc=Kai.Makisara@kolumbus.fi \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pjones@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.