On 2012-01-24 14:01 +0100, Paolo Bonzini wrote:
> You need to return -ENOTTY from scsi_verify_blk_ioctl and -ENOIOCTLCMD from
> sd_compat_ioctl, because -ENOIOCTLCMD will not be handled correctly by
> block/ioctl.c. This would break BLKROSET and BLKFLSBUF done by non-root
> but with the appropriate capabilities.
I assume this is the reason why I suddenly got lots of ioctl32 warnings
in dmesg with 3.2.2-rc1?
,----
| $ dmesg | grep ioctl | head
| [ 0.815394] ioctl32(blkid:150): Unknown cmd fd(3) cmd(00005331){t:'S';sz:0} arg(00000000) on /dev/sda1
| [ 0.815812] ioctl32(blkid:154): Unknown cmd fd(3) cmd(00005331){t:'S';sz:0} arg(00000000) on /dev/sda6
| [ 0.816184] ioctl32(blkid:151): Unknown cmd fd(3) cmd(00005331){t:'S';sz:0} arg(00000000) on /dev/sda5
| [ 0.816559] ioctl32(blkid:155): Unknown cmd fd(3) cmd(00005331){t:'S';sz:0} arg(00000000) on /dev/sda9
| [ 0.816997] ioctl32(blkid:157): Unknown cmd fd(3) cmd(00005331){t:'S';sz:0} arg(00000000) on /dev/sda8
| [ 0.817371] ioctl32(blkid:153): Unknown cmd fd(3) cmd(00005331){t:'S';sz:0} arg(00000000) on /dev/sda3
| [ 0.817692] ioctl32(blkid:156): Unknown cmd fd(3) cmd(00005331){t:'S';sz:0} arg(00000000) on /dev/sda2
| [ 0.818063] ioctl32(blkid:152): Unknown cmd fd(3) cmd(00005331){t:'S';sz:0} arg(00000000) on /dev/sda7
| [ 2.824909] ioctl32(findfs:204): Unknown cmd fd(3) cmd(00005331){t:'S';sz:0} arg(00000000) on /dev/sda6
| [ 5.545235] ioctl32(blkid:435): Unknown cmd fd(3) cmd(00005331){t:'S';sz:0} arg(00000000) on /dev/sda3
`----
> Fixed patch follows. If you prefer that I send an interdiff, let me know.
Going to try that.
> Paolo
>
> -------- 8< ---------
> From: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> Subject: [PATCH] block: fail SCSI passthrough ioctls on partition devices
>
> commit 0bfc96cb77224736dfa35c3c555d37b3646ef35e upstream.
>
> Linux allows executing the SG_IO ioctl on a partition or LVM volume, and
> will pass the command to the underlying block device. This is
> well-known, but it is also a large security problem when (via Unix
> permissions, ACLs, SELinux or a combination thereof) a program or user
> needs to be granted access only to part of the disk.
>
> This patch lets partitions forward a small set of harmless ioctls;
> others are logged with printk so that we can see which ioctls are
> actually sent. In my tests only CDROM_GET_CAPABILITY actually occurred.
> Of course it was being sent to a (partition on a) hard disk, so it would
> have failed with ENOTTY and the patch isn't changing anything in
> practice. Still, I'm treating it specially to avoid spamming the logs.
>
> In principle, this restriction should include programs running with
> CAP_SYS_RAWIO. If for example I let a program access /dev/sda2 and
> /dev/sdb, it still should not be able to read/write outside the
> boundaries of /dev/sda2 independent of the capabilities. However, for
> now programs with CAP_SYS_RAWIO will still be allowed to send the
> ioctls. Their actions will still be logged.
>
> This patch does not affect the non-libata IDE driver. That driver
> however already tests for bd != bd->bd_contains before issuing some
> ioctl; it could be restricted further to forbid these ioctls even for
> programs running with CAP_SYS_ADMIN/CAP_SYS_RAWIO.
>
> Cc: linux-scsi@xxxxxxxxxxxxxxx
> Cc: Jens Axboe <axboe@xxxxxxxxx>
> Cc: James Bottomley <JBottomley@xxxxxxxxxxxxx>
> Signed-off-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> [ Make it also print the command name when warning - Linus ]
> Signed-off-by: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
>
> [ Changes with respect to 3.3: return -ENOTTY from scsi_verify_blk_ioctl
> and -ENOIOCTLCMD from sd_compat_ioctl. ]
>
> ---
> block/scsi_ioctl.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
> drivers/scsi/sd.c | 11 +++++++++--
> include/linux/blkdev.h | 1 +
> 3 files changed, 55 insertions(+), 2 deletions(-)
>
> --- a/block/scsi_ioctl.c
> +++ b/block/scsi_ioctl.c
> @@ -24,6 +24,7 @@
> #include <linux/capability.h>
> #include <linux/completion.h>
> #include <linux/cdrom.h>
> +#include <linux/ratelimit.h>
> #include <linux/slab.h>
> #include <linux/times.h>
> #include <asm/uaccess.h>
> @@ -690,9 +691,53 @@ int scsi_cmd_ioctl(struct request_queue
> }
> EXPORT_SYMBOL(scsi_cmd_ioctl);
>
> +int scsi_verify_blk_ioctl(struct block_device *bd, unsigned int cmd)
> +{
> + if (bd && bd == bd->bd_contains)
> + return 0;
> +
> + /* Actually none of these is particularly useful on a partition,
> + * but they are safe.
> + */
> + switch (cmd) {
> + case SCSI_IOCTL_GET_IDLUN:
> + case SCSI_IOCTL_GET_BUS_NUMBER:
> + case SCSI_IOCTL_GET_PCI:
> + case SCSI_IOCTL_PROBE_HOST:
> + case SG_GET_VERSION_NUM:
> + case SG_SET_TIMEOUT:
> + case SG_GET_TIMEOUT:
> + case SG_GET_RESERVED_SIZE:
> + case SG_SET_RESERVED_SIZE:
> + case SG_EMULATED_HOST:
> + return 0;
> + case CDROM_GET_CAPABILITY:
> + /* Keep this until we remove the printk below. udev sends it
> + * and we do not want to spam dmesg about it. CD-ROMs do
> + * not have partitions, so we get here only for disks.
> + */
> + return -ENOTTY;
> + default:
> + break;
> + }
> +
> + /* In particular, rule out all resets and host-specific ioctls. */
> + printk_ratelimited(KERN_WARNING
> + "%s: sending ioctl %x to a partition!\n", current->comm, cmd);
> +
> + return capable(CAP_SYS_RAWIO) ? 0 : -ENOTTY;
> +}
> +EXPORT_SYMBOL(scsi_verify_blk_ioctl);
> +
> int scsi_cmd_blk_ioctl(struct block_device *bd, fmode_t mode,
> unsigned int cmd, void __user *arg)
> {
> + int ret;
> +
> + ret = scsi_verify_blk_ioctl(bd, cmd);
> + if (ret < 0)
> + return ret;
> +
> return scsi_cmd_ioctl(bd->bd_disk->queue, bd->bd_disk, mode, cmd, arg);
> }
> EXPORT_SYMBOL(scsi_cmd_blk_ioctl);
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -1074,6 +1074,10 @@ static int sd_ioctl(struct block_device
> SCSI_LOG_IOCTL(1, sd_printk(KERN_INFO, sdkp, "sd_ioctl: disk=%s, "
> "cmd=0x%x\n", disk->disk_name, cmd));
>
> + error = scsi_verify_blk_ioctl(bdev, cmd);
> + if (error < 0)
> + return error;
> +
> /*
> * If we are in the middle of error recovery, don't let anyone
> * else try and use this device. Also, if error recovery fails, it
> @@ -1266,6 +1270,11 @@ static int sd_compat_ioctl(struct block_
> unsigned int cmd, unsigned long arg)
> {
> struct scsi_device *sdev = scsi_disk(bdev->bd_disk)->device;
> + int ret;
> +
> + ret = scsi_verify_blk_ioctl(bdev, cmd);
> + if (ret < 0)
> + return -ENOIOCTLCMD;
>
> /*
> * If we are in the middle of error recovery, don't let anyone
> @@ -1277,8 +1286,6 @@ static int sd_compat_ioctl(struct block_
> return -ENODEV;
>
> if (sdev->host->hostt->compat_ioctl) {
> - int ret;
> -
> ret = sdev->host->hostt->compat_ioctl(sdev, cmd, (void __user *)arg);
>
> return ret;
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -675,6 +675,7 @@ extern int blk_insert_cloned_request(str
> struct request *rq);
> extern void blk_delay_queue(struct request_queue *, unsigned long);
> extern void blk_recount_segments(struct request_queue *, struct bio *);
> +extern int scsi_verify_blk_ioctl(struct block_device *, unsigned int);
> extern int scsi_cmd_blk_ioctl(struct block_device *, fmode_t,
> unsigned int, void __user *);
> extern int scsi_cmd_ioctl(struct request_queue *, struct gendisk *, fmode_t,
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
