Ioctl warning for a partition

Ioctl warning for a partition

[Jan Kara]

  Hello,

  with your patches (commit 0bfc96cb in particular) to limit ioctl on
partitions we get warning:
dd: sending ioctl 80306d02 to a partition!
because dd checks whether given device is a tape (it's MTIOCGET ioctl).
It's easy enough to silence the warning the same way as
CDROM_GET_CAPABILITY since the ioctl is safe but it's not so simple for
32-bit userspace.  MTIOCGET32 is defined only in fs/compat_ioctl.c so we
cannot easily add it to scsi_verify_blk_ioctl(). Any opinion how to cleanly
solve this? The only idea I had was to define compat structures and ioctl
numbers in a special header and use it both in fs/compat_ioctl.c and in
block/scsi_ioctl.c.

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

Re: Ioctl warning for a partition

[Linus Torvalds]

On Thu, Jan 26, 2012 at 2:30 PM, Jan Kara <jack@suse.cz> wrote:
>
> It's easy enough to silence the warning the same way as
> CDROM_GET_CAPABILITY since the ioctl is safe but it's not so simple for
> 32-bit userspace.  MTIOCGET32 is defined only in fs/compat_ioctl.c so we
> cannot easily add it to scsi_verify_blk_ioctl(). Any opinion how to cleanly
> solve this? The only idea I had was to define compat structures and ioctl
> numbers in a special header and use it both in fs/compat_ioctl.c and in
> block/scsi_ioctl.c.

I suspect we can just remove the warning entirely - once we've gotten
enough coverage with the -rc kernels that people (me in particular)
are happy that no normal load really needs it, and returning an error
is fine.

So I don't really consider the warning to be something long-term - I
wanted it to make sure that some random binary in some odd
distribution wouldn't break in mysterious ways that would take a lot
of debugging to find. And so that we really know what we end up
blocking in practice.

I'm not sure how good the -rc kernel coverage is, but I think it's
good enough that we can drop the warning before doing a real 3.3
release. And I don't think the stable kernel versions ever got that
warning printout, did they? That would be great for coverage, of
course, if they did.

                      Linus

Re: Ioctl warning for a partition

[Michael Tokarev]

On 27.01.2012 03:01, Linus Torvalds wrote:
> On Thu, Jan 26, 2012 at 2:30 PM, Jan Kara <jack@suse.cz> wrote:
>>
>> It's easy enough to silence the warning the same way as
>> CDROM_GET_CAPABILITY since the ioctl is safe but it's not so simple for
>> 32-bit userspace.  MTIOCGET32 is defined only in fs/compat_ioctl.c so we
>> cannot easily add it to scsi_verify_blk_ioctl(). Any opinion how to cleanly
>> solve this? The only idea I had was to define compat structures and ioctl
>> numbers in a special header and use it both in fs/compat_ioctl.c and in
>> block/scsi_ioctl.c.
> 
> I suspect we can just remove the warning entirely - once we've gotten
> enough coverage with the -rc kernels that people (me in particular)
> are happy that no normal load really needs it, and returning an error
> is fine.
> 
> So I don't really consider the warning to be something long-term - I
> wanted it to make sure that some random binary in some odd
> distribution wouldn't break in mysterious ways that would take a lot
> of debugging to find. And so that we really know what we end up
> blocking in practice.
> 
> I'm not sure how good the -rc kernel coverage is, but I think it's
> good enough that we can drop the warning before doing a real 3.3
> release. And I don't think the stable kernel versions ever got that
> warning printout, did they? That would be great for coverage, of
> course, if they did.

They did, 3.0 and 3.2.

For example, 3.0.18:

[  610.488489] kvm: sending ioctl 5326 to a partition!
[  610.488540] kvm: sending ioctl 80200204 to a partition!

mdadm ioctls reported in various places apparently got fixed by
ENOTTY/ENOIOCTLCMD change.

Thanks,

/mjt

Re: Ioctl warning for a partition

[Paolo Bonzini]

On 01/27/2012 12:01 AM, Linus Torvalds wrote:
> I suspect we can just remove the warning entirely - once we've gotten
> enough coverage with the -rc kernels that people (me in particular)
> are happy that no normal load really needs it, and returning an error
> is fine.
>
> So I don't really consider the warning to be something long-term - I
> wanted it to make sure that some random binary in some odd
> distribution wouldn't break in mysterious ways that would take a lot
> of debugging to find. And so that we really know what we end up
> blocking in practice.
>
> I'm not sure how good the -rc kernel coverage is, but I think it's
> good enough that we can drop the warning before doing a real 3.3
> release. And I don't think the stable kernel versions ever got that
> warning printout, did they? That would be great for coverage, of
> course, if they did.

They did.

Here is the list I put together from people who contacted me about the 
warning:

BLKFLSBUF, BLKROSET:
	These two can be passed down to ops->ioctl even though
	they are generic block layer ioctls.  Nothing overrides them
	*and* calls scsi_verify_blk_ioctl so we aren't breaking
	anything.  However, these ioctls are obviously good for
	partitions so we should add them to the whitelist.

CDROM_DRIVE_STATUS, FDGETPRM, MTIOCGET32:
	These three are used for detection of devices that do not
	support	partitions.  They can be handled the same as
	CDROM_GET_CAPABILITY, i.e. we can fail them and not break
	anything.

RAID_VERSION:
	Also used for detection, however (unlike floppies and CD-ROMs)
	RAID devices do have partitions.  RAID partitions do not
	support SCSI ioctls and thus do not call scsi_verify_blk_ioctl,
	which means we can fail this one right away too.

I was preparing a patch to update the whitelist, but I think I will wait 
a couple more weeks and remove the warning altogether.

Paolo

Re: Ioctl warning for a partition

[Henrique de Moraes Holschuh]

Hi Michael!

On Fri, 27 Jan 2012, Michael Tokarev wrote:

> On 27.01.2012 03:01, Linus Torvalds wrote:
> > On Thu, Jan 26, 2012 at 2:30 PM, Jan Kara <jack@suse.cz> wrote:
> >>
> >> It's easy enough to silence the warning the same way as
> >> CDROM_GET_CAPABILITY since the ioctl is safe but it's not so simple for
> >> 32-bit userspace.  MTIOCGET32 is defined only in fs/compat_ioctl.c so we
> >> cannot easily add it to scsi_verify_blk_ioctl(). Any opinion how to cleanly
> >> solve this? The only idea I had was to define compat structures and ioctl
> >> numbers in a special header and use it both in fs/compat_ioctl.c and in
> >> block/scsi_ioctl.c.
> > 
> > I suspect we can just remove the warning entirely - once we've gotten
> > enough coverage with the -rc kernels that people (me in particular)
> > are happy that no normal load really needs it, and returning an error
> > is fine.
> > 
> > So I don't really consider the warning to be something long-term - I
> > wanted it to make sure that some random binary in some odd
> > distribution wouldn't break in mysterious ways that would take a lot
> > of debugging to find. And so that we really know what we end up
> > blocking in practice.
> > 
> > I'm not sure how good the -rc kernel coverage is, but I think it's
> > good enough that we can drop the warning before doing a real 3.3
> > release. And I don't think the stable kernel versions ever got that
> > warning printout, did they? That would be great for coverage, of
> > course, if they did.
> 
> They did, 3.0 and 3.2.
> 
> For example, 3.0.18:
> 
> [  610.488489] kvm: sending ioctl 5326 to a partition!
> [  610.488540] kvm: sending ioctl 80200204 to a partition!
> 
> mdadm ioctls reported in various places apparently got fixed by
> ENOTTY/ENOIOCTLCMD change.

Does that fix still need to be backported to -stable?

Linux v3.0.18 + Debian stable userspace (mdadm v3.1.4) causes this on
boot (when mdadm runs in the initramfs):

mdadm: sending ioctl 1261 to a partition!
mdadm: sending ioctl 1261 to a partition!
mdadm: sending ioctl 1261 to a partition!
mdadm: sending ioctl 1261 to a partition!
mdadm: sending ioctl 1261 to a partition!
mdadm: sending ioctl 1261 to a partition!
mdadm: sending ioctl 1261 to a partition!
mdadm: sending ioctl 1261 to a partition!
mdadm: sending ioctl 800c0910 to a partition!
mdadm: sending ioctl 800c0910 to a partition!

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

Re: Ioctl warning for a partition

[Henrique de Moraes Holschuh]

On Fri, 27 Jan 2012, Henrique de Moraes Holschuh wrote:
> On Fri, 27 Jan 2012, Michael Tokarev wrote:
> > mdadm ioctls reported in various places apparently got fixed by
> > ENOTTY/ENOIOCTLCMD change.
> 
> Does that fix still need to be backported to -stable?
> 
> Linux v3.0.18 + Debian stable userspace (mdadm v3.1.4) causes this on
> boot (when mdadm runs in the initramfs):
> 
> mdadm: sending ioctl 1261 to a partition!
> mdadm: sending ioctl 1261 to a partition!
> mdadm: sending ioctl 1261 to a partition!
> mdadm: sending ioctl 1261 to a partition!
> mdadm: sending ioctl 1261 to a partition!
> mdadm: sending ioctl 1261 to a partition!
> mdadm: sending ioctl 1261 to a partition!
> mdadm: sending ioctl 1261 to a partition!
> mdadm: sending ioctl 800c0910 to a partition!
> mdadm: sending ioctl 800c0910 to a partition!

This is on x86-64/amd64.

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

Re: Ioctl warning for a partition

[Paolo Bonzini]

On 01/27/2012 08:28 PM, Henrique de Moraes Holschuh wrote:
> Does that fix still need to be backported to -stable?
>
> Linux v3.0.18 + Debian stable userspace (mdadm v3.1.4) causes this on
> boot (when mdadm runs in the initramfs):
>
> mdadm: sending ioctl 1261 to a partition!
> mdadm: sending ioctl 1261 to a partition!
> mdadm: sending ioctl 1261 to a partition!
> mdadm: sending ioctl 1261 to a partition!
> mdadm: sending ioctl 1261 to a partition!
> mdadm: sending ioctl 1261 to a partition!
> mdadm: sending ioctl 1261 to a partition!
> mdadm: sending ioctl 1261 to a partition!
> mdadm: sending ioctl 800c0910 to a partition!
> mdadm: sending ioctl 800c0910 to a partition!

The warnings are intended; the kernel will behave as before they were 
introduced.  I'm monitoring reports and looking at them as they come in. 
  They are harmless as long as the ioctls have always failed, and so far 
this has always been the case.  As soon as we are reasonably certain, 
the warnings will go away.

Paolo

Re: Ioctl warning for a partition

[Henrique de Moraes Holschuh]

On Sat, 28 Jan 2012, Paolo Bonzini wrote:
> On 01/27/2012 08:28 PM, Henrique de Moraes Holschuh wrote:
> >Does that fix still need to be backported to -stable?
> >
> >Linux v3.0.18 + Debian stable userspace (mdadm v3.1.4) causes this on
> >boot (when mdadm runs in the initramfs):
> >
> >mdadm: sending ioctl 1261 to a partition!
> >mdadm: sending ioctl 1261 to a partition!
> >mdadm: sending ioctl 1261 to a partition!
> >mdadm: sending ioctl 1261 to a partition!
> >mdadm: sending ioctl 1261 to a partition!
> >mdadm: sending ioctl 1261 to a partition!
> >mdadm: sending ioctl 1261 to a partition!
> >mdadm: sending ioctl 1261 to a partition!
> >mdadm: sending ioctl 800c0910 to a partition!
> >mdadm: sending ioctl 800c0910 to a partition!
> 
> The warnings are intended; the kernel will behave as before they
> were introduced.  I'm monitoring reports and looking at them as they
> come in.  They are harmless as long as the ioctls have always
> failed, and so far this has always been the case.  As soon as we are
> reasonably certain, the warnings will go away.

I am aware of that.  It is just that a previous message in this thread
implied the ioctls from mdadm had already been dealth with (i.e.  should
not be issuing warnings anymore).

After it has been tested enough in mainline, please make sure to direct
all patches related to this issue to stable@v.k.o.

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh