Just inform and dont warn when DM_DEV_REMOVE is tried on a open device

PATCH NAK Nikanth Karthikesan 2 participants 3 messages Apr 20, 2010 Device mapper

lore

Share & tools

lore

Nikanth Karthikesan

author

2010-04-20 07:16 UTC #0 lore raw

PATCH 1/1 Just inform and dont warn when DM_DEV_REMOVE is tried on a open device

Dont warn when DM_DEV_REMOVE ioctl is tried on a open device.

On openSUSE when cryptsetup and udev interact, cryptsetup sometimes tries to remove a dm device currently opened by blkid, which is not a serious issue to print a kernel warning. The application should retry if -EBUSY is returned. So let us make it an informational message instead.

Signed-off-by Nikanth Karthikesan <[email protected]>

---

drivers/md/dm-ioctl.c +1 -1

index d7500e1..dcfa48a 100644
@@ -729,7 +729,7 @@ static int dev_remove(struct dm_ioctl *param, size_t param_size)
 	 */
 	r = dm_lock_for_deletion(md);
 	if (r) {
-		DMWARN("unable to remove open device %s", hc->name);
+		DMINFO("unable to remove open device %s", hc->name);
 		up_write(&_hash_lock);
 		dm_put(md);
 		return r;

Milan Broz

NAK

2010-04-20 08:00 UTC #1 lore raw

drivers/md/dm-ioctl.c static int dev_remove(struct dm_ioctl *param, size_t param_size) · line 732

Quoted 9 lines · Nikanth Karthikesan

Dont warn when DM_DEV_REMOVE ioctl is tried on a open device.
On openSUSE when cryptsetup and udev interact, cryptsetup sometimes tries to remove a dm device currently opened by blkid, which is not a serious issue to print a kernel warning. The application should retry if -EBUSY is returned. So let us make it an informational message instead.
drivers/md/dm-ioctl.c +1 -1
-		DMWARN("unable to remove open device %s", hc->name);
+		DMINFO("unable to remove open device %s", hc->name);

NAK.

Firstly, it is security problem if someone from udev rules tries to open temporary-cryptsetup device, it contains only private key material.

Secondly, it must be solved on application level and not paper kernel using patches which hides errors.

FYI there is already patch in upstream cryptsetup which uses udev cookie mechanism. It still cannot solve artificial change events (which causes scan from rules randomly), but should help here. (will be in cryptsetup 1.1.1)

Milan

Nikanth Karthikesan

author

2010-04-20 08:43 UTC #2 lore raw

Quoted 8 lines · On Tuesday 20 April 2010 13:30:18 Milan Broz wrote:

Quoted 7 lines · Nikanth Karthikesan
Dont warn when DM_DEV_REMOVE ioctl is tried on a open device.
On openSUSE when cryptsetup and udev interact, cryptsetup sometimes tries to remove a dm device currently opened by blkid, which is not a serious issue to print a kernel warning. The application should retry if -EBUSY is returned. So let us make it an informational message instead.

Quoted 8 lines

drivers/md/dm-ioctl.c static int dev_remove(struct dm_ioctl *param, size_t param_size) · line 732
Quoted 2 lines
drivers/md/dm-ioctl.c +1 -1
-		DMWARN("unable to remove open device %s", hc->name);
+		DMINFO("unable to remove open device %s", hc->name);
NAK.
Firstly, it is security problem if someone from udev rules tries to open temporary-cryptsetup device, it contains only private key material.

Okay.

Quoted 3 lines

Secondly, it must be solved on application level and not paper kernel using patches which hides errors.

Agreed, it must be solved in the application level.

/me wonders whether this is an error worth a warning from kernel? Returning -EBUSY seems enough. May be a KERN_INFO message... But yes rest of DM prints warnings for these kind of events.

Quoted 5 lines

FYI there is already patch in upstream cryptsetup which uses udev cookie
mechanism. It still cannot solve artificial change events (which causes scan from rules randomly), but should help here. (will be in cryptsetup 1.1.1)

Oh, ok. Thanks for the pointer.

Thanks
Nikanth