Skip to contentSkip to conversation

Let non-root users eject their ipods?

RFC In review john stultz 12 participants 29 messages Dec 20, 2005 – Dec 24, 2005 Block layer

lore
Share & tools
lore
#0 lore raw

RFC 1/1 Let non-root users eject their ipods?

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

block/scsi_ioctl.c +3 -0
@@ -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]];
 
#1 lore raw
Quoted 3 lines · 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

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).

Quoted 20 lines
block/scsi_ioctl.c +3 -1
@@ -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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Signature · 2 lines
 Lab tests show that use of micro$oft causes cancer in lab animals
 Got Gas???
#2 lore raw
Quoted 5 lines · Wakko Warner
Quoted 3 lines · 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

Quoted 7 lines

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

#3 lore raw
Quoted 14 lines · john stultz
Quoted 5 lines · Wakko Warner
Quoted 3 lines · 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

Quoted 7 lines

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).

Quoted 4 lines

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.

Quoted 4 lines

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

Signature · 7 lines
Matthew Dharm                              Home: [email protected] 
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
#4 lore raw
In reply to Wakko Warner
Quoted 6 lines
Quoted 4 lines

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

#5 lore raw
In reply to john stultz

Hi John,

Quoted 8 lines · 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).

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).

Quoted 13 lines

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.

Quoted 3 lines

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.

Quoted 4 lines

Comments, flames?

thanks
-john

Cheers,
Willy

Quoted 21 lines
block/scsi_ioctl.c +3 -1
@@ -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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

#6 lore raw

2005/12/20, Willy Tarreau <[email protected]>:

Quoted 12 lines

Hi John,

Quoted 8 lines · 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).

Quoted 3 lines

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

#7 lore raw

Coywolf Qi Hunt wrote (ao):

Quoted 10 lines

2005/12/20, Willy Tarreau <[email protected]>:

Quoted 9 lines · john stultz
Quoted 7 lines

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).

Quoted 5 lines
Quoted 3 lines

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.

Signature · 2 lines
Humilis IT Services and Solutions
http://www.humilis.net
#8 lore raw

2005/12/20, Sander <[email protected]>:

Quoted 16 lines

Coywolf Qi Hunt wrote (ao):

Quoted 15 lines

2005/12/20, Willy Tarreau <[email protected]>:

Quoted 9 lines · john stultz
Quoted 7 lines

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).

Quoted 3 lines

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.

Quoted 2 lines

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

Quoted 8 lines

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

#9 lore raw

Coywolf Qi Hunt wrote (ao):

Quoted 22 lines

2005/12/20, Sander <[email protected]>:

Quoted 16 lines

Coywolf Qi Hunt wrote (ao):

Quoted 15 lines

2005/12/20, Willy Tarreau <[email protected]>:

Quoted 9 lines · john stultz
Quoted 7 lines

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).

Quoted 3 lines

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.

Quoted 1 line
Quoted 8 lines
Quoted 1 line

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.

Signature · 2 lines
Humilis IT Services and Solutions
http://www.humilis.net
#10 lore raw
Quoted 34 lines · Sander

Coywolf Qi Hunt wrote (ao):

Quoted 32 lines

2005/12/20, Sander <[email protected]>:

Quoted 19 lines

Coywolf Qi Hunt wrote (ao):

Quoted 17 lines

2005/12/20, Willy Tarreau <[email protected]>:

Quoted 10 lines · john stultz
Quoted 8 lines

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).

Quoted 3 lines

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.

Quoted 1 line

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.

Quoted 9 lines
Quoted 2 lines

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...

Signature · 3 lines
   -bill davidsen ([email protected])
"The secret to procrastination is to put things off until the
 last possible moment - but no longer"  -me
#11 lore raw
In reply to Coywolf Qi Hunt
Quoted 2 lines · Coywolf Qi Hunt

2005/12/20, Sander <[email protected]>:

Quoted 1 line

Coywolf Qi Hunt wrote (ao):

[...]

Quoted 6 lines
Quoted 1 line
Quoted 1 line

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.

Quoted 1 line
Quoted 8 lines
Quoted 1 line

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

Quoted 2 lines

-- coywolf

Nikita.

#12 lore raw
In reply to john stultz
Quoted 8 lines · 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).

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.

Signature · 1 line
Jens Axboe
#13 lore raw
Quoted 10 lines · Jens Axboe
Quoted 8 lines · 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).

Quoted 4 lines

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.

Signature · 3 lines
   Ben Collins <[email protected]>
   Developer
   Ubuntu Linux
#14 lore raw
Quoted 16 lines · Ben Collins
Quoted 10 lines · Jens Axboe
Quoted 8 lines · 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).

Quoted 4 lines

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.

Quoted 6 lines

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.

Signature · 1 line
Jens Axboe
#15 lore raw
Quoted 19 lines · Jens Axboe
Quoted 18 lines · Ben Collins
Quoted 10 lines · Jens Axboe
Quoted 8 lines · 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).

Quoted 4 lines

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:

Quoted 8 lines
Quoted 4 lines

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.

Signature · 3 lines
   Ben Collins <[email protected]>
   Developer
   Ubuntu Linux
#16 lore raw
Quoted 25 lines · Ben Collins
Quoted 24 lines · Jens Axboe
Quoted 22 lines · Ben Collins
Quoted 10 lines · Jens Axboe
Quoted 8 lines · 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).

Quoted 4 lines

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.

Quoted 6 lines
Quoted 3 lines

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.

Signature · 1 line
Jens Axboe
#17 lore raw

PATCH 1/1 block: Better CDROMEJECT

Quoted 11 lines · Jens Axboe
Quoted 2 lines

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

block/scsi_ioctl.c +58 -14
index 382dea7..df259f7 100644
@@ -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;
Signature · 3 lines
   Ben Collins <[email protected]>
   Developer
   Ubuntu Linux
#18 lore raw

Re: [PATCH] block: Better CDROMEJECT

Quoted 13 lines · Ben Collins
Quoted 11 lines · Jens Axboe
Quoted 2 lines

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.

Quoted 5 lines

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 :/

Signature · 1 line
Jens Axboe
#19 lore raw

Re: [PATCH] block: Better CDROMEJECT

In reply to Ben Collins
Quoted 13 lines · Ben Collins
Quoted 11 lines · Jens Axboe
Quoted 2 lines

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.

Quoted 5 lines

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?

Quoted 5 lines

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

#20 lore raw

Re: [PATCH] block: Better CDROMEJECT

Quoted 6 lines · john stultz

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.

Signature · 1 line
Jens Axboe
#21 lore raw

Re: [PATCH] block: Better CDROMEJECT

Quoted 10 lines · Jens Axboe
Quoted 6 lines · john stultz

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

#22 lore raw

Re: [PATCH] block: Better CDROMEJECT

Quoted 9 lines · john stultz
Quoted 8 lines · Jens Axboe
Quoted 6 lines · john stultz

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?

Quoted 6 lines
Quoted 2 lines

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.

Signature · 1 line
Jens Axboe
#23 lore raw

Re: [PATCH] block: Better CDROMEJECT

Quoted 10 lines · Jens Axboe
Quoted 9 lines · john stultz
Quoted 8 lines · Jens Axboe
Quoted 6 lines · john stultz

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?

Quoted 8 lines
Quoted 6 lines
Quoted 2 lines

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

#24 lore raw

Re: [PATCH] block: Better CDROMEJECT

In reply to john stultz
Quoted 26 lines · john stultz
Quoted 13 lines · Ben Collins
Quoted 11 lines · Jens Axboe
Quoted 2 lines

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.

Quoted 5 lines

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.

Quoted 4 lines

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.

Signature · 3 lines
   Ben Collins <[email protected]>
   Developer
   Ubuntu Linux
#25 lore raw
In reply to Jens Axboe
Quoted 9 lines · Jens Axboe

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.

Signature · 3 lines
   -bill davidsen ([email protected])
"The secret to procrastination is to put things off until the
 last possible moment - but no longer"  -me
#26 lore raw
In reply to john stultz
Quoted 5 lines · john stultz

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.

#27 lore raw
Quoted 5 lines · Alan Cox
Quoted 4 lines · john stultz

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).

Quoted 7 lines
Quoted 1 line

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

#28 lore raw
In reply to Alan Cox
Quoted 9 lines
Quoted 5 lines

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