* Ioctl warning for a partition @ 2012-01-26 22:30 Jan Kara 2012-01-26 23:01 ` Linus Torvalds 0 siblings, 1 reply; 8+ messages in thread From: Jan Kara @ 2012-01-26 22:30 UTC (permalink / raw) To: Paolo Bonzini Cc: Linus Torvalds, LKML, linux-scsi, Jens Axboe, James Bottomley, mmarek 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Ioctl warning for a partition 2012-01-26 22:30 Ioctl warning for a partition Jan Kara @ 2012-01-26 23:01 ` Linus Torvalds 2012-01-26 23:56 ` Michael Tokarev 2012-01-27 8:35 ` Paolo Bonzini 0 siblings, 2 replies; 8+ messages in thread From: Linus Torvalds @ 2012-01-26 23:01 UTC (permalink / raw) To: Jan Kara Cc: Paolo Bonzini, LKML, linux-scsi, Jens Axboe, James Bottomley, mmarek 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Ioctl warning for a partition 2012-01-26 23:01 ` Linus Torvalds @ 2012-01-26 23:56 ` Michael Tokarev 2012-01-27 19:28 ` Henrique de Moraes Holschuh 2012-01-27 8:35 ` Paolo Bonzini 1 sibling, 1 reply; 8+ messages in thread From: Michael Tokarev @ 2012-01-26 23:56 UTC (permalink / raw) To: Linus Torvalds Cc: Jan Kara, Paolo Bonzini, LKML, linux-scsi, Jens Axboe, James Bottomley, mmarek 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Ioctl warning for a partition 2012-01-26 23:56 ` Michael Tokarev @ 2012-01-27 19:28 ` Henrique de Moraes Holschuh 2012-01-27 19:37 ` Henrique de Moraes Holschuh 2012-01-28 10:22 ` Paolo Bonzini 0 siblings, 2 replies; 8+ messages in thread From: Henrique de Moraes Holschuh @ 2012-01-27 19:28 UTC (permalink / raw) To: Michael Tokarev Cc: Linus Torvalds, Jan Kara, Paolo Bonzini, LKML, linux-scsi, Jens Axboe, James Bottomley, mmarek 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Ioctl warning for a partition 2012-01-27 19:28 ` Henrique de Moraes Holschuh @ 2012-01-27 19:37 ` Henrique de Moraes Holschuh 2012-01-28 10:22 ` Paolo Bonzini 1 sibling, 0 replies; 8+ messages in thread From: Henrique de Moraes Holschuh @ 2012-01-27 19:37 UTC (permalink / raw) To: Michael Tokarev Cc: Linus Torvalds, Jan Kara, Paolo Bonzini, LKML, linux-scsi, Jens Axboe, James Bottomley, mmarek 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Ioctl warning for a partition 2012-01-27 19:28 ` Henrique de Moraes Holschuh 2012-01-27 19:37 ` Henrique de Moraes Holschuh @ 2012-01-28 10:22 ` Paolo Bonzini 2012-01-28 14:41 ` Henrique de Moraes Holschuh 1 sibling, 1 reply; 8+ messages in thread From: Paolo Bonzini @ 2012-01-28 10:22 UTC (permalink / raw) To: Henrique de Moraes Holschuh Cc: Michael Tokarev, Linus Torvalds, Jan Kara, LKML, linux-scsi, Jens Axboe, James Bottomley, mmarek 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Ioctl warning for a partition 2012-01-28 10:22 ` Paolo Bonzini @ 2012-01-28 14:41 ` Henrique de Moraes Holschuh 0 siblings, 0 replies; 8+ messages in thread From: Henrique de Moraes Holschuh @ 2012-01-28 14:41 UTC (permalink / raw) To: Paolo Bonzini Cc: Michael Tokarev, Linus Torvalds, Jan Kara, LKML, linux-scsi, Jens Axboe, James Bottomley, mmarek 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Ioctl warning for a partition 2012-01-26 23:01 ` Linus Torvalds 2012-01-26 23:56 ` Michael Tokarev @ 2012-01-27 8:35 ` Paolo Bonzini 1 sibling, 0 replies; 8+ messages in thread From: Paolo Bonzini @ 2012-01-27 8:35 UTC (permalink / raw) To: Linus Torvalds Cc: Jan Kara, LKML, linux-scsi, Jens Axboe, James Bottomley, mmarek 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2012-01-28 14:41 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2012-01-26 22:30 Ioctl warning for a partition Jan Kara 2012-01-26 23:01 ` Linus Torvalds 2012-01-26 23:56 ` Michael Tokarev 2012-01-27 19:28 ` Henrique de Moraes Holschuh 2012-01-27 19:37 ` Henrique de Moraes Holschuh 2012-01-28 10:22 ` Paolo Bonzini 2012-01-28 14:41 ` Henrique de Moraes Holschuh 2012-01-27 8:35 ` Paolo Bonzini
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).