[PATCH] fs/compat_ioctl.c: add missing FS_IOC_FIEMAP support

[PATCH] fs/compat_ioctl.c: add missing FS_IOC_FIEMAP support

[Mark Lord]

Add support for the FIEMAP ioctl for 32-bit user on 64-bit kernel.

When using a 32-bit runtime on top of a 64-bit kernel,
programs like "filefrag" and "hdparm --fibmap" do not work correctly.

This is because there's no compat ioctl entry for the FIEMAP call.
FIEMAP returns file extent info, similar to FIBMAP (but better).

Since FIBMAP itself is b0rked on ext4, this leaves no way for a 32-bit
program to reliably get detailed block information for a file
when run on top of a 64-bit kernel.  This patch addresses the issue.

Once upstream, this patch could also be a candidate for -stable.

Signed-off-by: Mark Lord <mlord@pobox.com>

--- old/fs/compat_ioctl.c	2009-08-01 10:47:16.601066905 -0400
+++ linux/fs/compat_ioctl.c	2009-08-01 10:49:23.054387926 -0400
@@ -35,6 +35,7 @@
 #include <linux/falloc.h>
 #include <linux/fs.h>
 #include <linux/file.h>
+#include <linux/fiemap.h>
 #include <linux/ppp_defs.h>
 #include <linux/if_ppp.h>
 #include <linux/if_pppox.h>
@@ -1907,6 +1908,7 @@
 COMPATIBLE_IOCTL(FIONREAD)  /* This is also TIOCINQ */
 /* 0x00 */
 COMPATIBLE_IOCTL(FIBMAP)
+COMPATIBLE_IOCTL(FS_IOC_FIEMAP)
 COMPATIBLE_IOCTL(FIGETBSZ)
 /* 'X' - originally XFS but some now in the VFS */
 COMPATIBLE_IOCTL(FIFREEZE)
@@ -2805,6 +2807,7 @@
 		goto out_fput;
 #endif
 
+	case FS_IOC_FIEMAP:
 	case FIBMAP:
 	case FIGETBSZ:
 	case FIONREAD:

Re: [PATCH] fs/compat_ioctl.c: add missing FS_IOC_FIEMAP support

[Mark Lord]

Mark Lord wrote:
..
> Since FIBMAP itself is b0rked on ext4, this leaves no way for a 32-bit
..

To stave off the inevitable question, FIBMAP doesn't appear to cope well
with allocated/uncommitted blocks.  Creating a file with fallocate(),
and then immediately running FIBMAP, will not give any allocation data.

Doing a sync() after the fallocate() allows FIBMAP to work again.

FIEMAP doesn't require the sync().

Cheers

Re: [PATCH] fs/compat_ioctl.c: add missing FS_IOC_FIEMAP support

[Mark Lord]

(resending, no ack from anyone first time around).

Add support for the FIEMAP ioctl for 32-bit user on 64-bit kernel.

When using a 32-bit runtime on top of a 64-bit kernel,
programs like "filefrag" and "hdparm --fibmap" do not work correctly.

This is because there's no compat ioctl entry for the FIEMAP call.
FIEMAP returns file extent info, similar to FIBMAP (but better).

Since FIBMAP itself is b0rked on ext4, this leaves no way for a 32-bit
program to reliably get detailed block information for a file
when run on top of a 64-bit kernel.  This patch addresses the issue.

Once upstream, this patch could also be a candidate for -stable.

Signed-off-by: Mark Lord <mlord@pobox.com>

Add support for the FIEMAP ioctl for 32-bit user on 64-bit kernel.

When using a 32-bit runtime on top of a 64-bit kernel,
programs like "filefrag" and "hdparm --fibmap" do not work correctly.

This is because there's no compat ioctl entry for the FIEMAP call.
FIEMAP returns file extent info, similar to FIBMAP (but better).

Since FIBMAP itself is b0rked on ext4, this leaves no way for a 32-bit
program to reliably get detailed block information for a file
when run on top of a 64-bit kernel.  This patch addresses the issue.

Once upstream, this patch could also be a candidate for -stable.

Signed-off-by: Mark Lord <mlord@pobox.com> 

--- old/fs/compat_ioctl.c	2009-08-01 10:47:16.601066905 -0400
+++ linux/fs/compat_ioctl.c	2009-08-01 10:49:23.054387926 -0400
@@ -35,6 +35,7 @@
 #include <linux/falloc.h>
 #include <linux/fs.h>
 #include <linux/file.h>
+#include <linux/fiemap.h>
 #include <linux/ppp_defs.h>
 #include <linux/if_ppp.h>
 #include <linux/if_pppox.h>
@@ -1907,6 +1908,7 @@
 COMPATIBLE_IOCTL(FIONREAD)  /* This is also TIOCINQ */
 /* 0x00 */
 COMPATIBLE_IOCTL(FIBMAP)
+COMPATIBLE_IOCTL(FS_IOC_FIEMAP)
 COMPATIBLE_IOCTL(FIGETBSZ)
 /* 'X' - originally XFS but some now in the VFS */
 COMPATIBLE_IOCTL(FIFREEZE)
@@ -2805,6 +2807,7 @@
 		goto out_fput;
 #endif
 
+	case FS_IOC_FIEMAP:
 	case FIBMAP:
 	case FIGETBSZ:
 	case FIONREAD:

Re: [PATCH] fs/compat_ioctl.c: add missing FS_IOC_FIEMAP support

[Arnd Bergmann]

On Monday 03 August 2009, Mark Lord wrote:
> (resending, no ack from anyone first time around).

I actually wrote a lengthy reply on how I think the code around it
should be done differently and then realized why we had done it
that way originally and did not send out my reply.

Your addition looks ok, the data structures are compatible
on all architectures.

> Add support for the FIEMAP ioctl for 32-bit user on 64-bit kernel.
> 
> When using a 32-bit runtime on top of a 64-bit kernel,
> programs like "filefrag" and "hdparm --fibmap" do not work correctly.
> 
> This is because there's no compat ioctl entry for the FIEMAP call.
> FIEMAP returns file extent info, similar to FIBMAP (but better).
> 
> Since FIBMAP itself is b0rked on ext4, this leaves no way for a 32-bit
> program to reliably get detailed block information for a file
> when run on top of a 64-bit kernel.  This patch addresses the issue.
> 
> Once upstream, this patch could also be a candidate for -stable.
> 
> Signed-off-by: Mark Lord <mlord@pobox.com>

Reviewed-by: Arnd Bergmann <arnd@arndb.de>

Re: [PATCH] fs/compat_ioctl.c: add missing FS_IOC_FIEMAP support

[Eric Sandeen]

Mark Lord wrote:
> (resending, no ack from anyone first time around).

I had previously sent a fix for this to the ext4 list as well, although
w/o the added case for compat_ioctl or the extra #include.

Because this ioctl should be 100% compat everywhere, I don't -think-
it's needed, and

http://marc.info/?l=linux-ext4&m=124872536713005&w=2

suffices....

But if not, minor nitpick, you should move the COMPAT_IOCTL under the
"little f" comment.

-Eric


> Add support for the FIEMAP ioctl for 32-bit user on 64-bit kernel.
> 
> When using a 32-bit runtime on top of a 64-bit kernel,
> programs like "filefrag" and "hdparm --fibmap" do not work correctly.
> 
> This is because there's no compat ioctl entry for the FIEMAP call.
> FIEMAP returns file extent info, similar to FIBMAP (but better).
> 
> Since FIBMAP itself is b0rked on ext4, this leaves no way for a 32-bit
> program to reliably get detailed block information for a file
> when run on top of a 64-bit kernel.  This patch addresses the issue.
> 
> Once upstream, this patch could also be a candidate for -stable.
> 
> Signed-off-by: Mark Lord <mlord@pobox.com>
> 
> Add support for the FIEMAP ioctl for 32-bit user on 64-bit kernel.
> 
> When using a 32-bit runtime on top of a 64-bit kernel,
> programs like "filefrag" and "hdparm --fibmap" do not work correctly.
> 
> This is because there's no compat ioctl entry for the FIEMAP call.
> FIEMAP returns file extent info, similar to FIBMAP (but better).
> 
> Since FIBMAP itself is b0rked on ext4, this leaves no way for a 32-bit
> program to reliably get detailed block information for a file
> when run on top of a 64-bit kernel.  This patch addresses the issue.
> 
> Once upstream, this patch could also be a candidate for -stable.
> 
> Signed-off-by: Mark Lord <mlord@pobox.com> 
> 
> --- old/fs/compat_ioctl.c	2009-08-01 10:47:16.601066905 -0400
> +++ linux/fs/compat_ioctl.c	2009-08-01 10:49:23.054387926 -0400
> @@ -35,6 +35,7 @@
>  #include <linux/falloc.h>
>  #include <linux/fs.h>
>  #include <linux/file.h>
> +#include <linux/fiemap.h>
>  #include <linux/ppp_defs.h>
>  #include <linux/if_ppp.h>
>  #include <linux/if_pppox.h>
> @@ -1907,6 +1908,7 @@
>  COMPATIBLE_IOCTL(FIONREAD)  /* This is also TIOCINQ */
>  /* 0x00 */
>  COMPATIBLE_IOCTL(FIBMAP)
> +COMPATIBLE_IOCTL(FS_IOC_FIEMAP)
>  COMPATIBLE_IOCTL(FIGETBSZ)
>  /* 'X' - originally XFS but some now in the VFS */
>  COMPATIBLE_IOCTL(FIFREEZE)
> @@ -2805,6 +2807,7 @@
>  		goto out_fput;
>  #endif
>  
> +	case FS_IOC_FIEMAP:
>  	case FIBMAP:
>  	case FIGETBSZ:
>  	case FIONREAD:
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] fs/compat_ioctl.c: add missing FS_IOC_FIEMAP support

[Mark Lord]

Eric Sandeen wrote:
> Mark Lord wrote:
>> (resending, no ack from anyone first time around).
> 
> I had previously sent a fix for this to the ext4 list as well, although
> w/o the added case for compat_ioctl or the extra #include.
> 
> Because this ioctl should be 100% compat everywhere, I don't -think-
> it's needed, and
> 
> http://marc.info/?l=linux-ext4&m=124872536713005&w=2
> 
> suffices....
..

Well, whichever of the two works best for the maintainers.

We need *something* for it upstream, and probably back in -stable too.
Otherwise this prevents using 64-bit kernels on 32-bit userland,
as Linus likes to recommend so often.  ;)

Cheers

Re: [PATCH] fs/compat_ioctl.c: add missing FS_IOC_FIEMAP support

[Eric Sandeen]

Mark Lord wrote:
> Eric Sandeen wrote:
>> Mark Lord wrote:
>>> (resending, no ack from anyone first time around).
>> I had previously sent a fix for this to the ext4 list as well, although
>> w/o the added case for compat_ioctl or the extra #include.
>>
>> Because this ioctl should be 100% compat everywhere, I don't -think-
>> it's needed, and
>>
>> http://marc.info/?l=linux-ext4&m=124872536713005&w=2
>>
>> suffices....
> ..
> 
> Well, whichever of the two works best for the maintainers.
> 
> We need *something* for it upstream, and probably back in -stable too.
> Otherwise this prevents using 64-bit kernels on 32-bit userland,
> as Linus likes to recommend so often.  ;)
> 
> Cheers
> 

Heh, it's probably far from the only ioctl missing a compat handler, but
yeah.  :)

-Eric

Re: [PATCH] fs/compat_ioctl.c: add missing FS_IOC_FIEMAP support

[Andrew Morton]

On Mon, 03 Aug 2009 18:07:53 -0400
Mark Lord <lkml@rtr.ca> wrote:

> Eric Sandeen wrote:
> > Mark Lord wrote:
> >> (resending, no ack from anyone first time around).
> > 
> > I had previously sent a fix for this to the ext4 list as well, although
> > w/o the added case for compat_ioctl or the extra #include.
> > 
> > Because this ioctl should be 100% compat everywhere, I don't -think-
> > it's needed, and
> > 
> > http://marc.info/?l=linux-ext4&m=124872536713005&w=2

mutter. I don't read linux-fsdevel much.

> > suffices....
> ..
> 
> Well, whichever of the two works best for the maintainers.
> 
> We need *something* for it upstream, and probably back in -stable too.
> Otherwise this prevents using 64-bit kernels on 32-bit userland,
> as Linus likes to recommend so often.  ;)

OK, here's what I have, with a somewhat reworked changelog.

I assumed that "Josef" == josef@redhat.com.

Arnd, could you please check that it still looks OK?

Thanks.


From: Eric Sandeen <sandeen@redhat.com>

The FIEMAP_IOC_FIEMAP mapping ioctl was missing a 32-bit compat handler,
which means that 32-bit suerspace on 64-bit kernels cannot use this ioctl
command.

The structure is nicely aligned, padded, and sized, so it is just this
simple.

Tested w/ 32-bit ioctl tester (from Josef) on a 64-bit kernel on ext4.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Cc: <linux-ext4@vger.kernel.org>
Cc: Mark Lord <lkml@rtr.ca>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Josef Bacik <josef@redhat.com>
Cc: Jan Kara <jack@suse.cz>
Cc: <stable@kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 fs/compat_ioctl.c |    1 +
 1 file changed, 1 insertion(+)

diff -puN fs/compat_ioctl.c~compat_ioctl-hook-up-compat-handler-for-fiemap-ioctl fs/compat_ioctl.c
--- a/fs/compat_ioctl.c~compat_ioctl-hook-up-compat-handler-for-fiemap-ioctl
+++ a/fs/compat_ioctl.c
@@ -1838,6 +1838,7 @@ COMPATIBLE_IOCTL(FIONCLEX)
 COMPATIBLE_IOCTL(FIOASYNC)
 COMPATIBLE_IOCTL(FIONBIO)
 COMPATIBLE_IOCTL(FIONREAD)  /* This is also TIOCINQ */
+COMPATIBLE_IOCTL(FS_IOC_FIEMAP)
 /* 0x00 */
 COMPATIBLE_IOCTL(FIBMAP)
 COMPATIBLE_IOCTL(FIGETBSZ)
_

Re: [PATCH] fs/compat_ioctl.c: add missing FS_IOC_FIEMAP support

[Arnd Bergmann]

On Tuesday 04 August 2009, Andrew Morton wrote:
> > Well, whichever of the two works best for the maintainers.
> > 
> > We need something for it upstream, and probably back in -stable too.
> > Otherwise this prevents using 64-bit kernels on 32-bit userland,
> > as Linus likes to recommend so often.  ;)
> 
> OK, here's what I have, with a somewhat reworked changelog.
> 
> I assumed that "Josef" == josef@redhat.com.
> 
> Arnd, could you please check that it still looks OK?

Yes, it looks ok as well. The #include is not needed here,
and the difference in compat_sys_ioctl() is that with Eric's
patch, a file system or device driver could in theory implement
its own compat handler for FS_IOC_FIEMAP while it cannot do
that for the native ioctl.

I would like to keep the logic in compat_sys_ioctl in sync with
do_vfs_ioctl, but they have diverged already. I have an experimental
patch set to rework compat_ioctl handling that will also take care
of this.

	Arnd <><