[PATCH v3 0/2] add per-device sysfs knob to enable unrestricted, unprivileged SG_IO

[PATCH v3 0/2] add per-device sysfs knob to enable unrestricted, unprivileged SG_IO

[Paolo Bonzini]

Privilege restrictions for SG_IO right now apply without distinction to
all devices, based on the single capability CAP_SYS_RAWIO.  This is a very
broad capability, and makes it difficult to give SG_IO access to trusted
clients that need access to persistent reservations, trim/discard, or
vendor-specific commands.  One problem here is that CAP_SYS_RAWIO allows
to escape a partition and issue commands that affect the full disk,
thus making DAC almost useless.

For simplicity, this series attempts to solve one case only: you want
to pass through almost everything, but still run as confined as possible.
This is for example the case for virtualization, where more complex
filtering can be done just as easily in userspace, in the virtual
machine monitor.  (This does mean the filter can be subverted if the
guest can escape the QEMU jail, but a more generic approach involving
a bitmap was NACKed).

Ok for 3.8?

v2->v3: change bitmap filter to boolean

Paolo Bonzini (2):
  sg_io: pass request_queue to blk_verify_command
  sg_io: introduce unpriv_sgio queue flag

 block/blk-sysfs.c      |   32 ++++++++++++++++++++++++++++++++
 block/bsg.c            |    2 +-
 block/scsi_ioctl.c     |    9 +++++----
 drivers/scsi/sg.c      |    3 ++-
 include/linux/blkdev.h |    6 +++++-
 5 files changed, 45 insertions(+), 7 deletions(-)

-- 
1.7.4.1

[PATCH v3 1/2] sg_io: pass request_queue to blk_verify_command

[Paolo Bonzini]

Adjust the blk_verify_command function to let it look at per-queue
data.  This will be done in the next patch.

Cc: linux-scsi@vger.kernel.org
Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: Ric Wheeler <rwheeler@redhat.com>
Cc: Tejun Heo <tj@kernel.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
	v2->v3: separated from "block: add back queue-private command filter"

 block/bsg.c            |    2 +-
 block/scsi_ioctl.c     |    7 ++++---
 drivers/scsi/sg.c      |    3 ++-
 include/linux/blkdev.h |    3 ++-
 4 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/block/bsg.c b/block/bsg.c
index ff64ae3..eb5ad59 100644
--- a/block/bsg.c
+++ b/block/bsg.c
@@ -187,7 +187,7 @@ static int blk_fill_sgv4_hdr_rq(struct request_queue *q, struct request *rq,
 		return -EFAULT;
 
 	if (hdr->subprotocol == BSG_SUB_PROTOCOL_SCSI_CMD) {
-		if (blk_verify_command(rq->cmd, has_write_perm))
+		if (blk_verify_command(q, rq->cmd, has_write_perm))
 			return -EPERM;
 	} else if (!capable(CAP_SYS_RAWIO))
 		return -EPERM;
diff --git a/block/scsi_ioctl.c b/block/scsi_ioctl.c
index 9a87daa..a737562 100644
--- a/block/scsi_ioctl.c
+++ b/block/scsi_ioctl.c
@@ -196,7 +196,8 @@ static void blk_set_cmd_filter_defaults(struct blk_cmd_filter *filter)
 	__set_bit(GPCMD_SET_READ_AHEAD, filter->write_ok);
 }
 
-int blk_verify_command(unsigned char *cmd, fmode_t has_write_perm)
+int blk_verify_command(struct request_queue *q,
+		       unsigned char *cmd, fmode_t has_write_perm)
 {
 	struct blk_cmd_filter *filter = &blk_default_cmd_filter;
 
@@ -225,7 +226,7 @@ static int blk_fill_sghdr_rq(struct request_queue *q, struct request *rq,
 {
 	if (copy_from_user(rq->cmd, hdr->cmdp, hdr->cmd_len))
 		return -EFAULT;
-	if (blk_verify_command(rq->cmd, mode & FMODE_WRITE))
+	if (blk_verify_command(q, rq->cmd, mode & FMODE_WRITE))
 		return -EPERM;
 
 	/*
@@ -472,7 +473,7 @@ int sg_scsi_ioctl(struct request_queue *q, struct gendisk *disk, fmode_t mode,
 	if (in_len && copy_from_user(buffer, sic->data + cmdlen, in_len))
 		goto error;
 
-	err = blk_verify_command(rq->cmd, mode & FMODE_WRITE);
+	err = blk_verify_command(q, rq->cmd, mode & FMODE_WRITE);
 	if (err)
 		goto error;
 
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index be2c9a6..cab816f 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -217,11 +217,12 @@ static void sg_put_dev(Sg_device *sdp);
 static int sg_allow_access(struct file *filp, unsigned char *cmd)
 {
 	struct sg_fd *sfp = filp->private_data;
+	struct request_queue *q = sfp->parentdp->device->request_queue;
 
 	if (sfp->parentdp->device->type == TYPE_SCANNER)
 		return 0;
 
-	return blk_verify_command(cmd, filp->f_mode & FMODE_WRITE);
+	return blk_verify_command(q, cmd, filp->f_mode & FMODE_WRITE);
 }
 
 static int get_exclude(Sg_device *sdp)
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 1756001..69a5e55 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1053,7 +1053,8 @@ static inline int sb_issue_zeroout(struct super_block *sb, sector_t block,
 				    gfp_mask);
 }
 
-extern int blk_verify_command(unsigned char *cmd, fmode_t has_write_perm);
+extern int blk_verify_command(struct request_queue *q,
+			      unsigned char *cmd, fmode_t has_write_perm);
 
 enum blk_default_limits {
 	BLK_MAX_SEGMENTS	= 128,
-- 
1.7.4.1

Re: [PATCH v3 1/2] sg_io: pass request_queue to blk_verify_command

[Tejun Heo]

On Tue, Nov 13, 2012 at 06:25:12PM +0100, Paolo Bonzini wrote:
> Adjust the blk_verify_command function to let it look at per-queue
> data.  This will be done in the next patch.
> 
> Cc: linux-scsi@vger.kernel.org
> Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
> Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
> Cc: Jens Axboe <axboe@kernel.dk>
> Cc: Ric Wheeler <rwheeler@redhat.com>
> Cc: Tejun Heo <tj@kernel.org>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Acked-by: Tejun Heo <tj@kernel.org>

Thanks.

-- 
tejun

[PATCH v3 2/2] sg_io: introduce unpriv_sgio queue flag

[Paolo Bonzini]

This queue flag will let unprivileged users send any SG_IO command to the
device, without any filtering.  This makes it possible to run a program
where you want to access the full range of SCSI commands, while still
running as confined as possible.  With this patch, such a program will
not need the CAP_SYS_RAWIO capability anymore, and will also not be
able to send SCSI commands to a partition (which would affect the full
disk).

Cc: linux-scsi@vger.kernel.org
Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: Ric Wheeler <rwheeler@redhat.com>
Cc: Tejun Heo <tj@kernel.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
	v2->v3: change bitmap filter to boolean

 block/blk-sysfs.c      |   32 ++++++++++++++++++++++++++++++++
 block/scsi_ioctl.c     |    2 +-
 include/linux/blkdev.h |    3 +++
 3 files changed, 36 insertions(+), 1 deletions(-)

diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index ce62046..935d10a 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -215,6 +215,31 @@ static ssize_t queue_max_hw_sectors_show(struct request_queue *q, char *page)
 	return queue_var_show(max_hw_sectors_kb, (page));
 }
 
+static ssize_t
+queue_show_unpriv_sgio(struct request_queue *q, char *page)
+{
+	int bit;
+	bit = test_bit(QUEUE_FLAG_UNPRIV_SGIO, &q->queue_flags);
+	return queue_var_show(bit, page);
+}
+static ssize_t
+queue_store_unpriv_sgio(struct request_queue *q, const char *page, size_t count)
+{
+	unsigned long val;
+	ssize_t ret;
+
+	if (!capable(CAP_SYS_ADMIN))
+		return -EPERM;
+
+	ret = queue_var_store(&val, page, count);
+	spin_lock_irq(q->queue_lock);
+	if (val)
+		queue_flag_set(QUEUE_FLAG_UNPRIV_SGIO, q);
+	else
+		queue_flag_clear(QUEUE_FLAG_UNPRIV_SGIO, q);
+	spin_unlock_irq(q->queue_lock);
+	return ret;
+}
 #define QUEUE_SYSFS_BIT_FNS(name, flag, neg)				\
 static ssize_t								\
 queue_show_##name(struct request_queue *q, char *page)			\
@@ -403,6 +428,12 @@ static struct queue_sysfs_entry queue_nonrot_entry = {
 	.store = queue_store_nonrot,
 };
 
+static struct queue_sysfs_entry queue_unpriv_sgio_entry = {
+	.attr = {.name = "unpriv_sgio", .mode = S_IRUGO | S_IWUSR },
+	.show = queue_show_unpriv_sgio,
+	.store = queue_store_unpriv_sgio,
+};
+
 static struct queue_sysfs_entry queue_nomerges_entry = {
 	.attr = {.name = "nomerges", .mode = S_IRUGO | S_IWUSR },
 	.show = queue_nomerges_show,
@@ -445,6 +476,7 @@ static struct attribute *default_attrs[] = {
 	&queue_discard_max_entry.attr,
 	&queue_discard_zeroes_data_entry.attr,
 	&queue_write_same_max_entry.attr,
+	&queue_unpriv_sgio_entry.attr,
 	&queue_nonrot_entry.attr,
 	&queue_nomerges_entry.attr,
 	&queue_rq_affinity_entry.attr,
diff --git a/block/scsi_ioctl.c b/block/scsi_ioctl.c
index a737562..1a999d6 100644
--- a/block/scsi_ioctl.c
+++ b/block/scsi_ioctl.c
@@ -202,7 +202,7 @@ int blk_verify_command(struct request_queue *q,
 	struct blk_cmd_filter *filter = &blk_default_cmd_filter;
 
 	/* root can do any command. */
-	if (capable(CAP_SYS_RAWIO))
+	if (capable(CAP_SYS_RAWIO) || blk_queue_unpriv_sgio(q))
 		return 0;
 
 	/* if there's no filter set, assume we're filtering everything out */
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 69a5e55..169a883 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -452,6 +452,7 @@ struct request_queue {
 #define QUEUE_FLAG_ADD_RANDOM  16	/* Contributes to random pool */
 #define QUEUE_FLAG_SECDISCARD  17	/* supports SECDISCARD */
 #define QUEUE_FLAG_SAME_FORCE  18	/* force complete on same CPU */
+#define QUEUE_FLAG_UNPRIV_SGIO 19	/* SG_IO free for unprivileged users */
 
 #define QUEUE_FLAG_DEFAULT	((1 << QUEUE_FLAG_IO_STAT) |		\
 				 (1 << QUEUE_FLAG_STACKABLE)	|	\
@@ -526,6 +527,8 @@ static inline void queue_flag_clear(unsigned int flag, struct request_queue *q)
 #define blk_queue_nomerges(q)	test_bit(QUEUE_FLAG_NOMERGES, &(q)->queue_flags)
 #define blk_queue_noxmerges(q)	\
 	test_bit(QUEUE_FLAG_NOXMERGES, &(q)->queue_flags)
+#define blk_queue_unpriv_sgio(q) \
+	test_bit(QUEUE_FLAG_UNPRIV_SGIO, &(q)->queue_flags)
 #define blk_queue_nonrot(q)	test_bit(QUEUE_FLAG_NONROT, &(q)->queue_flags)
 #define blk_queue_io_stat(q)	test_bit(QUEUE_FLAG_IO_STAT, &(q)->queue_flags)
 #define blk_queue_add_random(q)	test_bit(QUEUE_FLAG_ADD_RANDOM, &(q)->queue_flags)
-- 
1.7.4.1

Re: [PATCH v3 2/2] sg_io: introduce unpriv_sgio queue flag

[Tejun Heo]

On Tue, Nov 13, 2012 at 06:25:13PM +0100, Paolo Bonzini wrote:
> This queue flag will let unprivileged users send any SG_IO command to the
> device, without any filtering.  This makes it possible to run a program
> where you want to access the full range of SCSI commands, while still
> running as confined as possible.  With this patch, such a program will
> not need the CAP_SYS_RAWIO capability anymore, and will also not be
> able to send SCSI commands to a partition (which would affect the full
> disk).
> 
> Cc: linux-scsi@vger.kernel.org
> Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
> Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
> Cc: Jens Axboe <axboe@kernel.dk>
> Cc: Ric Wheeler <rwheeler@redhat.com>
> Cc: Tejun Heo <tj@kernel.org>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Acked-by: Tejun Heo <tj@kernel.org>

Thanks.

-- 
tejun

Re: [PATCH v3 0/2] add per-device sysfs knob to enable unrestricted, unprivileged SG_IO

[Paolo Bonzini]

Il 13/11/2012 18:25, Paolo Bonzini ha scritto:
> Privilege restrictions for SG_IO right now apply without distinction to
> all devices, based on the single capability CAP_SYS_RAWIO.  This is a very
> broad capability, and makes it difficult to give SG_IO access to trusted
> clients that need access to persistent reservations, trim/discard, or
> vendor-specific commands.  One problem here is that CAP_SYS_RAWIO allows
> to escape a partition and issue commands that affect the full disk,
> thus making DAC almost useless.
> 
> For simplicity, this series attempts to solve one case only: you want
> to pass through almost everything, but still run as confined as possible.
> This is for example the case for virtualization, where more complex
> filtering can be done just as easily in userspace, in the virtual
> machine monitor.  (This does mean the filter can be subverted if the
> guest can escape the QEMU jail, but a more generic approach involving
> a bitmap was NACKed).
> 
> Ok for 3.8?

Ping... Jens, I haven't seen any pull request for 3.8 from you.  Are
these patches on your radar?  Tejun acked both of them.

Paolo

> v2->v3: change bitmap filter to boolean
> 
> Paolo Bonzini (2):
>   sg_io: pass request_queue to blk_verify_command
>   sg_io: introduce unpriv_sgio queue flag
> 
>  block/blk-sysfs.c      |   32 ++++++++++++++++++++++++++++++++
>  block/bsg.c            |    2 +-
>  block/scsi_ioctl.c     |    9 +++++----
>  drivers/scsi/sg.c      |    3 ++-
>  include/linux/blkdev.h |    6 +++++-
>  5 files changed, 45 insertions(+), 7 deletions(-)
> 

Ping^2 Re: [PATCH v3 0/2] add per-device sysfs knob to enable unrestricted, unprivileged SG_IO

[Paolo Bonzini]

Il 17/12/2012 15:27, Paolo Bonzini ha scritto:
> Il 13/11/2012 18:25, Paolo Bonzini ha scritto:
>> > Privilege restrictions for SG_IO right now apply without distinction to
>> > all devices, based on the single capability CAP_SYS_RAWIO.  This is a very
>> > broad capability, and makes it difficult to give SG_IO access to trusted
>> > clients that need access to persistent reservations, trim/discard, or
>> > vendor-specific commands.  One problem here is that CAP_SYS_RAWIO allows
>> > to escape a partition and issue commands that affect the full disk,
>> > thus making DAC almost useless.
>> > 
>> > For simplicity, this series attempts to solve one case only: you want
>> > to pass through almost everything, but still run as confined as possible.
>> > This is for example the case for virtualization, where more complex
>> > filtering can be done just as easily in userspace, in the virtual
>> > machine monitor.  (This does mean the filter can be subverted if the
>> > guest can escape the QEMU jail, but a more generic approach involving
>> > a bitmap was NACKed).
>> > 
>> > Ok for 3.8?
> Ping... Jens, I haven't seen any pull request for 3.8 from you.  Are
> these patches on your radar?  Tejun acked both of them.

Ping^2.

Paolo