[RFC] Let non-root users eject their ipods?

[RFC] Let non-root users eject their ipods?

[john stultz]

All,
	I'm getting a little tired of my roommates not knowing how to safely
eject their usb-flash disks from my system and I'd personally like it if
I could avoid bringing up a root shell to eject my ipod. Sure, one could
suid the eject command, but that seems just as bad as changing the
permissions in the kernel (eject wouldn't be able to check if the user
has read/write permissions on the device, allowing them to eject
anything).

I've looked around trying to find some references to why this isn't
currently allowed or how safe this is, but I couldn't find anything
except the 2.6.8/k3b thread from awhile back and it didn't speak to why
eject would need root permissions even if the user has r/w permissions
on the device.

I really know nothing about scsi ioctls, so this is probably the wrong
solution, but I figured I'd offer my head upon a stake so others could
learn what not to do and why, and maybe start some discussion on what
the proper fix should be (for the kernel or the distributions to make)
since non root users really should be able to eject the flash disk they
just plugged in.

So below is a patch that allows non-root users to eject their ipods. (It
seems it should be safe_for_write() but eject opens the device for
RDONLY, so eject may be wrong here as well). 

Comments, flames?

thanks
-john

diff --git a/block/scsi_ioctl.c b/block/scsi_ioctl.c
--- a/block/scsi_ioctl.c
+++ b/block/scsi_ioctl.c
@@ -188,6 +188,9 @@ static int verify_command(struct file *f
 		safe_for_write(GPCMD_PREVENT_ALLOW_MEDIUM_REMOVAL),
 		safe_for_write(GPCMD_LOAD_UNLOAD),
 		safe_for_write(GPCMD_SET_STREAMING),
+
+		/* let me eject my damn ipod */
+		safe_for_read(ALLOW_MEDIUM_REMOVAL),
 	};
 	unsigned char type = cmd_type[cmd[0]];
 

Re: [RFC] Let non-root users eject their ipods?

[Wakko Warner]

john stultz wrote:
> All,
> 	I'm getting a little tired of my roommates not knowing how to safely
> eject their usb-flash disks from my system and I'd personally like it if

What exactly is ejecting flash media?

I have USB hard disks, USB Flash sticks, USB DVD-RAM, and an Ipod.  The only
one that even needs eject is the DVDRam.  IIRC, ALLOW_MEDIUM_MREMOVAL is for
CD-Rom (and possibly tape).  If the device is not in use, there's no reason
it cannot be unplugged then.  (Not in use as in not mounted, and noone's
accessing the raw device).

> diff --git a/block/scsi_ioctl.c b/block/scsi_ioctl.c
> --- a/block/scsi_ioctl.c
> +++ b/block/scsi_ioctl.c
> @@ -188,6 +188,9 @@ static int verify_command(struct file *f
>  		safe_for_write(GPCMD_PREVENT_ALLOW_MEDIUM_REMOVAL),
>  		safe_for_write(GPCMD_LOAD_UNLOAD),
>  		safe_for_write(GPCMD_SET_STREAMING),
> +
> +		/* let me eject my damn ipod */
> +		safe_for_read(ALLOW_MEDIUM_REMOVAL),
>  	};
>  	unsigned char type = cmd_type[cmd[0]];
>  
> 
> 
> -
> 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/
-- 
 Lab tests show that use of micro$oft causes cancer in lab animals
 Got Gas???

Re: [RFC] Let non-root users eject their ipods?

[john stultz]

On Mon, 2005-12-19 at 22:51 -0500, Wakko Warner wrote:
> john stultz wrote:
> > All,
> > 	I'm getting a little tired of my roommates not knowing how to safely
> > eject their usb-flash disks from my system and I'd personally like it if
> 
> What exactly is ejecting flash media?
> 
> I have USB hard disks, USB Flash sticks, USB DVD-RAM, and an Ipod.  The only
> one that even needs eject is the DVDRam.  IIRC, ALLOW_MEDIUM_MREMOVAL is for
> CD-Rom (and possibly tape).  If the device is not in use, there's no reason
> it cannot be unplugged then.  (Not in use as in not mounted, and noone's
> accessing the raw device).

Again, I'm not much of a SCSI person, so you might be right here.
However, ejecting scsi devices, like firewire or usb disks, tends to
spin the devices down. This, to my understanding, allows for safe
removal (ipods stop flashing the "do not remove" message). 

On USB flash disks, I'd hope umounting the device would suffice in
flushing out any writes, but it seems quite a bit of writing can go on
when the eject command is issued. It might be I'm just being paranoid,
but I've always ejected flash drives as well just to be sure.

thanks
-john

Re: [RFC] Let non-root users eject their ipods?

[Matthew Dharm]

[-- Attachment #1: Type: text/plain, Size: 2125 bytes --]

On Mon, Dec 19, 2005 at 07:49:18PM -0800, john stultz wrote:
> On Mon, 2005-12-19 at 22:51 -0500, Wakko Warner wrote:
> > john stultz wrote:
> > > All,
> > > 	I'm getting a little tired of my roommates not knowing how to safely
> > > eject their usb-flash disks from my system and I'd personally like it if
> > 
> > What exactly is ejecting flash media?
> > 
> > I have USB hard disks, USB Flash sticks, USB DVD-RAM, and an Ipod.  The only
> > one that even needs eject is the DVDRam.  IIRC, ALLOW_MEDIUM_MREMOVAL is for
> > CD-Rom (and possibly tape).  If the device is not in use, there's no reason
> > it cannot be unplugged then.  (Not in use as in not mounted, and noone's
> > accessing the raw device).
> 
> Again, I'm not much of a SCSI person, so you might be right here.
> However, ejecting scsi devices, like firewire or usb disks, tends to
> spin the devices down. This, to my understanding, allows for safe
> removal (ipods stop flashing the "do not remove" message). 

It turns out that a lot of USB devices look for ALLOW_MEDIUM_REMOVAL for
various purposes, including flushing internal buffer-cache.  Several CF
readers do this, some "fixed" flash drives do this, etc.

Also, things like the iPod and others (and this group overlaps with the
aforementioned types of devices) give the user a visible indication that
_this_particular_ unit is ready to be removed.

> On USB flash disks, I'd hope umounting the device would suffice in
> flushing out any writes, but it seems quite a bit of writing can go on
> when the eject command is issued. It might be I'm just being paranoid,
> but I've always ejected flash drives as well just to be sure.

The eject shouldn't flush anything from the kernel-level, but it won't
complete until the device's internal buffers are flushed.

Matt

-- 
Matthew Dharm                              Home: mdharm-usb@one-eyed-alien.net 
Maintainer, Linux USB Mass Storage Driver

E:  You run this ship with Windows?!  YOU IDIOT!
L:  Give me a break, it came bundled with the computer!
					-- ESR and Lan Solaris
User Friendly, 12/8/1998

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [RFC] Let non-root users eject their ipods?

[Jan Engelhardt]

>> All,
>> 	I'm getting a little tired of my roommates not knowing how to safely
>> eject their usb-flash disks from my system and I'd personally like it if
>
>What exactly is ejecting flash media?

  (That's when the spring in the usb connector decides it had enough and 
  spits the ipod cable out. It is advised to wear a helmet to not get hurt 
  by the HV usb device, e.g. usb mass storage sticks. j/k)

Ejecting flash media is what `sdparm -C eject /dev/sdX` does. In case of 
cdroms, that's eject. In case of zip drives, I guess, it will be - see 
above - unloading zip disks via springs (now these things really eject 
hard), and in case of mass storage drives that can usually not be ejected 
(so-called "non-removable media", such as harddisks), it disconnects them. 
Another stage after umount, so to speak. (And BTW, you cannot run `sdparm 
-C load` on these usb media again; at least I cannot for my memory stick.)


Jan Engelhardt
-- 

Re: [RFC] Let non-root users eject their ipods?

[Willy Tarreau]

Hi John,

On Mon, Dec 19, 2005 at 06:51:58PM -0800, john stultz wrote:
> All,
> 	I'm getting a little tired of my roommates not knowing how to safely
> eject their usb-flash disks from my system and I'd personally like it if
> I could avoid bringing up a root shell to eject my ipod. Sure, one could
> suid the eject command, but that seems just as bad as changing the
> permissions in the kernel (eject wouldn't be able to check if the user
> has read/write permissions on the device, allowing them to eject
> anything).

You may find my question stupid, but what is wrong with umount ? That's
how I proceed with usb-flash and I've never sent any eject command to
it (I even didn't know that the ioctl would be accepted by an sd device).

> I've looked around trying to find some references to why this isn't
> currently allowed or how safe this is, but I couldn't find anything
> except the 2.6.8/k3b thread from awhile back and it didn't speak to why
> eject would need root permissions even if the user has r/w permissions
> on the device.
> 
> I really know nothing about scsi ioctls, so this is probably the wrong
> solution, but I figured I'd offer my head upon a stake so others could
> learn what not to do and why, and maybe start some discussion on what
> the proper fix should be (for the kernel or the distributions to make)
> since non root users really should be able to eject the flash disk they
> just plugged in.
>
> So below is a patch that allows non-root users to eject their ipods. (It
> seems it should be safe_for_write() but eject opens the device for
> RDONLY, so eject may be wrong here as well). 

If there is a special ioctl to be called after the device has been
unmounted, then probably it would be easier to call it in umount() ?
The advantage is that mount/umount are already suid on distros which
allow user access, and you just have to put a 'users' option in the
fstab for this.

> Comments, flames?
> 
> thanks
> -john

Cheers,
Willy


> 
> diff --git a/block/scsi_ioctl.c b/block/scsi_ioctl.c
> --- a/block/scsi_ioctl.c
> +++ b/block/scsi_ioctl.c
> @@ -188,6 +188,9 @@ static int verify_command(struct file *f
>  		safe_for_write(GPCMD_PREVENT_ALLOW_MEDIUM_REMOVAL),
>  		safe_for_write(GPCMD_LOAD_UNLOAD),
>  		safe_for_write(GPCMD_SET_STREAMING),
> +
> +		/* let me eject my damn ipod */
> +		safe_for_read(ALLOW_MEDIUM_REMOVAL),
>  	};
>  	unsigned char type = cmd_type[cmd[0]];
>  
> 
> 
> -
> 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/

Re: [RFC] Let non-root users eject their ipods?

[Coywolf Qi Hunt]

2005/12/20, Willy Tarreau <willy@w.ods.org>:
> Hi John,
>
> On Mon, Dec 19, 2005 at 06:51:58PM -0800, john stultz wrote:
> > All,
> >       I'm getting a little tired of my roommates not knowing how to safely
> > eject their usb-flash disks from my system and I'd personally like it if
> > I could avoid bringing up a root shell to eject my ipod. Sure, one could
> > suid the eject command, but that seems just as bad as changing the
> > permissions in the kernel (eject wouldn't be able to check if the user
> > has read/write permissions on the device, allowing them to eject
> > anything).
>
> You may find my question stupid, but what is wrong with umount ? That's
> how I proceed with usb-flash and I've never sent any eject command to
> it (I even didn't know that the ioctl would be accepted by an sd device).

IMHO, umount doesn't guarantee sync, isn't it? That's similar to why
users should press sysrq-s after sysrq-u.

(CC: Petr Vandrovec, Anton Altaparmakov, Andrew Morton)

-- coywolf

Re: [RFC] Let non-root users eject their ipods?

[Sander]

Coywolf Qi Hunt wrote (ao):
> 2005/12/20, Willy Tarreau <willy@w.ods.org>:
> > On Mon, Dec 19, 2005 at 06:51:58PM -0800, john stultz wrote:
> > >       I'm getting a little tired of my roommates not knowing how to safely
> > > eject their usb-flash disks from my system and I'd personally like it if
> > > I could avoid bringing up a root shell to eject my ipod. Sure, one could
> > > suid the eject command, but that seems just as bad as changing the
> > > permissions in the kernel (eject wouldn't be able to check if the user
> > > has read/write permissions on the device, allowing them to eject
> > > anything).
> >
> > You may find my question stupid, but what is wrong with umount ? That's
> > how I proceed with usb-flash and I've never sent any eject command to
> > it (I even didn't know that the ioctl would be accepted by an sd device).
> 
> IMHO, umount doesn't guarantee sync, isn't it?

I'm pretty sure it does :-)

Anyway, that is how I treat all usb/firewire disks, and I've never lost
data. Just umount and unplug when the prompt returns.

-- 
Humilis IT Services and Solutions
http://www.humilis.net

Re: [RFC] Let non-root users eject their ipods?

[Coywolf Qi Hunt]

2005/12/20, Sander <sander@humilis.net>:
> Coywolf Qi Hunt wrote (ao):
> > 2005/12/20, Willy Tarreau <willy@w.ods.org>:
> > > On Mon, Dec 19, 2005 at 06:51:58PM -0800, john stultz wrote:
> > > >       I'm getting a little tired of my roommates not knowing how to safely
> > > > eject their usb-flash disks from my system and I'd personally like it if
> > > > I could avoid bringing up a root shell to eject my ipod. Sure, one could
> > > > suid the eject command, but that seems just as bad as changing the
> > > > permissions in the kernel (eject wouldn't be able to check if the user
> > > > has read/write permissions on the device, allowing them to eject
> > > > anything).
> > >
> > > You may find my question stupid, but what is wrong with umount ? That's
> > > how I proceed with usb-flash and I've never sent any eject command to
> > > it (I even didn't know that the ioctl would be accepted by an sd device).
> >
> > IMHO, umount doesn't guarantee sync, isn't it?

Actually I was think umount(2), since this is the kernel list, but off
topic here.

>
> I'm pretty sure it does :-)

That is because: usually your removable media is not the file system
root, hence umount(8) can return successfully only if no processes are
busy working on it.

If you boot from or chroot/pivot into a removable media, and you
remount it ro, and unplug it, then you may lose data.

-- coywolf

>
> Anyway, that is how I treat all usb/firewire disks, and I've never lost
> data. Just umount and unplug when the prompt returns.
>
> --
> Humilis IT Services and Solutions
> http://www.humilis.net
>

Re: [RFC] Let non-root users eject their ipods?

[Sander]

Coywolf Qi Hunt wrote (ao):
> 2005/12/20, Sander <sander@humilis.net>:
> > Coywolf Qi Hunt wrote (ao):
> > > 2005/12/20, Willy Tarreau <willy@w.ods.org>:
> > > > On Mon, Dec 19, 2005 at 06:51:58PM -0800, john stultz wrote:
> > > > >       I'm getting a little tired of my roommates not knowing how to safely
> > > > > eject their usb-flash disks from my system and I'd personally like it if
> > > > > I could avoid bringing up a root shell to eject my ipod. Sure, one could
> > > > > suid the eject command, but that seems just as bad as changing the
> > > > > permissions in the kernel (eject wouldn't be able to check if the user
> > > > > has read/write permissions on the device, allowing them to eject
> > > > > anything).
> > > >
> > > > You may find my question stupid, but what is wrong with umount ? That's
> > > > how I proceed with usb-flash and I've never sent any eject command to
> > > > it (I even didn't know that the ioctl would be accepted by an sd device).
> > >
> > > IMHO, umount doesn't guarantee sync, isn't it?
> 
> Actually I was think umount(2), since this is the kernel list, but off
> topic here.
> 
> >
> > I'm pretty sure it does :-)
> 
> That is because: usually your removable media is not the file system
> root, hence umount(8) can return successfully only if no processes are
> busy working on it.
> 
> If you boot from or chroot/pivot into a removable media, and you
> remount it ro, and unplug it, then you may lose data.
 
eject wont help you here, right?

And the OP was talking about usb-flash sticks his roommates use and his
ipod. He doesn't need to eject those. umount will do.

-- 
Humilis IT Services and Solutions
http://www.humilis.net

Re: [RFC] Let non-root users eject their ipods?

[Bill Davidsen]

Sander wrote:
> Coywolf Qi Hunt wrote (ao):
> 
>>2005/12/20, Sander <sander@humilis.net>:
>>
>>>Coywolf Qi Hunt wrote (ao):
>>>
>>>>2005/12/20, Willy Tarreau <willy@w.ods.org>:
>>>>
>>>>>On Mon, Dec 19, 2005 at 06:51:58PM -0800, john stultz wrote:
>>>>>
>>>>>>      I'm getting a little tired of my roommates not knowing how to safely
>>>>>>eject their usb-flash disks from my system and I'd personally like it if
>>>>>>I could avoid bringing up a root shell to eject my ipod. Sure, one could
>>>>>>suid the eject command, but that seems just as bad as changing the
>>>>>>permissions in the kernel (eject wouldn't be able to check if the user
>>>>>>has read/write permissions on the device, allowing them to eject
>>>>>>anything).
>>>>>
>>>>>You may find my question stupid, but what is wrong with umount ? That's
>>>>>how I proceed with usb-flash and I've never sent any eject command to
>>>>>it (I even didn't know that the ioctl would be accepted by an sd device).
>>>>
>>>>IMHO, umount doesn't guarantee sync, isn't it?
>>
>>Actually I was think umount(2), since this is the kernel list, but off
>>topic here.
>>
>>
>>>I'm pretty sure it does :-)
>>
>>That is because: usually your removable media is not the file system
>>root, hence umount(8) can return successfully only if no processes are
>>busy working on it.
>>
>>If you boot from or chroot/pivot into a removable media, and you
>>remount it ro, and unplug it, then you may lose data.
> 
>  
> eject wont help you here, right?
> 
> And the OP was talking about usb-flash sticks his roommates use and his
> ipod. He doesn't need to eject those. umount will do.
> 
Using umount still leaves the iPod flashing a "do not disconnect" 
message as I recall, while eject clears it. So while umount may be all 
the o/s needs, and all some external storage media need, it may be 
highly desirable to do the eject for the benefit of the attached device, 
to cue it to finish whatever it's caching internally. Whatever eject 
does clearly is device visible, and in the case of iPod the device 
objects if it isn't given.

I would think that allowing it on any device on which the caller has 
write permission would cover the security aspects. In this case I would 
prefer not taking the "my XXX doesn't need it" approach, and do it 
unless there's a reason not to.

Not allowing a CD/DVD burner to "prevent media removal" on a device for 
which the user has write permission is another case of questionable 
security. Since that prevents unpatched growisofs from being used by a 
user it has a real negative effect and no obvious (to me) security 
benefit. I don't think of a case where I want to pull a media as it's 
burning...

-- 
    -bill davidsen (davidsen@tmr.com)
"The secret to procrastination is to put things off until the
  last possible moment - but no longer"  -me

Re: [RFC] Let non-root users eject their ipods?

[Nikita Danilov]

Coywolf Qi Hunt writes:
 > 2005/12/20, Sander <sander@humilis.net>:
 > > Coywolf Qi Hunt wrote (ao):

[...]

 > > > IMHO, umount doesn't guarantee sync, isn't it?
 > 
 > Actually I was think umount(2), since this is the kernel list, but off
 > topic here.
 > 
 > >
 > > I'm pretty sure it does :-)
 > 
 > That is because: usually your removable media is not the file system
 > root, hence umount(8) can return successfully only if no processes are
 > busy working on it.
 > 
 > If you boot from or chroot/pivot into a removable media, and you
 > remount it ro, and unplug it, then you may lose data.

sys_umount() writes all cached data back, see generic_shutdown_super()->fsync_super()

 > 
 > -- coywolf

Nikita.

Re: [RFC] Let non-root users eject their ipods?

[Jens Axboe]

On Mon, Dec 19 2005, john stultz wrote:
> All,
> 	I'm getting a little tired of my roommates not knowing how to safely
> eject their usb-flash disks from my system and I'd personally like it if
> I could avoid bringing up a root shell to eject my ipod. Sure, one could
> suid the eject command, but that seems just as bad as changing the
> permissions in the kernel (eject wouldn't be able to check if the user
> has read/write permissions on the device, allowing them to eject
> anything).

This just came up yesterday, eject isn't opening the device RDWR hence
you have a permission problem with a command requiring write
permissions. So just fix eject, there's no need to suid eject or run it
as root.

-- 
Jens Axboe

Re: [RFC] Let non-root users eject their ipods?

[Ben Collins]

On Tue, 2005-12-20 at 08:46 +0100, Jens Axboe wrote:
> On Mon, Dec 19 2005, john stultz wrote:
> > All,
> > 	I'm getting a little tired of my roommates not knowing how to safely
> > eject their usb-flash disks from my system and I'd personally like it if
> > I could avoid bringing up a root shell to eject my ipod. Sure, one could
> > suid the eject command, but that seems just as bad as changing the
> > permissions in the kernel (eject wouldn't be able to check if the user
> > has read/write permissions on the device, allowing them to eject
> > anything).
> 
> This just came up yesterday, eject isn't opening the device RDWR hence
> you have a permission problem with a command requiring write
> permissions. So just fix eject, there's no need to suid eject or run it
> as root.

Yep, and here's the patch to do it:

http://bugzilla.ubuntu.com/attachment.cgi?id=5415

Still, this whole issue with ALLOW_MEDIUM_REMOVAL. Would be nice if
CDROMEJECT just did the right thing.

-- 
   Ben Collins <ben.collins@ubuntu.com>
   Developer
   Ubuntu Linux

Re: [RFC] Let non-root users eject their ipods?

[Jens Axboe]

On Tue, Dec 20 2005, Ben Collins wrote:
> On Tue, 2005-12-20 at 08:46 +0100, Jens Axboe wrote:
> > On Mon, Dec 19 2005, john stultz wrote:
> > > All,
> > > 	I'm getting a little tired of my roommates not knowing how to safely
> > > eject their usb-flash disks from my system and I'd personally like it if
> > > I could avoid bringing up a root shell to eject my ipod. Sure, one could
> > > suid the eject command, but that seems just as bad as changing the
> > > permissions in the kernel (eject wouldn't be able to check if the user
> > > has read/write permissions on the device, allowing them to eject
> > > anything).
> > 
> > This just came up yesterday, eject isn't opening the device RDWR hence
> > you have a permission problem with a command requiring write
> > permissions. So just fix eject, there's no need to suid eject or run it
> > as root.
> 
> Yep, and here's the patch to do it:
> 
> http://bugzilla.ubuntu.com/attachment.cgi?id=5415
> 
> Still, this whole issue with ALLOW_MEDIUM_REMOVAL. Would be nice if
> CDROMEJECT just did the right thing.

I would tend to agree, but you are bypassing the checks that are in
there by doing so which isn't very nice. Then we might as well just mark
ALLOW_MEDIUM_REMOVAL as safe-for-read in the first place.

-- 
Jens Axboe

Re: [RFC] Let non-root users eject their ipods?

[Ben Collins]

On Tue, 2005-12-20 at 14:28 +0100, Jens Axboe wrote:
> On Tue, Dec 20 2005, Ben Collins wrote:
> > On Tue, 2005-12-20 at 08:46 +0100, Jens Axboe wrote:
> > > On Mon, Dec 19 2005, john stultz wrote:
> > > > All,
> > > > 	I'm getting a little tired of my roommates not knowing how to safely
> > > > eject their usb-flash disks from my system and I'd personally like it if
> > > > I could avoid bringing up a root shell to eject my ipod. Sure, one could
> > > > suid the eject command, but that seems just as bad as changing the
> > > > permissions in the kernel (eject wouldn't be able to check if the user
> > > > has read/write permissions on the device, allowing them to eject
> > > > anything).
> > > 
> > > This just came up yesterday, eject isn't opening the device RDWR hence
> > > you have a permission problem with a command requiring write
> > > permissions. So just fix eject, there's no need to suid eject or run it
> > > as root.
> > 
> > Yep, and here's the patch to do it:
> > 
> > http://bugzilla.ubuntu.com/attachment.cgi?id=5415
> > 
> > Still, this whole issue with ALLOW_MEDIUM_REMOVAL. Would be nice if
> > CDROMEJECT just did the right thing.
> 
> I would tend to agree, but you are bypassing the checks that are in
> there by doing so which isn't very nice. Then we might as well just mark
> ALLOW_MEDIUM_REMOVAL as safe-for-read in the first place.

Should be an easy check to add. In fact, I'll resend both patches with
that in place if you want.

-- 
   Ben Collins <ben.collins@ubuntu.com>
   Developer
   Ubuntu Linux

Re: [RFC] Let non-root users eject their ipods?

[Jens Axboe]

On Tue, Dec 20 2005, Ben Collins wrote:
> On Tue, 2005-12-20 at 14:28 +0100, Jens Axboe wrote:
> > On Tue, Dec 20 2005, Ben Collins wrote:
> > > On Tue, 2005-12-20 at 08:46 +0100, Jens Axboe wrote:
> > > > On Mon, Dec 19 2005, john stultz wrote:
> > > > > All,
> > > > > 	I'm getting a little tired of my roommates not knowing how to safely
> > > > > eject their usb-flash disks from my system and I'd personally like it if
> > > > > I could avoid bringing up a root shell to eject my ipod. Sure, one could
> > > > > suid the eject command, but that seems just as bad as changing the
> > > > > permissions in the kernel (eject wouldn't be able to check if the user
> > > > > has read/write permissions on the device, allowing them to eject
> > > > > anything).
> > > > 
> > > > This just came up yesterday, eject isn't opening the device RDWR hence
> > > > you have a permission problem with a command requiring write
> > > > permissions. So just fix eject, there's no need to suid eject or run it
> > > > as root.
> > > 
> > > Yep, and here's the patch to do it:
> > > 
> > > http://bugzilla.ubuntu.com/attachment.cgi?id=5415
> > > 
> > > Still, this whole issue with ALLOW_MEDIUM_REMOVAL. Would be nice if
> > > CDROMEJECT just did the right thing.
> > 
> > I would tend to agree, but you are bypassing the checks that are in
> > there by doing so which isn't very nice. Then we might as well just mark
> > ALLOW_MEDIUM_REMOVAL as safe-for-read in the first place.
> 
> Should be an easy check to add. In fact, I'll resend both patches with
> that in place if you want.

There's still the quirky problem of forcing a locked tray out. In some
cases this is what you want, if things get stuck for some reason or
another. But usually the tray is locked for a good reason, because there
are active users of the device.

Say two processes has the cdrom open, one of them doing io (maybe even
writing!), the other could do a CDROMEJECT now and force the ejection of
a busy drive.

-- 
Jens Axboe

[PATCH] block: Better CDROMEJECT

[Ben Collins]

On Tue, 2005-12-20 at 14:39 +0100, Jens Axboe wrote:
> > Should be an easy check to add. In fact, I'll resend both patches with
> > that in place if you want.
> 
> There's still the quirky problem of forcing a locked tray out. In some
> cases this is what you want, if things get stuck for some reason or
> another. But usually the tray is locked for a good reason, because there
> are active users of the device.
> 
> Say two processes has the cdrom open, one of them doing io (maybe even
> writing!), the other could do a CDROMEJECT now and force the ejection of
> a busy drive.

But that's possible now with "eject -s" as long as you have write access
to it. Most users are using "eject -s" anyway.

You can't stop this from happening. However, the fact is that a lot of
devices (iPod's being the most popular) require this to work.

Here's the patch. Currently it will not even try the
ALLOW_MEDIUM_REMOVAL unless they have write access (to avoid returning
an uneeded error for people using eject -r that isn't patched to open
the device O_RDRW). However, I still changed the __blk_send_generic()
function to use verify_command().

Signed-off-by: Ben Collins <bcollins@ubuntu.com>

diff --git a/block/scsi_ioctl.c b/block/scsi_ioctl.c
index 382dea7..df259f7 100644
--- a/block/scsi_ioctl.c
+++ b/block/scsi_ioctl.c
@@ -442,11 +442,54 @@ error:
 	return err;
 }
 
+
+/* Send basic block requests */
+static int __blk_send_generic(struct file *file, request_queue_t *q, struct gendisk *bd_disk, int cmd, int data)
+{
+	struct request *rq;
+	int err;
+
+	rq = blk_get_request(q, READ, __GFP_WAIT);
+	rq->flags |= REQ_BLOCK_PC;
+	rq->data = NULL;
+	rq->data_len = 0;
+	rq->timeout = BLK_DEFAULT_TIMEOUT;
+	memset(rq->cmd, 0, sizeof(rq->cmd));
+
+	rq->cmd[0] = cmd;
+	rq->cmd[4] = data;
+	rq->cmd_len = 6;
+
+	if (file) {
+		err = verify_command(file, rq->cmd);
+		if (err)
+			goto send_error;
+	}
+	
+	err = blk_execute_rq(q, bd_disk, rq, 0);
+
+send_error:
+	blk_put_request(rq);
+
+	return err;
+}
+
+/* START_STOP just needs read only, so safe */
+static inline int blk_send_start_stop(request_queue_t *q, struct gendisk *bd_disk, int data)
+{
+	return __blk_send_generic(NULL, q, bd_disk, GPCMD_START_STOP_UNIT, data);
+}
+
+/* ALLOW_MEDIUM_REMOVAL needs write access */
+static inline int blk_send_allow_medium_removal(struct file *file, request_queue_t *q, struct gendisk *bd_disk)
+{
+	return __blk_send_generic(file, q, bd_disk, GPCMD_PREVENT_ALLOW_MEDIUM_REMOVAL, 0);
+}
+
 int scsi_cmd_ioctl(struct file *file, struct gendisk *bd_disk, unsigned int cmd, void __user *arg)
 {
 	request_queue_t *q;
-	struct request *rq;
-	int close = 0, err;
+	int err;
 
 	q = bd_disk->queue;
 	if (!q)
@@ -564,19 +607,20 @@ int scsi_cmd_ioctl(struct file *file, st
 			err = sg_scsi_ioctl(file, q, bd_disk, arg);
 			break;
 		case CDROMCLOSETRAY:
-			close = 1;
+			err = blk_send_start_stop(q, bd_disk, 0x03);
+			break;
 		case CDROMEJECT:
-			rq = blk_get_request(q, WRITE, __GFP_WAIT);
-			rq->flags |= REQ_BLOCK_PC;
-			rq->data = NULL;
-			rq->data_len = 0;
-			rq->timeout = BLK_DEFAULT_TIMEOUT;
-			memset(rq->cmd, 0, sizeof(rq->cmd));
-			rq->cmd[0] = GPCMD_START_STOP_UNIT;
-			rq->cmd[4] = 0x02 + (close != 0);
-			rq->cmd_len = 6;
-			err = blk_execute_rq(q, bd_disk, rq, 0);
-			blk_put_request(rq);
+			err = 0;
+
+			/* We don't even want to try the
+			 * ALLOW_MEDIUM_REMOVAL unless the user has write
+			 * access. The START_STOP will fail if the drive
+			 * needs ALLOW_MEDIUM_REMOVAL anyway. */
+			if (!(file->f_mode & FMODE_WRITE)) {
+				err |= blk_send_allow_medium_removal(file, q, bd_disk);
+				err |= blk_send_start_stop(q, bd_disk, 0x01);
+			}
+			err |= blk_send_start_stop(q, bd_disk, 0x02);
 			break;
 		default:
 			err = -ENOTTY;


-- 
   Ben Collins <ben.collins@ubuntu.com>
   Developer
   Ubuntu Linux

Re: [PATCH] block: Better CDROMEJECT

[Jens Axboe]

On Tue, Dec 20 2005, Ben Collins wrote:
> On Tue, 2005-12-20 at 14:39 +0100, Jens Axboe wrote:
> > > Should be an easy check to add. In fact, I'll resend both patches with
> > > that in place if you want.
> > 
> > There's still the quirky problem of forcing a locked tray out. In some
> > cases this is what you want, if things get stuck for some reason or
> > another. But usually the tray is locked for a good reason, because there
> > are active users of the device.
> > 
> > Say two processes has the cdrom open, one of them doing io (maybe even
> > writing!), the other could do a CDROMEJECT now and force the ejection of
> > a busy drive.
> 
> But that's possible now with "eject -s" as long as you have write access
> to it. Most users are using "eject -s" anyway.
> 
> You can't stop this from happening. However, the fact is that a lot of
> devices (iPod's being the most popular) require this to work.

Well just because it (unfortunately) is already done by some other path,
doesn't make it a good idea by default and necessarily a reason to
further such bad principles.

The only way to really fix this would be requiring programs issuing
direct commands to a device to have it opened O_EXCL (and making sure
this actually works, right now it doesn't).

In reality, we are probably screwed :/

-- 
Jens Axboe

Re: [PATCH] block: Better CDROMEJECT

[john stultz]

On Tue, 2005-12-20 at 09:07 -0500, Ben Collins wrote:
> On Tue, 2005-12-20 at 14:39 +0100, Jens Axboe wrote:
> > > Should be an easy check to add. In fact, I'll resend both patches with
> > > that in place if you want.
> > 
> > There's still the quirky problem of forcing a locked tray out. In some
> > cases this is what you want, if things get stuck for some reason or
> > another. But usually the tray is locked for a good reason, because there
> > are active users of the device.
> > 
> > Say two processes has the cdrom open, one of them doing io (maybe even
> > writing!), the other could do a CDROMEJECT now and force the ejection of
> > a busy drive.
> 
> But that's possible now with "eject -s" as long as you have write access
> to it. Most users are using "eject -s" anyway.
> 
> You can't stop this from happening. However, the fact is that a lot of
> devices (iPod's being the most popular) require this to work.

I'm a little confused. Eject has a number of different ways it
interfaces with the kernel (scsi, cdrom, floppy, tape), which I assume
map to different ioctl commands. In the case I'm familiar with (my usb
ipod, and my firewire disk), the scsi method (via eject -s) is used
which sends a ALLOW_MEDIUM_REMOVAL.

Now I know without passing a specific method, eject will try different
methods until one works, but it seems that the patch below is overriding
the CDROMEJECT ioctl so that it then sends an ALLOW_MEDIUM_REMOVAL as
well as the normal GPCMD_START_STOP_UNIT.

Again, I don't know about the hardware bits, do you want
ALLOW_MEDIUM_REMOVAL to be sent to cdroms when the "eject -r" option is
used, or just for "eject -s"?

Or maybe your patch is addressing more then just my issue w/ USB and
Firewire devices and that is the disconnect on my side?

> Here's the patch. Currently it will not even try the
> ALLOW_MEDIUM_REMOVAL unless they have write access (to avoid returning
> an uneeded error for people using eject -r that isn't patched to open
> the device O_RDRW). However, I still changed the __blk_send_generic()
> function to use verify_command().

I'll play with your patch tonight and let you know how it goes.

Although from just looking at it, don't you still need to add
ALLOW_MEDIUM_REMOVAL in the verify_command() list for this to work?

Alternatively, would just the "safe_for_write(ALLOW_MEDIUM_REMOVAL);" in
verify_command along with the eject-opens-RW fix have almost the same
effect?

thanks
-john

Re: [PATCH] block: Better CDROMEJECT

[Jens Axboe]

On Tue, Dec 20 2005, john stultz wrote:
> Although from just looking at it, don't you still need to add
> ALLOW_MEDIUM_REMOVAL in the verify_command() list for this to work?
> 
> Alternatively, would just the "safe_for_write(ALLOW_MEDIUM_REMOVAL);" in
> verify_command along with the eject-opens-RW fix have almost the same
> effect?

The command is already in the safe-for-write list, so you don't have to
change anything but fix eject to open the device O_RDWR.

-- 
Jens Axboe

Re: [PATCH] block: Better CDROMEJECT

[john stultz]

On Tue, 2005-12-20 at 21:54 +0100, Jens Axboe wrote:
> On Tue, Dec 20 2005, john stultz wrote:
> > Although from just looking at it, don't you still need to add
> > ALLOW_MEDIUM_REMOVAL in the verify_command() list for this to work?
> > 
> > Alternatively, would just the "safe_for_write(ALLOW_MEDIUM_REMOVAL);" in
> > verify_command along with the eject-opens-RW fix have almost the same
> > effect?
> 
> The command is already in the safe-for-write list, so you don't have to
> change anything but fix eject to open the device O_RDWR.

Errr? I don't see it in verify_command() from Linus' current git tree.

Is there some other name for the same command?

thanks
-john

Re: [PATCH] block: Better CDROMEJECT

[Jens Axboe]

On Tue, Dec 20 2005, john stultz wrote:
> On Tue, 2005-12-20 at 21:54 +0100, Jens Axboe wrote:
> > On Tue, Dec 20 2005, john stultz wrote:
> > > Although from just looking at it, don't you still need to add
> > > ALLOW_MEDIUM_REMOVAL in the verify_command() list for this to work?
> > > 
> > > Alternatively, would just the "safe_for_write(ALLOW_MEDIUM_REMOVAL);" in
> > > verify_command along with the eject-opens-RW fix have almost the same
> > > effect?
> > 
> > The command is already in the safe-for-write list, so you don't have to
> > change anything but fix eject to open the device O_RDWR.
> 
> Errr? I don't see it in verify_command() from Linus' current git tree.
> 
> Is there some other name for the same command?

It's listed as GPCMD_PREVENT_ALLOW_MEDIUM_REMOVAL.

-- 
Jens Axboe

Re: [PATCH] block: Better CDROMEJECT

[john stultz]

On Tue, 2005-12-20 at 21:58 +0100, Jens Axboe wrote:
> On Tue, Dec 20 2005, john stultz wrote:
> > On Tue, 2005-12-20 at 21:54 +0100, Jens Axboe wrote:
> > > On Tue, Dec 20 2005, john stultz wrote:
> > > > Although from just looking at it, don't you still need to add
> > > > ALLOW_MEDIUM_REMOVAL in the verify_command() list for this to work?
> > > > 
> > > > Alternatively, would just the "safe_for_write(ALLOW_MEDIUM_REMOVAL);" in
> > > > verify_command along with the eject-opens-RW fix have almost the same
> > > > effect?
> > > 
> > > The command is already in the safe-for-write list, so you don't have to
> > > change anything but fix eject to open the device O_RDWR.
> > 
> > Errr? I don't see it in verify_command() from Linus' current git tree.
> > 
> > Is there some other name for the same command?
> 
> It's listed as GPCMD_PREVENT_ALLOW_MEDIUM_REMOVAL.

Ah! Ok, sorry about that! Indeed then, the eject fix appears to be all
that is needed.

Thanks for the clarification.
-john

Re: [PATCH] block: Better CDROMEJECT

[Ben Collins]

On Tue, 2005-12-20 at 12:41 -0800, john stultz wrote:
> On Tue, 2005-12-20 at 09:07 -0500, Ben Collins wrote:
> > On Tue, 2005-12-20 at 14:39 +0100, Jens Axboe wrote:
> > > > Should be an easy check to add. In fact, I'll resend both patches with
> > > > that in place if you want.
> > > 
> > > There's still the quirky problem of forcing a locked tray out. In some
> > > cases this is what you want, if things get stuck for some reason or
> > > another. But usually the tray is locked for a good reason, because there
> > > are active users of the device.
> > > 
> > > Say two processes has the cdrom open, one of them doing io (maybe even
> > > writing!), the other could do a CDROMEJECT now and force the ejection of
> > > a busy drive.
> > 
> > But that's possible now with "eject -s" as long as you have write access
> > to it. Most users are using "eject -s" anyway.
> > 
> > You can't stop this from happening. However, the fact is that a lot of
> > devices (iPod's being the most popular) require this to work.
> 
> I'm a little confused. Eject has a number of different ways it
> interfaces with the kernel (scsi, cdrom, floppy, tape), which I assume
> map to different ioctl commands. In the case I'm familiar with (my usb
> ipod, and my firewire disk), the scsi method (via eject -s) is used
> which sends a ALLOW_MEDIUM_REMOVAL.
> 
> Now I know without passing a specific method, eject will try different
> methods until one works, but it seems that the patch below is overriding
> the CDROMEJECT ioctl so that it then sends an ALLOW_MEDIUM_REMOVAL as
> well as the normal GPCMD_START_STOP_UNIT.

These are MMC commands, not specific to the bus it is sitting on (be it
scsi, IDE, USB, or firewire).

The "scsi" method in eject has been using pretty much the same code path
in the kernel as CDROMEJECT for some time (probably since Linus did the
whole SG_IO thing to make burning to IDE devices without ide-scsi
possible).

In fact, one user affected by the bug I was trying to fix had an IDE
CDROM drive that would only eject with "eject -s". The "eject -s" being
"Scsi Eject" in the eject program is a misnomer really.

-- 
   Ben Collins <ben.collins@ubuntu.com>
   Developer
   Ubuntu Linux

Re: [RFC] Let non-root users eject their ipods?

[Bill Davidsen]

Jens Axboe wrote:

> There's still the quirky problem of forcing a locked tray out. In some
> cases this is what you want, if things get stuck for some reason or
> another. But usually the tray is locked for a good reason, because there
> are active users of the device.
> 
> Say two processes has the cdrom open, one of them doing io (maybe even
> writing!), the other could do a CDROMEJECT now and force the ejection of
> a busy drive.
> 
I think the whole area of permissions for locking the tray and doing 
eject need rethinking. I won't rehash what I have said before, that if I 
have write permission growisofs should be able to lock the tray.

-- 
    -bill davidsen (davidsen@tmr.com)
"The secret to procrastination is to put things off until the
  last possible moment - but no longer"  -me

Re: [RFC] Let non-root users eject their ipods?

[Alan Cox]

On Llu, 2005-12-19 at 18:51 -0800, john stultz wrote:
> So below is a patch that allows non-root users to eject their ipods. (It
> seems it should be safe_for_write() but eject opens the device for
> RDONLY, so eject may be wrong here as well). 
> 
> Comments, flames?

I think its probably uninteresting to the majority of users to solve it
that way (not that its wrong that I can see). The desktops handle
automount/umount these days and if anything what would cover most bases
is to teach umount a new option/fstab flag so that it will handle the
device eject as it handles the non-root umount management.

Re: [RFC] Let non-root users eject their ipods?

[john stultz]

On Thu, 2005-12-22 at 10:56 +0000, Alan Cox wrote:
> On Llu, 2005-12-19 at 18:51 -0800, john stultz wrote:
> > So below is a patch that allows non-root users to eject their ipods. (It
> > seems it should be safe_for_write() but eject opens the device for
> > RDONLY, so eject may be wrong here as well). 
> > 
> > Comments, flames?
> 
> I think its probably uninteresting to the majority of users to solve it
> that way (not that its wrong that I can see). The desktops handle
> automount/umount these days and if anything what would cover most bases
> is to teach umount a new option/fstab flag so that it will handle the
> device eject as it handles the non-root umount management.

I don't think that's necessary as I believe the desktops handle the
mount/unmount using eject (at least Gnome does on my Ubuntu system).

After using Ben's fix (suggested by Jens I believe) for the eject
command so it opens the device RDWR everything works without kernel
modifications (ends up the ALLOW_DEVICE_REMOVAL is already on the
safe_for_write list, just under a different name).

thanks
-john

Re: [RFC] Let non-root users eject their ipods?

[Jan Engelhardt]

>> So below is a patch that allows non-root users to eject their ipods. (It
>> seems it should be safe_for_write() but eject opens the device for
>> RDONLY, so eject may be wrong here as well). 
>> 
>> Comments, flames?
>
>I think its probably uninteresting to the majority of users to solve it
>that way (not that its wrong that I can see). The desktops handle
>automount/umount these days [...]

Don't forget about the folks that don't run udev. :D



Jan Engelhardt
-- 

Re: [RFC] Let non-root users eject their ipods?

[Bodo Eggert]

Bill Davidsen <davidsen@tmr.com> wrote:

> Using umount still leaves the iPod flashing a "do not disconnect"
> message as I recall, while eject clears it. So while umount may be all
> the o/s needs, and all some external storage media need, it may be
> highly desirable to do the eject for the benefit of the attached device,
> to cue it to finish whatever it's caching internally. Whatever eject
> does clearly is device visible, and in the case of iPod the device
> objects if it isn't given.

There is an auto-eject function causing the media to be ejected on the last
user closing the device. Unfortunately it makes the device open itself
during mounts, and setting this flag will inevitably eject the media.
Additionally, it didn't work with my SCSI cdroms.

With slight changes, it could do the trick.


OTOH, we could also introduce the 'eject' mount option to let umount() eject
the media after the (last if possible) mount is gone. This will behave sane
for audio CDs, too.
-- 
Ich danke GMX dafür, die Verwendung meiner Adressen mittels per SPF
verbreiteten Lügen zu sabotieren.