[PATCH] scsi: Silence unnecessary warnings about ioctl to partition

[PATCH] scsi: Silence unnecessary warnings about ioctl to partition

[Jan Kara]

Sometimes, warnings about ioctls to partition happen often enough that they
form majority of the warnings in the kernel log and users complain. In some
cases warnings are about ioctls such as SG_IO so it's not good to get rid of
the warnings completely as they can ease debugging of userspace problems
when ioctl is refused.

Since I have seen warnings from lots of commands, including some proprietary
userspace applications, I don't think disallowing the ioctls for processes
with CAP_SYS_RAWIO will happen in the near future if ever. So lets just
stop warning for processes with CAP_SYS_RAWIO for which ioctl is allowed.

CC: Paolo Bonzini <pbonzini@redhat.com>
CC: Jens Axboe <axboe@kernel.dk>
CC: James Bottomley <JBottomley@parallels.com>
CC: linux-scsi@vger.kernel.org
Signed-off-by: Jan Kara <jack@suse.cz>
---
 block/scsi_ioctl.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/block/scsi_ioctl.c b/block/scsi_ioctl.c
index 260fa80..9a87daa 100644
--- a/block/scsi_ioctl.c
+++ b/block/scsi_ioctl.c
@@ -721,11 +721,14 @@ int scsi_verify_blk_ioctl(struct block_device *bd, unsigned int cmd)
 		break;
 	}
 
+	if (capable(CAP_SYS_RAWIO))
+		return 0;
+
 	/* 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 : -ENOIOCTLCMD;
+	return -ENOIOCTLCMD;
 }
 EXPORT_SYMBOL(scsi_verify_blk_ioctl);
 
-- 
1.7.1

[PATCH] scsi: Silence unnecessary warnings about ioctl to partition

[Jan Kara]

Sometimes, warnings about ioctls to partition happen often enough that they
form majority of the warnings in the kernel log and users complain. In some
cases warnings are about ioctls such as SG_IO so it's not good to get rid of
the warnings completely as they can ease debugging of userspace problems
when ioctl is refused.

Since I have seen warnings from lots of commands, including some proprietary
userspace applications, I don't think disallowing the ioctls for processes
with CAP_SYS_RAWIO will happen in the near future if ever. So lets just
stop warning for processes with CAP_SYS_RAWIO for which ioctl is allowed.

CC: Paolo Bonzini <pbonzini@redhat.com>
CC: Jens Axboe <axboe@kernel.dk>
CC: James Bottomley <JBottomley@parallels.com>
CC: linux-scsi@vger.kernel.org
Signed-off-by: Jan Kara <jack@suse.cz>
---
 block/scsi_ioctl.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/block/scsi_ioctl.c b/block/scsi_ioctl.c
index 260fa80..9a87daa 100644
--- a/block/scsi_ioctl.c
+++ b/block/scsi_ioctl.c
@@ -721,11 +721,14 @@ int scsi_verify_blk_ioctl(struct block_device *bd, unsigned int cmd)
 		break;
 	}
 
+	if (capable(CAP_SYS_RAWIO))
+		return 0;
+
 	/* 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 : -ENOIOCTLCMD;
+	return -ENOIOCTLCMD;
 }
 EXPORT_SYMBOL(scsi_verify_blk_ioctl);
 
-- 
1.7.1

Re: [PATCH] scsi: Silence unnecessary warnings about ioctl to partition

[Paolo Bonzini]

Il 02/05/2012 12:10, Jan Kara ha scritto:
> Sometimes, warnings about ioctls to partition happen often enough that they
> form majority of the warnings in the kernel log and users complain. In some
> cases warnings are about ioctls such as SG_IO so it's not good to get rid of
> the warnings completely as they can ease debugging of userspace problems
> when ioctl is refused.
> 
> Since I have seen warnings from lots of commands, including some proprietary
> userspace applications, I don't think disallowing the ioctls for processes
> with CAP_SYS_RAWIO will happen in the near future if ever. So lets just
> stop warning for processes with CAP_SYS_RAWIO for which ioctl is allowed.

NACK.  I would bet that all the warnings you've seen are for ioctl that
would have failed anyway with ENOTTY.

The right fix has already been posted, we've been carrying it in RHEL
for over six months and not a single bug has been seen.

Paolo

Re: [PATCH] scsi: Silence unnecessary warnings about ioctl to partition

[Jens Axboe]

On 05/02/2012 12:15 PM, Paolo Bonzini wrote:
> Il 02/05/2012 12:10, Jan Kara ha scritto:
>> Sometimes, warnings about ioctls to partition happen often enough that they
>> form majority of the warnings in the kernel log and users complain. In some
>> cases warnings are about ioctls such as SG_IO so it's not good to get rid of
>> the warnings completely as they can ease debugging of userspace problems
>> when ioctl is refused.
>>
>> Since I have seen warnings from lots of commands, including some proprietary
>> userspace applications, I don't think disallowing the ioctls for processes
>> with CAP_SYS_RAWIO will happen in the near future if ever. So lets just
>> stop warning for processes with CAP_SYS_RAWIO for which ioctl is allowed.
> 
> NACK.  I would bet that all the warnings you've seen are for ioctl that
> would have failed anyway with ENOTTY.
> 
> The right fix has already been posted, we've been carrying it in RHEL
> for over six months and not a single bug has been seen.

I don't remember seeing the "right" fix. The messages are really
annoying, so lets please get it resubmitted so that we can merge it.

-- 
Jens Axboe

Re: [PATCH] scsi: Silence unnecessary warnings about ioctl to partition

[Alan Cox]

On Wed, 02 May 2012 12:15:10 +0200
Paolo Bonzini <pbonzini@redhat.com> wrote:

> Il 02/05/2012 12:10, Jan Kara ha scritto:
> > Sometimes, warnings about ioctls to partition happen often enough that they
> > form majority of the warnings in the kernel log and users complain. In some
> > cases warnings are about ioctls such as SG_IO so it's not good to get rid of
> > the warnings completely as they can ease debugging of userspace problems
> > when ioctl is refused.
> > 
> > Since I have seen warnings from lots of commands, including some proprietary
> > userspace applications, I don't think disallowing the ioctls for processes
> > with CAP_SYS_RAWIO will happen in the near future if ever. So lets just
> > stop warning for processes with CAP_SYS_RAWIO for which ioctl is allowed.
> 
> NACK.  I would bet that all the warnings you've seen are for ioctl that
> would have failed anyway with ENOTTY.

Then we don't need the bogus warning do we.

Alan

Re: [PATCH] scsi: Silence unnecessary warnings about ioctl to partition

[Paolo Bonzini]

Il 02/05/2012 12:54, Alan Cox ha scritto:
>>> > > Since I have seen warnings from lots of commands, including some proprietary
>>> > > userspace applications, I don't think disallowing the ioctls for processes
>>> > > with CAP_SYS_RAWIO will happen in the near future if ever. So lets just
>>> > > stop warning for processes with CAP_SYS_RAWIO for which ioctl is allowed.
>> > 
>> > NACK.  I would bet that all the warnings you've seen are for ioctl that
>> > would have failed anyway with ENOTTY.
> Then we don't need the bogus warning do we.

Sure, but then disallowing the ioctls for processes with CAP_SYS_RAWIO
will not cause regressions and _can_ happen.  The transition period only
needs to be prolonged for SG_IO, the only one that was reported in the
wild, until people have time to fix their bugs or (I hope not) we give
up and implement a very restrictive filter for SCSI commands sent to
partition.

The right patch is one that prepares for these step,
http://permalink.gmane.org/gmane.linux.kernel/1254625 for example.  It
leaves the warning only for SG_IO, and silently blocks the rest (more
rationale in the commit message there).

However, that patch should be applied only at the beginning of the merge
window, not at the end of the release cycle.

Paolo

Re: [PATCH] scsi: Silence unnecessary warnings about ioctl to partition

[Alan Cox]

> Sure, but then disallowing the ioctls for processes with CAP_SYS_RAWIO
> will not cause regressions and _can_ happen.  The transition period only

The user has CAP_SYS_RAWIO, they can already do it by poking the
registers on the chip directly. It is a nonsense to attempt to block or
warn about this.

> up and implement a very restrictive filter for SCSI commands sent to
> partition.

The process has CAP_SYS_RAWIO it can already bypass any check you try and
put in place.

> The right patch is one that prepares for these step,

Doesn't look very right to me.

> http://permalink.gmane.org/gmane.linux.kernel/1254625 for example.  It
> leaves the warning only for SG_IO, and silently blocks the rest (more
> rationale in the commit message there).

Even the printk in that patch is wrong. We have capabilities. Being a
"root" user is a meaningless distinction here so your ratelimited printk
isn't just bogus - its wrong. It may have got into RHEL somehow but the
kernel QA process is a bit higher standard than this proposed patch.

A process with CAP_SYS_RAWIO has total power. It's assumed to know what
it is doing. Trying to block it doing stuff like that simply makes
authors do them via different more crass methods.

Alan

Re: [PATCH] scsi: Silence unnecessary warnings about ioctl to partition

[Paolo Bonzini]

Il 02/05/2012 13:12, Alan Cox ha scritto:
>> Sure, but then disallowing the ioctls for processes with CAP_SYS_RAWIO
>> will not cause regressions and _can_ happen.  The transition period only
> 
> The user has CAP_SYS_RAWIO, they can already do it by poking the
> registers on the chip directly. It is a nonsense to attempt to block or
> warn about this.

Not true, for example CAP_SYS_RAWIO is still subject to access control.
 Even simple Unix DAC can prevent you from issuing register writes to
/dev/sdb, while letting you do so on /dev/sda and access /dev/sdb1.  I'm
not inventing anything, the old ATA subsystem is already blocking most
"dangerous" ioctls for partitions, even if you have CAP_SYS_RAWIO.

Now of course CAP_SYS_RAWIO lets you use ioperm or iopl, but that's a
separate issue and only limited to x86.

>> up and implement a very restrictive filter for SCSI commands sent to
>> partition.
> 
> The process has CAP_SYS_RAWIO it can already bypass any check you try and
> put in place.

Almost any capability can be abused to bypass checks.  True,
CAP_SYS_RAWIO is especially good at that, but still you can try.

>> The right patch is one that prepares for these step,
> 
> Doesn't look very right to me.
> 
>> http://permalink.gmane.org/gmane.linux.kernel/1254625 for example.  It
>> leaves the warning only for SG_IO, and silently blocks the rest (more
>> rationale in the commit message there).
> 
> Even the printk in that patch is wrong. We have capabilities. Being a
> "root" user is a meaningless distinction here so your ratelimited printk
> isn't just bogus - its wrong. It may have got into RHEL somehow but the
> kernel QA process is a bit higher standard than this proposed patch.

Indeed, RHEL doesn't have the warning at all and blocks all ioctls
including SG_IO (and in the past six months nobody has complained that
something stopped working for them).  Never said the patch is perfect...

> A process with CAP_SYS_RAWIO has total power. It's assumed to know what
> it is doing. Trying to block it doing stuff like that simply makes
> authors do them via different more crass methods.

Getting appropriate permission on device nodes is less crass than
abusing partition device nodes.

Paolo

Re: [PATCH] scsi: Silence unnecessary warnings about ioctl to partition

[Alan Cox]

> not inventing anything, the old ATA subsystem is already blocking most
> "dangerous" ioctls for partitions, even if you have CAP_SYS_RAWIO.

It blocked a few by default to protect hardware. It's a tricky tradeoff,
which is quite different to this.

> Now of course CAP_SYS_RAWIO lets you use ioperm or iopl, but that's a
> separate issue and only limited to x86.

Ie only 99.99% of the systems running desktop/server Linux OS designs.

> Almost any capability can be abused to bypass checks.  True,
> CAP_SYS_RAWIO is especially good at that, but still you can try.

Why try - you are seeking to arbitarily impose your own worldview on the
interface (and in doing so break back compatibility). The whole basis of
the Unix philosophy is that the OS shouldn't try and micromanage the
priviledged apps because that just leads to crap code.

Think "small government" on this aspect of design. And with the patch you
propose the analogy for your patch is the TSA.

> > A process with CAP_SYS_RAWIO has total power. It's assumed to know what
> > it is doing. Trying to block it doing stuff like that simply makes
> > authors do them via different more crass methods.
> 
> Getting appropriate permission on device nodes is less crass than
> abusing partition device nodes.

Given a passed file handle how do you do that securely. Remember that
open /dev/foo while you have a handle on /dev/foo1 could open a
different disk if a hotplug has occurred.

So there are good reasons to keep the partition behaviour.

Alan

Re: [PATCH] scsi: Silence unnecessary warnings about ioctl to partition

[Paolo Bonzini]

> > not inventing anything, the old ATA subsystem is already blocking
> > most
> > "dangerous" ioctls for partitions, even if you have CAP_SYS_RAWIO.
> 
> It blocked a few by default to protect hardware. It's a tricky
> tradeoff, which is quite different to this.

I agree it's tricky.

> > Now of course CAP_SYS_RAWIO lets you use ioperm or iopl, but that's
> > a separate issue and only limited to x86.
> 
> Ie only 99.99% of the systems running desktop/server Linux OS
> designs.

You can still block those in other ways, seccompv2 could be one.

> > Almost any capability can be abused to bypass checks.  True,
> > CAP_SYS_RAWIO is especially good at that, but still you can try.
> 
> Why try - you are seeking to arbitarily impose your own worldview on
> the interface (and in doing so break back compatibility). The whole basis
> of the Unix philosophy is that the OS shouldn't try and micromanage the
> priviledged apps because that just leads to crap code.

The whole point of capabilities, SELinux, etc. is micromanaging privileged
apps.  Linux has gone beyond the Unix philosophy, it seems.

Besides, privileged apps can and do use capabilities to micromanage
themselves.  Sure, ioperm/iopl do mean that a compromised CAP_SYS_RAWIO app
has free reins.  However, until that app is compromised, it may like to get
help from the OS in making effective use of access control.

> > > A process with CAP_SYS_RAWIO has total power. It's assumed to
> > > know what it is doing. Trying to block it doing stuff like that simply
> > > makes authors do them via different more crass methods.
> > 
> > Getting appropriate permission on device nodes is less crass than
> > abusing partition device nodes.
> 
> Given a passed file handle how do you do that securely. Remember that
> open /dev/foo while you have a handle on /dev/foo1 could open a
> different disk if a hotplug has occurred.

What is the exact use case for this, where you wouldn't have gotten an
fd for /dev/foo in the first place?  And what command do you want to
send to the disk?  The only "valid" case found in the wild was using
SG_IO to send "SYNCHRONIZE CACHE", which is wrong anyway (does not
flush the OS cache).  fdatasync could and should have been used instead.

Paolo

Re: [PATCH] scsi: Silence unnecessary warnings about ioctl to partition

[Jan Kara]

On Wed 02-05-12 12:15:10, Paolo Bonzini wrote:
> Il 02/05/2012 12:10, Jan Kara ha scritto:
> > Sometimes, warnings about ioctls to partition happen often enough that they
> > form majority of the warnings in the kernel log and users complain. In some
> > cases warnings are about ioctls such as SG_IO so it's not good to get rid of
> > the warnings completely as they can ease debugging of userspace problems
> > when ioctl is refused.
> > 
> > Since I have seen warnings from lots of commands, including some proprietary
> > userspace applications, I don't think disallowing the ioctls for processes
> > with CAP_SYS_RAWIO will happen in the near future if ever. So lets just
> > stop warning for processes with CAP_SYS_RAWIO for which ioctl is allowed.
> 
> NACK.  I would bet that all the warnings you've seen are for ioctl that
> would have failed anyway with ENOTTY.
  Actually, you would loose the bet ;) The customer was complaining about
warning about SG_IO ioctl. Apparently some Veritas filesystem thread generates
a *lot* of these (I don't know if they happen to do all the filesystem IO
with SG_IO and I'm not sure I want to know ;). Given this I don't think we
want to block SG_IO for CAP_SYS_RAWIO threads in the near future if ever...

> The right fix has already been posted, we've been carrying it in RHEL
> for over six months and not a single bug has been seen.
  Your patch won't work for our customer because you still generate
messages for SG_IO. Also I tend to side with Alan that I don't quite see
the point in trying to restrict CAP_SYS_RAWIO threads and thus breaking the
compatibility (if ioctls would be restricted for partitions from the
beginning, then sure it seems like a cleaner choice). But I don't feel that
strongly about it.

									Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

Re: [PATCH] scsi: Silence unnecessary warnings about ioctl to partition

[Paolo Bonzini]

Il 02/05/2012 15:51, Jan Kara ha scritto:
>> > NACK.  I would bet that all the warnings you've seen are for ioctl that
>> > would have failed anyway with ENOTTY.
>   Actually, you would loose the bet ;)

Doh. :)

> The customer was complaining about
> warning about SG_IO ioctl. Apparently some Veritas filesystem thread generates
> a *lot* of these (I don't know if they happen to do all the filesystem IO
> with SG_IO and I'm not sure I want to know ;).

Can you at least ask the customer for help finding which command was
sent?  And perhaps have them try a kernel that blocks SG_IO to see what
breaks if anything?

> Also I tend to side with Alan that I don't quite see
> the point in trying to restrict CAP_SYS_RAWIO threads and thus breaking the
> compatibility

For example, we have a customer that wants this:

* a VM should be able to send vendor-specific commands to a disk via
SG_IO (vendor-specific commands require CAP_SYS_RAWIO).

* they want to assign logical volumes or partitions to the same VM
without letting it read or write outside the logical volume or partition.

Of course a better solution for this would be customizable filters for
SG_IO commands, where a privileged application would open the block
device with CAP_SYS_RAWIO, set the filter and hand the file descriptor
to QEMU.  Or alternatively some extension of the device cgroup.  But
either solution would require a large amount of work.

Paolo

> (if ioctls would be restricted for partitions from the
> beginning, then sure it seems like a cleaner choice). But I don't feel that
> strongly about it.

Re: [PATCH] scsi: Silence unnecessary warnings about ioctl to partition

[Alan Cox]

> > Also I tend to side with Alan that I don't quite see
> > the point in trying to restrict CAP_SYS_RAWIO threads and thus breaking the
> > compatibility
> 
> For example, we have a customer that wants this:
> 
> * a VM should be able to send vendor-specific commands to a disk via
> SG_IO (vendor-specific commands require CAP_SYS_RAWIO).
> 
> * they want to assign logical volumes or partitions to the same VM
> without letting it read or write outside the logical volume or partition.

And if the process has CAP_SYS_RAWIO it can do it anyway. So not only is
your agenda destructive - it doesn't actually work.

> Of course a better solution for this would be customizable filters for
> SG_IO commands, where a privileged application would open the block
> device with CAP_SYS_RAWIO, set the filter and hand the file descriptor
> to QEMU.  Or alternatively some extension of the device cgroup.  But
> either solution would require a large amount of work.

Or you could just do the special case ioctl magic out of band in the apps.
It's hardly an ultra performance critical path for the SG_IO cases.

Customisable filters are not hard. We've got all the filtering code in
kernel and the ability to verify filters, even the ability to JIT them.
Just support adding/removing/running a BPF filter on the channel in
question.

So it shouldn't be much code to do what you want.

Alan

Re: [PATCH] scsi: Silence unnecessary warnings about ioctl to partition

[Paolo Bonzini]

Il 02/05/2012 17:10, Alan Cox ha scritto:
>>> Also I tend to side with Alan that I don't quite see
>>> the point in trying to restrict CAP_SYS_RAWIO threads and thus breaking the
>>> compatibility
>>
>> For example, we have a customer that wants this:
>>
>> * a VM should be able to send vendor-specific commands to a disk via
>> SG_IO (vendor-specific commands require CAP_SYS_RAWIO).
>>
>> * they want to assign logical volumes or partitions to the same VM
>> without letting it read or write outside the logical volume or partition.
> 
> And if the process has CAP_SYS_RAWIO it can do it anyway.

How so?  Assuming /dev/sdb is not accessible, /dev/sdb1 is accessible,
and no iopl/ioperm.

> Or you could just do the special case ioctl magic out of band in the apps.

You mentioned crass/gross hacks.  Forcing apps to detect if you're
targeting a partition or a block device _is_ gross.

> It's hardly an ultra performance critical path for the SG_IO cases.

That I agree with.

>> Of course a better solution for this would be customizable filters for
>> SG_IO commands, where a privileged application would open the block
>> device with CAP_SYS_RAWIO, set the filter and hand the file descriptor
>> to QEMU.  Or alternatively some extension of the device cgroup.  But
>> either solution would require a large amount of work.
> 
> Customisable filters are not hard. We've got all the filtering code in
> kernel and the ability to verify filters, even the ability to JIT them.
> Just support adding/removing/running a BPF filter on the channel in
> question.
> 
> So it shouldn't be much code to do what you want.

Yes, it's not much code if I don't get into cgroups land and stick with
a ioctl to add and remove BPF filters that look at CDBs.  One downside
is that such filtering would likely be enabled by CAP_SYS_RAWIO.
Because of this, tweaking the filter on the fly is still not too easy
because I want to run as unprivileged as possible.  I guess some
privileged helper program can set the filter and send me back the file
descriptor via SCM_RIGHTS.  The filter will be preserved across that, right?

I still believe this is suboptimal in the general case, and that Jan's
customer has a bug.  But hey I would have ended up implementing the
filters anyway sooner or later, so I'd rather avoid further flames and
work on them with someone supporting the idea. :)  Reluctantly

Acked-by: Paolo Bonzini <pbonzini@redhat.com>

Paolo

Re: [PATCH] scsi: Silence unnecessary warnings about ioctl to partition

[Mark Lord]

On 12-05-02 07:24 AM, Paolo Bonzini wrote:
>
> Indeed, RHEL doesn't have the warning at all and blocks all ioctls
> including SG_IO (and in the past six months nobody has complained that
> something stopped working for them).  Never said the patch is perfect...

hdparm.

Re: [PATCH] scsi: Silence unnecessary warnings about ioctl to partition

[Jan Kara]

On Wed 02-05-12 15:59:59, Paolo Bonzini wrote:
> Il 02/05/2012 15:51, Jan Kara ha scritto:
> >> > NACK.  I would bet that all the warnings you've seen are for ioctl that
> >> > would have failed anyway with ENOTTY.
> >   Actually, you would loose the bet ;)
> 
> Doh. :)
> 
> > The customer was complaining about
> > warning about SG_IO ioctl. Apparently some Veritas filesystem thread generates
> > a *lot* of these (I don't know if they happen to do all the filesystem IO
> > with SG_IO and I'm not sure I want to know ;).
> 
> Can you at least ask the customer for help finding which command was
> sent?  And perhaps have them try a kernel that blocks SG_IO to see what
> breaks if anything?
  I'm not sure they would be willing to try a different kernel because it's
a production system. But maybe I can find out what SG_IO command is sent
via strace?

> > Also I tend to side with Alan that I don't quite see
> > the point in trying to restrict CAP_SYS_RAWIO threads and thus breaking the
> > compatibility
> 
> For example, we have a customer that wants this:
> 
> * a VM should be able to send vendor-specific commands to a disk via
> SG_IO (vendor-specific commands require CAP_SYS_RAWIO).
> 
> * they want to assign logical volumes or partitions to the same VM
> without letting it read or write outside the logical volume or partition.
  But then it seems like they really want to be able to forbid sending
SG_IO commands to some devices while allowing them for other devices and
the distinction by partition / non-partition is a bit arbitrary?
 
> Of course a better solution for this would be customizable filters for
> SG_IO commands, where a privileged application would open the block
> device with CAP_SYS_RAWIO, set the filter and hand the file descriptor
> to QEMU.  Or alternatively some extension of the device cgroup.  But
> either solution would require a large amount of work.
  I'm not sure whether you need to filter individual SG_IO commands or not.
For your use case it seems that being able to forbid SG_IO completely for
some fd (which would be passed to qemu) would be enough? But maybe filters
are simpler to implement because they already exist, I don't really know...

								Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

Re: [PATCH] scsi: Silence unnecessary warnings about ioctl to partition

[Paolo Bonzini]

Il 02/05/2012 17:49, Paolo Bonzini ha scritto:
>> > Customisable filters are not hard. We've got all the filtering code in
>> > kernel and the ability to verify filters, even the ability to JIT them.
>> > Just support adding/removing/running a BPF filter on the channel in
>> > question.
>> > 
>> > So it shouldn't be much code to do what you want.
> Yes, it's not much code if I don't get into cgroups land and stick with
> a ioctl to add and remove BPF filters that look at CDBs.

Except that then I need to access some "struct file" member in SG_IO,
thus changing ioctl from block_device/fmode to block_device/file.  This
would partially undo the 2007 switch from inode/file by Al Viro.
Somehow I'm not that optimist anymore, though I'd be happy to be proven
wrong.

Paolo

Re: [PATCH] scsi: Silence unnecessary warnings about ioctl to partition

[Paolo Bonzini]

Il 02/05/2012 21:49, Jan Kara ha scritto:
>   I'm not sure they would be willing to try a different kernel because it's
> a production system. But maybe I can find out what SG_IO command is sent
> via strace?

Yes.

Hmm, you mentioned Veritas and that reminds me of
https://bugzilla.redhat.com/show_bug.cgi?id=740504.  If that is the
case, the filesystem is simply pinging the destination with INQUIRY
commands, something for which it would be worthwhile to have a
non-privileged ioctl anyway.

>>> Also I tend to side with Alan that I don't quite see
>>> the point in trying to restrict CAP_SYS_RAWIO threads and thus breaking the
>>> compatibility
>>
>> For example, we have a customer that wants this:
>>
>> * a VM should be able to send vendor-specific commands to a disk via
>> SG_IO (vendor-specific commands require CAP_SYS_RAWIO).
>>
>> * they want to assign logical volumes or partitions to the same VM
>> without letting it read or write outside the logical volume or partition.
>  
>   But then it seems like they really want to be able to forbid sending
> SG_IO commands to some devices while allowing them for other devices and
> the distinction by partition / non-partition is a bit arbitrary?

Yes, forbidding SG_IO commands on some disks would be nice.  Still,
partition/non-partition is an important distinction.  If you pass a
whole disk and give CAP_SYS_RAWIO to QEMU, the guest may do some damage
but not more than what a bare-metal system could do.  If you pass a
partition, the guest can stomp on other VMs or the host's data and even
write them, which is a security problem.

So you could add a more restrictive filter to partitions, but then
you're adding hack above hack to justify a wrong decision.

>> Of course a better solution for this would be customizable filters for
>> SG_IO commands, where a privileged application would open the block
>> device with CAP_SYS_RAWIO, set the filter and hand the file descriptor
>> to QEMU.  Or alternatively some extension of the device cgroup.  But
>> either solution would require a large amount of work.
>
>   I'm not sure whether you need to filter individual SG_IO commands or not.
> For your use case it seems that being able to forbid SG_IO completely for
> some fd (which would be passed to qemu) would be enough? But maybe filters
> are simpler to implement because they already exist, I don't really know...

If you implement a yes/no toggle, some use case will pop up later for
filters (in fact, a rudimentary filter based on CAP_SYS_RAWIO is
_already_ in the kernel which already proves this).

Paolo

Re: [PATCH] scsi: Silence unnecessary warnings about ioctl to partition

[Paolo Bonzini]

Il 02/05/2012 21:38, Mark Lord ha scritto:
>> >
>> > Indeed, RHEL doesn't have the warning at all and blocks all ioctls
>> > including SG_IO (and in the past six months nobody has complained that
>> > something stopped working for them).  Never said the patch is perfect...
> hdparm.
> 

Breaking "hdparm --write-sector /dev/sda1"?  I call it a security fix.

Paolo

Re: [PATCH] scsi: Silence unnecessary warnings about ioctl to partition

[Mark Lord]

On 12-05-03 03:47 AM, Paolo Bonzini wrote:
> Il 02/05/2012 21:38, Mark Lord ha scritto:
>>>>
>>>> Indeed, RHEL doesn't have the warning at all and blocks all ioctls
>>>> including SG_IO (and in the past six months nobody has complained that
>>>> something stopped working for them).  Never said the patch is perfect...
>> hdparm.
>>
> 
> Breaking "hdparm --write-sector /dev/sda1"?  I call it a security fix.

No, that would plain stupid on both our parts.  :)
The --write-sector flag is allowed only for non-partitions by hdparm itself.

But other flags commonly used by distros at boot time
seem to be triggering the current in-kernel noise.
Dunno which flags, I'm just ignoring them and waiting
for the noise message to get reverted.

Cheers

Re: [PATCH] scsi: Silence unnecessary warnings about ioctl to partition

[Paolo Bonzini]

Il 03/05/2012 14:40, Mark Lord ha scritto:
>> > 
>> > Breaking "hdparm --write-sector /dev/sda1"?  I call it a security fix.
> No, that would plain stupid on both our parts.  :)
> The --write-sector flag is allowed only for non-partitions by hdparm itself.

Excuse my laziness--how does it check?  Most programs I looked at check
at the shape of the file and some are even fooled by "ln -sf /dev/sda1
./sda".

> But other flags commonly used by distros at boot time
> seem to be triggering the current in-kernel noise.
> Dunno which flags, I'm just ignoring them and waiting
> for the noise message to get reverted.

That's exactly the behavior I hoped to get when I added the warnings.
Or maybe not. :)  What are the messages?  i.e. what ioctl do they
complain about?

Paolo

Re: [PATCH] scsi: Silence unnecessary warnings about ioctl to partition

[Mark Lord]

On 12-05-03 08:47 AM, Paolo Bonzini wrote:
> Il 03/05/2012 14:40, Mark Lord ha scritto:
>>>>
>>>> Breaking "hdparm --write-sector /dev/sda1"?  I call it a security fix.
>> No, that would plain stupid on both our parts.  :)
>> The --write-sector flag is allowed only for non-partitions by hdparm itself.
> 
> Excuse my laziness--how does it check?

I don't know if there's a feasible fool-proof method or not.
But what hdparm does is look at the sector offset of the device.
Partitions normally have a non-zero offset.

>> But other flags commonly used by distros at boot time
>> seem to be triggering the current in-kernel noise.
>> Dunno which flags, I'm just ignoring them and waiting
>> for the noise message to get reverted.
> 
> That's exactly the behavior I hoped to get when I added the warnings.
> Or maybe not. :)  What are the messages?
> i.e. what ioctl do they complain about?

As above:

>> Dunno which flags, I'm just ignoring them and waiting
>> for the noise message to get reverted.

Re: [PATCH] scsi: Silence unnecessary warnings about ioctl to partition

[Paolo Bonzini]

Il 03/05/2012 19:36, Mark Lord ha scritto:
>> > Excuse my laziness--how does it check?
> I don't know if there's a feasible fool-proof method or not.
> But what hdparm does is look at the sector offset of the device.
> Partitions normally have a non-zero offset.

Yeah, that should work.

>>> >> But other flags commonly used by distros at boot time
>>> >> seem to be triggering the current in-kernel noise.
>>> >> Dunno which flags, I'm just ignoring them and waiting
>>> >> for the noise message to get reverted.
>> > 
>> > That's exactly the behavior I hoped to get when I added the warnings.
>> > Or maybe not. :)  What are the messages?
>> > i.e. what ioctl do they complain about?
> As above:
> 
>>> >> Dunno which flags, I'm just ignoring them and waiting
>>> >> for the noise message to get reverted.

I said which ioctls, not which options.  I.e. cut-and-paste from dmesg.

Paolo

Re: [PATCH] scsi: Silence unnecessary warnings about ioctl to partition

[Mark Lord]

On 12-05-04 02:39 AM, Paolo Bonzini wrote:
> Il 03/05/2012 19:36, Mark Lord ha scritto:
>
>>>>>> Dunno which flags, I'm just ignoring them and waiting
>>>>>> for the noise message to get reverted.
> 
> I said which ioctls, not which options.  I.e. cut-and-paste from dmesg.

Here are some:


messages.1:Apr 28 14:08:09 zbox5 kernel: [  268.800536] hdparm: sending ioctl 330 to a partition!
messages.1:Apr 28 14:08:09 zbox5 kernel: [  268.800547] hdparm: sending ioctl 330 to a partition!
messages.1:Apr 28 14:08:57 zbox5 kernel: [  316.413944] hdparm: sending ioctl 330 to a partition!
messages.1:Apr 28 14:08:57 zbox5 kernel: [  316.413951] hdparm: sending ioctl 330 to a partition!
messages.1:Apr 28 14:20:10 zbox5 kernel: [  989.525085] hdparm: sending ioctl 330 to a partition!
messages.1:Apr 28 14:20:10 zbox5 kernel: [  989.525093] hdparm: sending ioctl 330 to a partition!

The in <linux/hdreg.h> say this about 330:

...
/* 0x330 is reserved - used to be HDIO_GETGEO_BIG */
...

So it's HDIO_GETGEO_BIG, which doesn't exist in newer kernels.
I wonder when that got removed?  Minor userspace breakage there.

hdparm issues it first as a backward-compatibility thing,
before falling back to the even-more obsolete HDIO_GETGEO,
which curiously enough is still in modern kernels.

Cheers

Re: [PATCH] scsi: Silence unnecessary warnings about ioctl to partition

[Paolo Bonzini]

Il 04/05/2012 15:06, Mark Lord ha scritto:
>> > I said which ioctls, not which options.  I.e. cut-and-paste from dmesg.
> Here are some:

Thanks!

> messages.1:Apr 28 14:08:09 zbox5 kernel: [  268.800536] hdparm: sending ioctl 330 to a partition!
> messages.1:Apr 28 14:08:09 zbox5 kernel: [  268.800547] hdparm: sending ioctl 330 to a partition!
> messages.1:Apr 28 14:08:57 zbox5 kernel: [  316.413944] hdparm: sending ioctl 330 to a partition!
> messages.1:Apr 28 14:08:57 zbox5 kernel: [  316.413951] hdparm: sending ioctl 330 to a partition!
> messages.1:Apr 28 14:20:10 zbox5 kernel: [  989.525085] hdparm: sending ioctl 330 to a partition!
> messages.1:Apr 28 14:20:10 zbox5 kernel: [  989.525093] hdparm: sending ioctl 330 to a partition!
> 
> The in <linux/hdreg.h> say this about 330:
> 
> ...
> /* 0x330 is reserved - used to be HDIO_GETGEO_BIG */
> ...
> 
> So it's HDIO_GETGEO_BIG, which doesn't exist in newer kernels.
> I wonder when that got removed?  Minor userspace breakage there.
> 
> hdparm issues it first as a backward-compatibility thing,
> before falling back to the even-more obsolete HDIO_GETGEO,
> which curiously enough is still in modern kernels.

Ok, so hdparm is not broken.  My patch (which left the warning only for
SG_IO, and failed all other ioctls) would have worked, too.

Paolo

Re: [PATCH] scsi: Silence unnecessary warnings about ioctl to partition

[Mark Lord]

On 12-05-04 09:06 AM, Mark Lord wrote:
> On 12-05-04 02:39 AM, Paolo Bonzini wrote:
>> Il 03/05/2012 19:36, Mark Lord ha scritto:
>>
>>>>>>> Dunno which flags, I'm just ignoring them and waiting
>>>>>>> for the noise message to get reverted.
>>
>> I said which ioctls, not which options.  I.e. cut-and-paste from dmesg.
> 
> Here are some:
> 
> 
> messages.1:Apr 28 14:08:09 zbox5 kernel: [  268.800536] hdparm: sending ioctl 330 to a partition!
> messages.1:Apr 28 14:08:09 zbox5 kernel: [  268.800547] hdparm: sending ioctl 330 to a partition!
> messages.1:Apr 28 14:08:57 zbox5 kernel: [  316.413944] hdparm: sending ioctl 330 to a partition!
> messages.1:Apr 28 14:08:57 zbox5 kernel: [  316.413951] hdparm: sending ioctl 330 to a partition!
> messages.1:Apr 28 14:20:10 zbox5 kernel: [  989.525085] hdparm: sending ioctl 330 to a partition!
> messages.1:Apr 28 14:20:10 zbox5 kernel: [  989.525093] hdparm: sending ioctl 330 to a partition!
> 
> The in <linux/hdreg.h> say this about 330:
> 
> ...
> /* 0x330 is reserved - used to be HDIO_GETGEO_BIG */
> ...
> 
> So it's HDIO_GETGEO_BIG, which doesn't exist in newer kernels.
> I wonder when that got removed?  Minor userspace breakage there.

Looks like it got removed about 10 years ago,
either in 2.5.xx or early 2.6.xx, so that's all fine now.
hdparm still issues it for backward compatibility with
kernels that lack more modern methods.  Currently we don't
try to inspect the kernel version at run-time, because
version numbers are not as reliable as simply issuing
the ioctl().

> hdparm issues it first as a backward-compatibility thing,
> before falling back to the even-more obsolete HDIO_GETGEO,
> which curiously enough is still in modern kernels.
> 
> Cheers
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] scsi: Silence unnecessary warnings about ioctl to partition

[Mark Lord]

On 12-05-04 09:11 AM, Mark Lord wrote:
> On 12-05-04 09:06 AM, Mark Lord wrote:
..
>> kernel: [  989.525093] hdparm: sending ioctl 330 to a partition!
>>
>> The in <linux/hdreg.h> say this about 330:
>> ...
>> /* 0x330 is reserved - used to be HDIO_GETGEO_BIG */
>> ...
>>
>> So it's HDIO_GETGEO_BIG, which doesn't exist in newer kernels.
>> I wonder when that got removed?  Minor userspace breakage there.
> 
> Looks like it got removed about 10 years ago,
> either in 2.5.xx or early 2.6.xx, so that's all fine now.
> hdparm still issues it for backward compatibility with
> kernels that lack more modern methods.  Currently we don't
> try to inspect the kernel version at run-time, because
> version numbers are not as reliable as simply issuing
> the ioctl().


I will update hdparm regardless (version 9.40, not out yet)
to try and avoid HDIO_GETGEO_BIG when possible.  It will still
use it under some circumstances (like when sysfs isn't mounted,
which should catch 2.4.xx kernels and older), but mostly not.

Cheers

Re: [PATCH] scsi: Silence unnecessary warnings about ioctl to partition

[Paolo Bonzini]

Il 02/05/2012 12:10, Jan Kara ha scritto:
> Sometimes, warnings about ioctls to partition happen often enough that they
> form majority of the warnings in the kernel log and users complain. In some
> cases warnings are about ioctls such as SG_IO so it's not good to get rid of
> the warnings completely as they can ease debugging of userspace problems
> when ioctl is refused.
> 
> Since I have seen warnings from lots of commands, including some proprietary
> userspace applications, I don't think disallowing the ioctls for processes
> with CAP_SYS_RAWIO will happen in the near future if ever. So lets just
> stop warning for processes with CAP_SYS_RAWIO for which ioctl is allowed.
> 
> CC: Paolo Bonzini <pbonzini@redhat.com>
> CC: Jens Axboe <axboe@kernel.dk>
> CC: James Bottomley <JBottomley@parallels.com>
> CC: linux-scsi@vger.kernel.org
> Signed-off-by: Jan Kara <jack@suse.cz>
> ---
>  block/scsi_ioctl.c |    5 ++++-
>  1 files changed, 4 insertions(+), 1 deletions(-)
> 
> diff --git a/block/scsi_ioctl.c b/block/scsi_ioctl.c
> index 260fa80..9a87daa 100644
> --- a/block/scsi_ioctl.c
> +++ b/block/scsi_ioctl.c
> @@ -721,11 +721,14 @@ int scsi_verify_blk_ioctl(struct block_device *bd, unsigned int cmd)
>  		break;
>  	}
>  
> +	if (capable(CAP_SYS_RAWIO))
> +		return 0;
> +
>  	/* 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 : -ENOIOCTLCMD;
> +	return -ENOIOCTLCMD;
>  }
>  EXPORT_SYMBOL(scsi_verify_blk_ioctl);
>  
> 

Ping?

Paolo

Re: [PATCH] scsi: Silence unnecessary warnings about ioctl to partition

[Jan Kara]

On Fri 15-06-12 10:14:12, Paolo Bonzini wrote:
> Il 02/05/2012 12:10, Jan Kara ha scritto:
> > Sometimes, warnings about ioctls to partition happen often enough that they
> > form majority of the warnings in the kernel log and users complain. In some
> > cases warnings are about ioctls such as SG_IO so it's not good to get rid of
> > the warnings completely as they can ease debugging of userspace problems
> > when ioctl is refused.
> > 
> > Since I have seen warnings from lots of commands, including some proprietary
> > userspace applications, I don't think disallowing the ioctls for processes
> > with CAP_SYS_RAWIO will happen in the near future if ever. So lets just
> > stop warning for processes with CAP_SYS_RAWIO for which ioctl is allowed.
> > 
> > CC: Paolo Bonzini <pbonzini@redhat.com>
> > CC: Jens Axboe <axboe@kernel.dk>
> > CC: James Bottomley <JBottomley@parallels.com>
> > CC: linux-scsi@vger.kernel.org
> > Signed-off-by: Jan Kara <jack@suse.cz>
> > ---
> >  block/scsi_ioctl.c |    5 ++++-
> >  1 files changed, 4 insertions(+), 1 deletions(-)
> > 
> > diff --git a/block/scsi_ioctl.c b/block/scsi_ioctl.c
> > index 260fa80..9a87daa 100644
> > --- a/block/scsi_ioctl.c
> > +++ b/block/scsi_ioctl.c
> > @@ -721,11 +721,14 @@ int scsi_verify_blk_ioctl(struct block_device *bd, unsigned int cmd)
> >  		break;
> >  	}
> >  
> > +	if (capable(CAP_SYS_RAWIO))
> > +		return 0;
> > +
> >  	/* 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 : -ENOIOCTLCMD;
> > +	return -ENOIOCTLCMD;
> >  }
> >  EXPORT_SYMBOL(scsi_verify_blk_ioctl);
> >  
> > 
> 
> Ping?
  Good that you bring this up :). I've asked the customer about whether
they can find out the particular SG_IO ioctl Veritas issues but never heard
back. So will we merge my patch? I remember you gave it reluctant ack once
;).

									Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

Re: [PATCH] scsi: Silence unnecessary warnings about ioctl to partition

[Nick Bowler]

On 2012-06-15 16:23 +0200, Jan Kara wrote:
> On Fri 15-06-12 09:58:25, Nick Bowler wrote:
> > On 2012-06-15 12:51 +0200, Jens Axboe wrote:
> > > On 06/15/2012 12:50 PM, Jan Kara wrote:
> > > > Sometimes, warnings about ioctls to partition happen often enough that they
> > > > form majority of the warnings in the kernel log and users complain. In some
> > > > cases warnings are about ioctls such as SG_IO so it's not good to get rid of
> > > > the warnings completely as they can ease debugging of userspace problems
> > > > when ioctl is refused.
> > > > 
> > > > Since I have seen warnings from lots of commands, including some proprietary
> > > > userspace applications, I don't think disallowing the ioctls for processes
> > > > with CAP_SYS_RAWIO will happen in the near future if ever. So lets just
> > > > stop warning for processes with CAP_SYS_RAWIO for which ioctl is allowed.
> > > 
> > > Agree, merged, finally.
> > 
> > Hurray!
> > 
> > This should be -stable material as well, as >=3.2.y kernel versions are
> > affected (I think 3.0.y is OK?).
>   3.0 also spits those warnings. But I'm not convinced this is really a
> -stable material. I don't see anybody loosing sleep over this...

These messages are quite annoying on my raid boxes (currently running
3.2.y) where mdadm spews a whole pile of these whenever you do anything.

Even with the rate limiting, they make up a substantial proportion of
the total log messages.

Cheers,
-- 
Nick Bowler, Elliptic Technologies (http://www.elliptictech.com/)

Re: [PATCH] scsi: Silence unnecessary warnings about ioctl to partition

[Jan Kara]

On Fri 15-06-12 09:58:25, Nick Bowler wrote:
> On 2012-06-15 12:51 +0200, Jens Axboe wrote:
> > On 06/15/2012 12:50 PM, Jan Kara wrote:
> > > Sometimes, warnings about ioctls to partition happen often enough that they
> > > form majority of the warnings in the kernel log and users complain. In some
> > > cases warnings are about ioctls such as SG_IO so it's not good to get rid of
> > > the warnings completely as they can ease debugging of userspace problems
> > > when ioctl is refused.
> > > 
> > > Since I have seen warnings from lots of commands, including some proprietary
> > > userspace applications, I don't think disallowing the ioctls for processes
> > > with CAP_SYS_RAWIO will happen in the near future if ever. So lets just
> > > stop warning for processes with CAP_SYS_RAWIO for which ioctl is allowed.
> > 
> > Agree, merged, finally.
> 
> Hurray!
> 
> This should be -stable material as well, as >=3.2.y kernel versions are
> affected (I think 3.0.y is OK?).
  3.0 also spits those warnings. But I'm not convinced this is really a
-stable material. I don't see anybody loosing sleep over this...

								Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

Re: [PATCH] scsi: Silence unnecessary warnings about ioctl to partition

[Paolo Bonzini]

> This should be -stable material as well, as >=3.2.y kernel versions
> are affected (I think 3.0.y is OK?).

Not sure, but -ENOIOCTLCMD needed to be changed to -ENOTTY on some
kernel versions (but the conflicts will make this pretty evident).

Paolo

Re: [PATCH] scsi: Silence unnecessary warnings about ioctl to partition

[Nick Bowler]

On 2012-06-15 12:51 +0200, Jens Axboe wrote:
> On 06/15/2012 12:50 PM, Jan Kara wrote:
> > Sometimes, warnings about ioctls to partition happen often enough that they
> > form majority of the warnings in the kernel log and users complain. In some
> > cases warnings are about ioctls such as SG_IO so it's not good to get rid of
> > the warnings completely as they can ease debugging of userspace problems
> > when ioctl is refused.
> > 
> > Since I have seen warnings from lots of commands, including some proprietary
> > userspace applications, I don't think disallowing the ioctls for processes
> > with CAP_SYS_RAWIO will happen in the near future if ever. So lets just
> > stop warning for processes with CAP_SYS_RAWIO for which ioctl is allowed.
> 
> Agree, merged, finally.

Hurray!

This should be -stable material as well, as >=3.2.y kernel versions are
affected (I think 3.0.y is OK?).

Thanks,
-- 
Nick Bowler, Elliptic Technologies (http://www.elliptictech.com/)

Re: [PATCH] scsi: Silence unnecessary warnings about ioctl to partition

[Alan Cox]

On Fri, 15 Jun 2012 12:50:57 +0200
Jan Kara <jack@suse.cz> wrote:

> Sometimes, warnings about ioctls to partition happen often enough that they
> form majority of the warnings in the kernel log and users complain. In some
> cases warnings are about ioctls such as SG_IO so it's not good to get rid of
> the warnings completely as they can ease debugging of userspace problems
> when ioctl is refused.
> 
> Since I have seen warnings from lots of commands, including some proprietary
> userspace applications, I don't think disallowing the ioctls for processes
> with CAP_SYS_RAWIO will happen in the near future if ever. So lets just
> stop warning for processes with CAP_SYS_RAWIO for which ioctl is allowed.
> 
> CC: Paolo Bonzini <pbonzini@redhat.com>
> CC: Jens Axboe <axboe@kernel.dk>
> CC: James Bottomley <JBottomley@parallels.com>
> CC: linux-scsi@vger.kernel.org
> Acked-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Jan Kara <jack@suse.cz>

Acked-by: Alan Cox <alan@linux.intel.com>

Re: [PATCH] scsi: Silence unnecessary warnings about ioctl to partition

[Jens Axboe]

On 06/15/2012 12:50 PM, Jan Kara wrote:
> Sometimes, warnings about ioctls to partition happen often enough that they
> form majority of the warnings in the kernel log and users complain. In some
> cases warnings are about ioctls such as SG_IO so it's not good to get rid of
> the warnings completely as they can ease debugging of userspace problems
> when ioctl is refused.
> 
> Since I have seen warnings from lots of commands, including some proprietary
> userspace applications, I don't think disallowing the ioctls for processes
> with CAP_SYS_RAWIO will happen in the near future if ever. So lets just
> stop warning for processes with CAP_SYS_RAWIO for which ioctl is allowed.

Agree, merged, finally.

-- 
Jens Axboe

[PATCH] scsi: Silence unnecessary warnings about ioctl to partition

[Jan Kara]

Sometimes, warnings about ioctls to partition happen often enough that they
form majority of the warnings in the kernel log and users complain. In some
cases warnings are about ioctls such as SG_IO so it's not good to get rid of
the warnings completely as they can ease debugging of userspace problems
when ioctl is refused.

Since I have seen warnings from lots of commands, including some proprietary
userspace applications, I don't think disallowing the ioctls for processes
with CAP_SYS_RAWIO will happen in the near future if ever. So lets just
stop warning for processes with CAP_SYS_RAWIO for which ioctl is allowed.

CC: Paolo Bonzini <pbonzini@redhat.com>
CC: Jens Axboe <axboe@kernel.dk>
CC: James Bottomley <JBottomley@parallels.com>
CC: linux-scsi@vger.kernel.org
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Jan Kara <jack@suse.cz>
---
 block/scsi_ioctl.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

 Resending rebased on 3.5-rc2 with and Ack from Paolo.

diff --git a/block/scsi_ioctl.c b/block/scsi_ioctl.c
index 260fa80..9a87daa 100644
--- a/block/scsi_ioctl.c
+++ b/block/scsi_ioctl.c
@@ -721,11 +721,14 @@ int scsi_verify_blk_ioctl(struct block_device *bd, unsigned int cmd)
 		break;
 	}
 
+	if (capable(CAP_SYS_RAWIO))
+		return 0;
+
 	/* 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 : -ENOIOCTLCMD;
+	return -ENOIOCTLCMD;
 }
 EXPORT_SYMBOL(scsi_verify_blk_ioctl);
 
-- 
1.7.1

[PATCH] scsi: Silence unnecessary warnings about ioctl to partition

[Jan Kara]

Sometimes, warnings about ioctls to partition happen often enough that they
form majority of the warnings in the kernel log and users complain. In some
cases warnings are about ioctls such as SG_IO so it's not good to get rid of
the warnings completely as they can ease debugging of userspace problems
when ioctl is refused.

Since I have seen warnings from lots of commands, including some proprietary
userspace applications, I don't think disallowing the ioctls for processes
with CAP_SYS_RAWIO will happen in the near future if ever. So lets just
stop warning for processes with CAP_SYS_RAWIO for which ioctl is allowed.

CC: Paolo Bonzini <pbonzini@redhat.com>
CC: Jens Axboe <axboe@kernel.dk>
CC: James Bottomley <JBottomley@parallels.com>
CC: linux-scsi@vger.kernel.org
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Jan Kara <jack@suse.cz>
---
 block/scsi_ioctl.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

 Resending rebased on 3.5-rc2 with and Ack from Paolo.

diff --git a/block/scsi_ioctl.c b/block/scsi_ioctl.c
index 260fa80..9a87daa 100644
--- a/block/scsi_ioctl.c
+++ b/block/scsi_ioctl.c
@@ -721,11 +721,14 @@ int scsi_verify_blk_ioctl(struct block_device *bd, unsigned int cmd)
 		break;
 	}
 
+	if (capable(CAP_SYS_RAWIO))
+		return 0;
+
 	/* 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 : -ENOIOCTLCMD;
+	return -ENOIOCTLCMD;
 }
 EXPORT_SYMBOL(scsi_verify_blk_ioctl);
 
-- 
1.7.1