Re: [PATCH 2/3] block: fail SCSI passthrough ioctls on partitiondevices

From: Paolo Bonzini
Date: Thu Jan 05 2012 - 08:18:49 EST


On 12/23/2011 11:46 PM, Linus Torvalds wrote:
> It sounds like people didn't even*think* of the potential issues this
> patch can bring. I'd absolutely be insane to apply them for -rc7.

Fair enough, I obviously cannot say they aren't intrusive.

Anyway, I set to change the patches to use ENOIOCTLCMD. I did some
research and found the following commit:

commit d9ecdea7ed7467db32ec160f4eca46c279255606
Author: Christoph Hellwig <hch@xxxxxx>
Date: Sat Jun 20 21:29:41 2009 +0200

virtio_blk: ioctl return value fix

Block driver ioctl methods must return ENOTTY and not -ENOIOCTLCMD if
they expect the block layer to handle generic ioctls.

This triggered a BLKROSET failure in xfsqa #200.

Signed-off-by: Christoph Hellwig <hch@xxxxxx>
Signed-off-by: Rusty Russell <rusty@xxxxxxxxxxxxxxx>

which indeed matches the current code in block/ioctl.c:

case BLKROSET:
ret = __blkdev_driver_ioctl(bdev, mode, cmd, arg);
/* -EINVAL to handle old uncorrected drivers */
if (ret != -EINVAL && ret != -ENOTTY)
return ret;
if (!capable(CAP_SYS_ADMIN))
return -EACCES;
if (get_user(n, (int __user *)(arg)))
return -EFAULT;
set_device_ro(bdev, n);
return 0;

Hence, changing scsi_verify_blk_ioctl to return ENOIOCTLCMD is not
really possible. I can make it return a boolean value, but I do not
like it: does true mean "pass this ioctl" or "forbid this ioctl"?

Would you apply the patches as they are or do you want me to squash in
something like this?

diff --git a/block/scsi_ioctl.c b/block/scsi_ioctl.c
index a6bedfe..bb94c88 100644
--- a/block/scsi_ioctl.c
+++ b/block/scsi_ioctl.c
@@ -710,6 +710,14 @@ int scsi_verify_blk_ioctl(struct block_device *bd, unsigned int cmd)
case SG_SET_RESERVED_SIZE:
case SG_EMULATED_HOST:
return 0;
+
+ case CDROMEJECT:
+ /* This is also unsafe for partition devices, but
+ * "eject /mnt/usb-drive" invokes it. Warn about it
+ * and keep backwards compatibility. */
+ printk_ratelimited(KERN_WARNING
+ "sending CDROMEJECT ioctl to a partition\n");
+ return 0;
default:
break;
}

... perhaps allowing it only for CAP_SYS_RAWIO?

Paolo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/