lkml.org 
[lkml]   [2004]   [Aug]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: ide-cd problems
    On Tue, Aug 03, 2004 at 07:53:40AM +0200, Jens Axboe wrote:
    > On Tue, Aug 03 2004, Alan Cox wrote:
    > > On Sad, 2004-07-31 at 21:00, Jens Axboe wrote:
    > > > If you want it to work that way, you have the have a pass-through filter
    > > > in the kernel knowing what commands are out there (including vendor
    > > > specific ones). That's just too ugly and not really doable or
    > > > maintainable, sorry.
    > >
    > > I disagree providing you turn it the other way around. The majority of
    > > scsi commands have to be protected because you can destroy the drive
    > > with some of them or bypass the I/O layers. (Eg using SG_IO to do writes
    > > to raw disk to bypass auditing layers)
    > >
    > > So you need CAP_SYS_RAWIO for most commands. You can easily build a list
    > > of sane commands for a given media type that are harmless and it fits
    > > the kernel role of a gatekeeper to do that.
    >
    > So that's where we vehemently disagree - it fits the kernel role, if you
    > allow it to control policy all of a sudden. And it's not easy, unless
    > you do it per specific device (not just type, make and model).
    >

    Vendor commands would be tricky (it would probably be best to limit
    vendor commands to well-established ones, and just disallow the rest
    without CAP_SYS_RAWIO or such), but standard commands wouldn't have a
    problem, and those don't get added very often.

    > > Providing the 'allowed' function is driver level and we also honour
    > > read/write properly for that case (so it doesnt bypass block I/O
    > > restrictions and fail the least suprise test) then it seems quite
    > > doable.
    > >
    > > For such I/O you'd then do
    > >
    > > if(capable(CAP_SYS_RAWIO) || driver->allowed(driver, blah, cmdblock))
    > >
    > > If the allowed function filters positively "unknown is not allowed" and
    > > the default allowed function is simply "no" it works.
    >
    > Until there's a new valid command for some device, in which case you
    > have to update your kernel?
    >

    As standard commands don't get added very often, that's not a huge
    problem, but.. see the attached patch.

    > > We'd end up with a list of allowed commands for all sorts of operations
    > > that don't threaten the machine while blocking vendor specific wonders
    > > and also cases where users can do stuff like firmware erase.
    >
    > Sorry, I think this model is totally bogus and I'd absolutely refuse to
    > merge any such beast into the block layer sg code.
    >

    Well, would something like this patch be acceptable? It just makes
    SG_IO require write access to the device (cdrecord and cdrdao both
    open it this way already, so users shouldn't have a problem with it).
    I probably forgot some stuff, etc. I'm not terribly familiar with the
    code in question.

    --
    Zinx Verituse http://zinx.xmms.org/
    Only in linux-2.6.8-rc2: .config
    diff -ru linux-2.6.8-rc2.orig/drivers/block/paride/pcd.c linux-2.6.8-rc2/drivers/block/paride/pcd.c
    --- linux-2.6.8-rc2.orig/drivers/block/paride/pcd.c 2004-08-02 19:58:12.000000000 -0500
    +++ linux-2.6.8-rc2/drivers/block/paride/pcd.c 2004-08-02 20:17:37.000000000 -0500
    @@ -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)
    diff -ru linux-2.6.8-rc2.orig/drivers/cdrom/cdrom.c linux-2.6.8-rc2/drivers/cdrom/cdrom.c
    --- linux-2.6.8-rc2.orig/drivers/cdrom/cdrom.c 2004-08-02 19:58:12.000000000 -0500
    +++ linux-2.6.8-rc2/drivers/cdrom/cdrom.c 2004-08-02 20:18:22.000000000 -0500
    @@ -2065,15 +2065,18 @@
    * 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;
    int ret;

    /* Try the generic SCSI command ioctl's first.. */
    - ret = scsi_cmd_ioctl(ip->i_bdev->bd_disk, cmd, (void __user *)arg);
    - if (ret != -ENOTTY)
    - return ret;
    + /* But only if the user has write access: open(,O_RDWR | O_NONBLOCK) */
    + if (file->f_mode & FMODE_WRITE) {
    + ret = scsi_cmd_ioctl(ip->i_bdev->bd_disk, cmd, (void __user *)arg);
    + if (ret != -ENOTTY)
    + return ret;
    + }

    /* the first few commands do not deal with audio drive_info, but
    only with routines in cdrom device operations. */
    diff -ru linux-2.6.8-rc2.orig/drivers/cdrom/cdu31a.c linux-2.6.8-rc2/drivers/cdrom/cdu31a.c
    --- linux-2.6.8-rc2.orig/drivers/cdrom/cdu31a.c 2004-06-16 00:19:42.000000000 -0500
    +++ linux-2.6.8-rc2/drivers/cdrom/cdu31a.c 2004-08-02 20:19:06.000000000 -0500
    @@ -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)
    diff -ru linux-2.6.8-rc2.orig/drivers/cdrom/cm206.c linux-2.6.8-rc2/drivers/cdrom/cm206.c
    --- linux-2.6.8-rc2.orig/drivers/cdrom/cm206.c 2004-06-16 00:20:03.000000000 -0500
    +++ linux-2.6.8-rc2/drivers/cdrom/cm206.c 2004-08-02 20:19:21.000000000 -0500
    @@ -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)
    diff -ru linux-2.6.8-rc2.orig/drivers/cdrom/mcd.c linux-2.6.8-rc2/drivers/cdrom/mcd.c
    --- linux-2.6.8-rc2.orig/drivers/cdrom/mcd.c 2004-06-16 00:19:37.000000000 -0500
    +++ linux-2.6.8-rc2/drivers/cdrom/mcd.c 2004-08-02 20:18:57.000000000 -0500
    @@ -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)
    diff -ru linux-2.6.8-rc2.orig/drivers/cdrom/mcdx.c linux-2.6.8-rc2/drivers/cdrom/mcdx.c
    --- linux-2.6.8-rc2.orig/drivers/cdrom/mcdx.c 2004-08-02 19:58:12.000000000 -0500
    +++ linux-2.6.8-rc2/drivers/cdrom/mcdx.c 2004-08-02 20:18:46.000000000 -0500
    @@ -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)
    diff -ru linux-2.6.8-rc2.orig/drivers/cdrom/sbpcd.c linux-2.6.8-rc2/drivers/cdrom/sbpcd.c
    --- linux-2.6.8-rc2.orig/drivers/cdrom/sbpcd.c 2004-06-16 00:18:59.000000000 -0500
    +++ linux-2.6.8-rc2/drivers/cdrom/sbpcd.c 2004-08-02 20:18:37.000000000 -0500
    @@ -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)
    diff -ru linux-2.6.8-rc2.orig/drivers/cdrom/viocd.c linux-2.6.8-rc2/drivers/cdrom/viocd.c
    --- linux-2.6.8-rc2.orig/drivers/cdrom/viocd.c 2004-08-02 19:58:12.000000000 -0500
    +++ linux-2.6.8-rc2/drivers/cdrom/viocd.c 2004-08-02 20:19:35.000000000 -0500
    @@ -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)
    diff -ru linux-2.6.8-rc2.orig/drivers/ide/ide-cd.c linux-2.6.8-rc2/drivers/ide/ide-cd.c
    --- linux-2.6.8-rc2.orig/drivers/ide/ide-cd.c 2004-08-02 19:58:13.000000000 -0500
    +++ linux-2.6.8-rc2/drivers/ide/ide-cd.c 2004-08-02 20:16:59.000000000 -0500
    @@ -3394,7 +3394,7 @@
    int err = generic_ide_ioctl(bdev, 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;
    }
    diff -ru linux-2.6.8-rc2.orig/drivers/scsi/sr.c linux-2.6.8-rc2/drivers/scsi/sr.c
    --- linux-2.6.8-rc2.orig/drivers/scsi/sr.c 2004-08-02 19:58:24.000000000 -0500
    +++ linux-2.6.8-rc2/drivers/scsi/sr.c 2004-08-02 20:17:23.000000000 -0500
    @@ -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)
    diff -ru linux-2.6.8-rc2.orig/include/linux/cdrom.h linux-2.6.8-rc2/include/linux/cdrom.h
    --- linux-2.6.8-rc2.orig/include/linux/cdrom.h 2004-06-16 00:18:52.000000000 -0500
    +++ linux-2.6.8-rc2/include/linux/cdrom.h 2004-08-02 20:20:09.000000000 -0500
    @@ -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);
    \
     
     \ /
      Last update: 2005-03-22 14:04    [from the cache]
    ©2003-2014 Jasper Spaans. hosted at Digital Ocean