[RFC PATCH 0/2] Fixing xfs ioctls on x32

[RFC PATCH 0/2] Fixing xfs ioctls on x32

[Nick Bowler]

An error I got running xfs_growfs on my x32 system opened this can of
worms...

This series is intended to fix all XFS ioctls which don't work from
x32 userspace applications.

Test is xfstests "check -g quick", excluding generic/081 and xfs/014
which don't behave well for me and don't seem related to XFS ioctls.

Note that glibc's fallocate wrapper appears to be buggy on x32,
which causes several test failures.  I hacked around that in xfs_io
(the fallocate syscall itself appears to work perfectly fine on x32)
to get a cleaner test run.  Obviously this bug should be fixed but
it doesn't appear to be a kernel or xfsprogs problem.

All tests are running with the same 4.20-rc6 kernel, but different
userspace installations (pure x32 versus pure i686).  The "After"
results are obtained by reloading the xfs.ko kernel module with
the patched version and running the same set of tests.

All the remaining failures on x32 appear to be aio related, or are
also failing on the i686 tests.  I have no experience with aio but
I'm going to assume those problems have nothing to do with ioctl.

Before (i686 w/ 64bit kernel):

  Failures: generic/484 xfs/191-input-validation xfs/269 xfs/495
  Failed 4 of 656 tests

Before (x32):

  Failures: generic/018 generic/075 generic/079 generic/112 generic/113
            generic/114 generic/198 generic/207 generic/210 generic/240
            generic/252 generic/386 generic/427 generic/451 generic/465
            generic/484 xfs/002 xfs/009 xfs/026 xfs/027 xfs/028 xfs/046
            xfs/054 xfs/056 xfs/059 xfs/060 xfs/062 xfs/063 xfs/066
            xfs/069 xfs/072 xfs/078 xfs/106 xfs/118 xfs/164 xfs/165
            xfs/166 xfs/170 xfs/190 xfs/191-input-validation xfs/195
            xfs/250 xfs/266 xfs/269 xfs/281 xfs/282 xfs/283 xfs/289
            xfs/296 xfs/443 xfs/449 xfs/495
  Failed 52 of 656 tests

After (i686 w/ 64bit kernel):

  Failures: generic/484 xfs/191-input-validation xfs/269 xfs/495
  Failed 4 of 656 tests

After (x32):

  Failures: generic/112 generic/113 generic/114 generic/198 generic/207
            generic/210 generic/240 generic/252 generic/427 generic/451
            generic/465 generic/484 xfs/191-input-validation xfs/269
            xfs/495
  Failed 15 of 656 tests

Nick Bowler (2):
  xfs: Fix bulkstat compat ioctls on x32 userspace.
  xfs: Fix x32 ioctls when cmd numbers differ from ia32.

 fs/xfs/xfs_ioctl32.c | 43 ++++++++++++++++++++++++++++++++++++-------
 1 file changed, 36 insertions(+), 7 deletions(-)

-- 
2.16.1

[RFC PATCH 1/2] xfs: Fix bulkstat compat ioctls on x32 userspace.

[Nick Bowler]

The bulkstat family of ioctls are problematic on x32, because there is
a mixup of native 32-bit and 64-bit conventions: the xfs_bulkstat struct
contains pointers and 32-bit integers so that fits the native 32-bit
layout fine.  However, one of those pointers is subsequently used to
refer to one of several structs which, on x32, all follow the native
64-bit way.

Fortunately the pointer chasing seems to end there, and the functions to
deal with this abstract things pretty well.  We just need a little tweak
to pass the right formatting function if called from x32 mode.

Signed-off-by: Nick Bowler <nbowler@draconx.ca>
---
 fs/xfs/xfs_ioctl32.c | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/fs/xfs/xfs_ioctl32.c b/fs/xfs/xfs_ioctl32.c
index fba115f4103a..6a759c0ffb54 100644
--- a/fs/xfs/xfs_ioctl32.c
+++ b/fs/xfs/xfs_ioctl32.c
@@ -241,6 +241,23 @@ xfs_compat_ioc_bulkstat(
 	int			done;
 	int			error;
 
+	/*
+	 * These functions and size are used later to handle individual
+	 * entries; x32 is annoying and needs different functions.
+	 */
+	inumbers_fmt_pf inumbers_func = xfs_inumbers_fmt_compat;
+	bulkstat_one_pf	bs_one_func = xfs_bulkstat_one_compat;
+	size_t bs_one_size = sizeof(compat_xfs_bstat_t);
+
+#ifdef CONFIG_X86_X32
+	if (in_x32_syscall()) {
+		/* x32 matches native amd64 bstat and inogrp layout */
+		inumbers_func = xfs_inumbers_fmt;
+		bs_one_func = xfs_bulkstat_one;
+		bs_one_size = sizeof(xfs_bstat_t);
+	}
+#endif
+
 	/* done = 1 if there are more stats to get and if bulkstat */
 	/* should be called again (unused here, but used in dmapi) */
 
@@ -272,15 +289,15 @@ xfs_compat_ioc_bulkstat(
 
 	if (cmd == XFS_IOC_FSINUMBERS_32) {
 		error = xfs_inumbers(mp, &inlast, &count,
-				bulkreq.ubuffer, xfs_inumbers_fmt_compat);
+				bulkreq.ubuffer, inumbers_func);
 	} else if (cmd == XFS_IOC_FSBULKSTAT_SINGLE_32) {
 		int res;
 
-		error = xfs_bulkstat_one_compat(mp, inlast, bulkreq.ubuffer,
-				sizeof(compat_xfs_bstat_t), NULL, &res);
+		error = bs_one_func(mp, inlast, bulkreq.ubuffer,
+				bs_one_size, NULL, &res);
 	} else if (cmd == XFS_IOC_FSBULKSTAT_32) {
 		error = xfs_bulkstat(mp, &inlast, &count,
-			xfs_bulkstat_one_compat, sizeof(compat_xfs_bstat_t),
+			bs_one_func, bs_one_size,
 			bulkreq.ubuffer, &done);
 	} else
 		error = -EINVAL;
-- 
2.16.1

[RFC PATCH 2/2] xfs: Fix x32 ioctls when cmd numbers differ from ia32.

[Nick Bowler]

Several ioctl structs change size between native 32-bit (ia32) and x32
applications, because x32 follows the native 64-bit (amd64) integer
alignment rules and uses 64-bit time_t.  In these instances, the ioctl
number changes so userspace simply gets -ENOTTY.  This scenario can be
handled by simply adding more cases.

Looking at the different ioctls implemented here:

- All the ones marked 'No size or alignment issue on any arch' should
  presumably all be fine.

- All the ones under BROKEN_X86_ALIGNMENT are different under integer
  alignment rules.  Since x32 matches amd64 here, we just need both
  sets of cases handled.

- XFS_IOC_SWAPEXT has both integer alignment differences and time_t
  differences.  Since x32 matches amd64 here, we need to add a case
  which calls the native implementation.

- The remaining ioctls have neither 64-bit integers nor time_t, so
  x32 matches ia32 here and no change is required at this level.  The
  bulkstat ioctl implementations have some pointer chasing which is
  handled separately.

Signed-off-by: Nick Bowler <nbowler@draconx.ca>
---
 fs/xfs/xfs_ioctl32.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/fs/xfs/xfs_ioctl32.c b/fs/xfs/xfs_ioctl32.c
index 6a759c0ffb54..c5b48e1f46fe 100644
--- a/fs/xfs/xfs_ioctl32.c
+++ b/fs/xfs/xfs_ioctl32.c
@@ -564,8 +564,12 @@ xfs_file_compat_ioctl(
 	case FS_IOC_GETFSMAP:
 	case XFS_IOC_SCRUB_METADATA:
 		return xfs_file_ioctl(filp, cmd, p);
-#ifndef BROKEN_X86_ALIGNMENT
-	/* These are handled fine if no alignment issues */
+#if !defined(BROKEN_X86_ALIGNMENT) || defined(CONFIG_X86_X32)
+	/*
+	 * These are handled fine if no alignment issues.  To support x32
+	 * which uses native 64-bit alignment we must emit these cases in
+	 * addition to the ia-32 compat set below.
+	 */
 	case XFS_IOC_ALLOCSP:
 	case XFS_IOC_FREESP:
 	case XFS_IOC_RESVSP:
@@ -578,8 +582,16 @@ xfs_file_compat_ioctl(
 	case XFS_IOC_FSGROWFSDATA:
 	case XFS_IOC_FSGROWFSRT:
 	case XFS_IOC_ZERO_RANGE:
+#ifdef CONFIG_X86_X32
+	/*
+	 * x32 special: this gets a different cmd number from the ia-32 compat
+	 * case below; the associated data will match native 64-bit alignment.
+	 */
+	case XFS_IOC_SWAPEXT:
+#endif
 		return xfs_file_ioctl(filp, cmd, p);
-#else
+#endif
+#if defined(BROKEN_X86_ALIGNMENT)
 	case XFS_IOC_ALLOCSP_32:
 	case XFS_IOC_FREESP_32:
 	case XFS_IOC_ALLOCSP64_32:
-- 
2.16.1

Re: [RFC PATCH 0/2] Fixing xfs ioctls on x32

[Nick Bowler]

On 12/12/18, Nick Bowler <nbowler@draconx.ca> wrote:
[...]
> All the remaining failures on x32 appear to be aio related, or are
> also failing on the i686 tests.  I have no experience with aio but
> I'm going to assume those problems have nothing to do with ioctl.
[...]
> After (x32):
>
>   Failures: generic/112 generic/113 generic/114 generic/198 generic/207
>             generic/210 generic/240 generic/252 generic/427 generic/451
>             generic/465 generic/484 xfs/191-input-validation xfs/269
>             xfs/495
>   Failed 15 of 656 tests

In support of my assumption that the "generic" failures above are
unrelated to xfs ioctl implementation, I've confirmed that all the
"generic" tests listed as failing above also fail on ext4...

Cheers,
  Nick

Re: [RFC PATCH 1/2] xfs: Fix bulkstat compat ioctls on x32 userspace.

[Brian Foster]

On Wed, Dec 12, 2018 at 08:29:59PM -0500, Nick Bowler wrote:
> The bulkstat family of ioctls are problematic on x32, because there is
> a mixup of native 32-bit and 64-bit conventions: the xfs_bulkstat struct
> contains pointers and 32-bit integers so that fits the native 32-bit
> layout fine.  However, one of those pointers is subsequently used to
> refer to one of several structs which, on x32, all follow the native
> 64-bit way.
> 
> Fortunately the pointer chasing seems to end there, and the functions to
> deal with this abstract things pretty well.  We just need a little tweak
> to pass the right formatting function if called from x32 mode.
> 

Could you be a bit more specific on the problem? What
pointers/structures are problematic? What exactly is "the xfs_bulkstat
struct?"

> Signed-off-by: Nick Bowler <nbowler@draconx.ca>
> ---
>  fs/xfs/xfs_ioctl32.c | 25 +++++++++++++++++++++----
>  1 file changed, 21 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/xfs/xfs_ioctl32.c b/fs/xfs/xfs_ioctl32.c
> index fba115f4103a..6a759c0ffb54 100644
> --- a/fs/xfs/xfs_ioctl32.c
> +++ b/fs/xfs/xfs_ioctl32.c
> @@ -241,6 +241,23 @@ xfs_compat_ioc_bulkstat(
>  	int			done;
>  	int			error;
>  
> +	/*
> +	 * These functions and size are used later to handle individual
> +	 * entries; x32 is annoying and needs different functions.
> +	 */

Same here, this describes the change but doesn't help me understand the
problem.

> +	inumbers_fmt_pf inumbers_func = xfs_inumbers_fmt_compat;
> +	bulkstat_one_pf	bs_one_func = xfs_bulkstat_one_compat;
> +	size_t bs_one_size = sizeof(compat_xfs_bstat_t);
> +
> +#ifdef CONFIG_X86_X32
> +	if (in_x32_syscall()) {
> +		/* x32 matches native amd64 bstat and inogrp layout */
> +		inumbers_func = xfs_inumbers_fmt;
> +		bs_one_func = xfs_bulkstat_one;
> +		bs_one_size = sizeof(xfs_bstat_t);
> +	}
> +#endif
> +

Would this be necessary if the higher level x32 code called into
xfs_ioc_bulkstat() instead of the compat variant, or is there some other
reason x32 wouldn't work through that path?

Brian

>  	/* done = 1 if there are more stats to get and if bulkstat */
>  	/* should be called again (unused here, but used in dmapi) */
>  
> @@ -272,15 +289,15 @@ xfs_compat_ioc_bulkstat(
>  
>  	if (cmd == XFS_IOC_FSINUMBERS_32) {
>  		error = xfs_inumbers(mp, &inlast, &count,
> -				bulkreq.ubuffer, xfs_inumbers_fmt_compat);
> +				bulkreq.ubuffer, inumbers_func);
>  	} else if (cmd == XFS_IOC_FSBULKSTAT_SINGLE_32) {
>  		int res;
>  
> -		error = xfs_bulkstat_one_compat(mp, inlast, bulkreq.ubuffer,
> -				sizeof(compat_xfs_bstat_t), NULL, &res);
> +		error = bs_one_func(mp, inlast, bulkreq.ubuffer,
> +				bs_one_size, NULL, &res);
>  	} else if (cmd == XFS_IOC_FSBULKSTAT_32) {
>  		error = xfs_bulkstat(mp, &inlast, &count,
> -			xfs_bulkstat_one_compat, sizeof(compat_xfs_bstat_t),
> +			bs_one_func, bs_one_size,
>  			bulkreq.ubuffer, &done);
>  	} else
>  		error = -EINVAL;
> -- 
> 2.16.1
> 

Re: [RFC PATCH 1/2] xfs: Fix bulkstat compat ioctls on x32 userspace.

[Nick Bowler]

On 2018-12-13, Brian Foster <bfoster@redhat.com> wrote:
> On Wed, Dec 12, 2018 at 08:29:59PM -0500, Nick Bowler wrote:
>> The bulkstat family of ioctls are problematic on x32, because there is
>> a mixup of native 32-bit and 64-bit conventions: the xfs_bulkstat struct
>> contains pointers and 32-bit integers so that fits the native 32-bit
>> layout fine.  However, one of those pointers is subsequently used to
>> refer to one of several structs which, on x32, all follow the native
>> 64-bit way.
>>
>> Fortunately the pointer chasing seems to end there, and the functions to
>> deal with this abstract things pretty well.  We just need a little tweak
>> to pass the right formatting function if called from x32 mode.
>>
> > Could you be a bit more specific on the problem? What
> pointers/structures are problematic? What exactly is "the xfs_bulkstat
> struct?"

A mistake.  I meant struct xfs_fsop_bulkreq; I'll fix the commit message.

The problem is:

  - xfs_fsop_bulkreq on x32 matches IA-32 layout on x32, so the
    ioctl cmd number matches and the implementation calls
    xfs_compat_ioc_bulkstat.

  - The 'ubuffer' pointer in that structure refers to either struct
    xfs_bstat or struct xfs_inogrp.  On x32 both of these structures
    match the native 64-bit layout; the compat path writes out the
    IA-32 layout which is incorrectly formatted for x32 userspace.

The proposed solution is:

  - Use in_x32_syscall to distinguish the IA-32 and x32 cases, the
    functions which do this have a easy way to select which output
    format is required, so we just need to pick the right one on x32.

>> ---
>>  fs/xfs/xfs_ioctl32.c | 25 +++++++++++++++++++++----
>>  1 file changed, 21 insertions(+), 4 deletions(-)
>>
>> diff --git a/fs/xfs/xfs_ioctl32.c b/fs/xfs/xfs_ioctl32.c
>> index fba115f4103a..6a759c0ffb54 100644
>> --- a/fs/xfs/xfs_ioctl32.c
>> +++ b/fs/xfs/xfs_ioctl32.c
>> @@ -241,6 +241,23 @@ xfs_compat_ioc_bulkstat(
>>  	int			done;
>>  	int			error;
>>
>> +	/*
>> +	 * These functions and size are used later to handle individual
>> +	 * entries; x32 is annoying and needs different functions.
>> +	 */
>
> Same here, this describes the change but doesn't help me understand the
> problem.

I'll think about a better way to write this comment.

>> +	inumbers_fmt_pf inumbers_func = xfs_inumbers_fmt_compat;
>> +	bulkstat_one_pf	bs_one_func = xfs_bulkstat_one_compat;
>> +	size_t bs_one_size = sizeof(compat_xfs_bstat_t);
>> +
>> +#ifdef CONFIG_X86_X32
>> +	if (in_x32_syscall()) {
>> +		/* x32 matches native amd64 bstat and inogrp layout */
>> +		inumbers_func = xfs_inumbers_fmt;
>> +		bs_one_func = xfs_bulkstat_one;
>> +		bs_one_size = sizeof(xfs_bstat_t);
>> +	}
>> +#endif
>> +
>
> Would this be necessary if the higher level x32 code called into
> xfs_ioc_bulkstat() instead of the compat variant, or is there some
> other reason x32 wouldn't work through that path?

The xfs_compat_ioc_bulkstat function does two things: it reads in the
xfs_fsop_bulkreq structure (matches ia-32 layout on x32), then it writes
out the xfs_inorgp or xfs_bstat structures depending on what operation
was requested; both of these structures match amd64 layout on x32.

So the goal of the change is to adjust the second behaviour only.

Cheers,
  Nick

Re: [RFC PATCH 1/2] xfs: Fix bulkstat compat ioctls on x32 userspace.

[Brian Foster]

On Thu, Dec 13, 2018 at 12:44:16PM -0500, Nick Bowler wrote:
> On 2018-12-13, Brian Foster <bfoster@redhat.com> wrote:
> > On Wed, Dec 12, 2018 at 08:29:59PM -0500, Nick Bowler wrote:
> >> The bulkstat family of ioctls are problematic on x32, because there is
> >> a mixup of native 32-bit and 64-bit conventions: the xfs_bulkstat struct
> >> contains pointers and 32-bit integers so that fits the native 32-bit
> >> layout fine.  However, one of those pointers is subsequently used to
> >> refer to one of several structs which, on x32, all follow the native
> >> 64-bit way.
> >>
> >> Fortunately the pointer chasing seems to end there, and the functions to
> >> deal with this abstract things pretty well.  We just need a little tweak
> >> to pass the right formatting function if called from x32 mode.
> >>
> > > Could you be a bit more specific on the problem? What
> > pointers/structures are problematic? What exactly is "the xfs_bulkstat
> > struct?"
> 
> A mistake.  I meant struct xfs_fsop_bulkreq; I'll fix the commit message.
> 
> The problem is:
> 
>   - xfs_fsop_bulkreq on x32 matches IA-32 layout on x32, so the
>     ioctl cmd number matches and the implementation calls
>     xfs_compat_ioc_bulkstat.
> 

I assume that this is because xfs_fsop_bulkreq includes pointers, which
is where x32 and x86_64 actually start to differ..? So in this
particular case, the two ioctl() structs actually are different between
x32 and x86_64.

>   - The 'ubuffer' pointer in that structure refers to either struct
>     xfs_bstat or struct xfs_inogrp.  On x32 both of these structures
>     match the native 64-bit layout; the compat path writes out the
>     IA-32 layout which is incorrectly formatted for x32 userspace.
> 

Ok.

> The proposed solution is:
> 
>   - Use in_x32_syscall to distinguish the IA-32 and x32 cases, the
>     functions which do this have a easy way to select which output
>     format is required, so we just need to pick the right one on x32.
> 

A little hairy, but it makes more sense now. Thanks.

> >> ---
> >>  fs/xfs/xfs_ioctl32.c | 25 +++++++++++++++++++++----
> >>  1 file changed, 21 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/fs/xfs/xfs_ioctl32.c b/fs/xfs/xfs_ioctl32.c
> >> index fba115f4103a..6a759c0ffb54 100644
> >> --- a/fs/xfs/xfs_ioctl32.c
> >> +++ b/fs/xfs/xfs_ioctl32.c
> >> @@ -241,6 +241,23 @@ xfs_compat_ioc_bulkstat(
> >>  	int			done;
> >>  	int			error;
> >>
> >> +	/*
> >> +	 * These functions and size are used later to handle individual
> >> +	 * entries; x32 is annoying and needs different functions.
> >> +	 */
> >
> > Same here, this describes the change but doesn't help me understand the
> > problem.
> 
> I'll think about a better way to write this comment.
> 

I'd suggest to use parts of what you've just described above.

Brian

> >> +	inumbers_fmt_pf inumbers_func = xfs_inumbers_fmt_compat;
> >> +	bulkstat_one_pf	bs_one_func = xfs_bulkstat_one_compat;
> >> +	size_t bs_one_size = sizeof(compat_xfs_bstat_t);
> >> +
> >> +#ifdef CONFIG_X86_X32
> >> +	if (in_x32_syscall()) {
> >> +		/* x32 matches native amd64 bstat and inogrp layout */
> >> +		inumbers_func = xfs_inumbers_fmt;
> >> +		bs_one_func = xfs_bulkstat_one;
> >> +		bs_one_size = sizeof(xfs_bstat_t);
> >> +	}
> >> +#endif
> >> +
> >
> > Would this be necessary if the higher level x32 code called into
> > xfs_ioc_bulkstat() instead of the compat variant, or is there some
> > other reason x32 wouldn't work through that path?
> 
> The xfs_compat_ioc_bulkstat function does two things: it reads in the
> xfs_fsop_bulkreq structure (matches ia-32 layout on x32), then it writes
> out the xfs_inorgp or xfs_bstat structures depending on what operation
> was requested; both of these structures match amd64 layout on x32.
> 
> So the goal of the change is to adjust the second behaviour only.
> 
> Cheers,
>   Nick

Re: [RFC PATCH 0/2] Fixing xfs ioctls on x32

[Nick Bowler]

OK, I went through every single case in xfs_file_compat_ioctl, and wrote
a brief justification for what, if any, changes appear to be be required
to correctly support x32 userspace.  The analysis is done by inspecting
how the implementation accesses userspace memory and what structures
are involved.

There is a general assumption in this analysis that the current compat
implementation is correct as written for IA-32 userspace.  However,
during this inspection I may have found a problem in one of the ioctls
which affects i686 (NOT x32!) today.  I am looking into this next.

Anyway, here's the complete list.

XFS_IOC_DIOINFO, XFS_IOC_FSGEOMETRY, XFS_IOC_FSGETXATTR,
XFS_IOC_FSSETXATTR, XFS_IOC_FSGETXATTRA, XFS_IOC_FSSETDM,
XFS_IOC_GETBMAP, XFS_IOC_GETBMAPA, XFS_IOC_GETBMAPX, XFS_IOC_FSCOUNTS,
XFS_IOC_SET_RESBLKS, XFS_IOC_GET_RESBLKS, XFS_IOC_FSGROWFSLOG,
XFS_IOC_GOINGDOWN, XFS_IOC_ERROR_INJECTION, XFS_IOC_ERROR_CLEARALL,
FS_IOC_GETFSMAP, XFS_IOC_SCRUB_METADATA:

  - All of these call directly the native implementation.  Since that
    implementation is correct for both ia32 and amd64 it will also be
    correct for x32.

XFS_IOC_ALLOCSP_32, XFS_IOC_FREESP_32, XFS_IOC_ALLOCSP64_32,
XFS_IOC_FREESP64_32, XFS_IOC_RESVSP_32, XFS_IOC_UNRESVSP_32,
XFS_IOC_RESVSP64_32, XFS_IOC_UNRESVSP64_32, XFS_IOC_ZERO_RANGE_32:

  - The relevant struct is xfs_flock64.  On x32 this structure matches
    exactly the amd64 layout, so the ioctl cmd number is changed.

  - No other data in userspace is involved.

  - Since the x32 structure matches the native layout exactly, it is
    required to add new cases which pass the arguments onward to the
    native implementation.  This is patch #2 in this series ("Fix x32
    ioctls when cmd numbers differ from ia32.")

XFS_IOC_FSGEOMETRY_V1_32:

  - The relevant struct is xfs_fsop_geom_v1.  On x32 this structure
    matches exactly the amd64 layout, so the ioctl cmd number is
    changed.

  - No other data in userspace is involved.

  - Since the x32 structure matches the native layout exactly, it is
    required to add a new case (XFS_IOC_FSGEOMETRY_V1) which passes the
    arguments onward to the native implementation.  This is patch #2
    in this series ("Fix x32 ioctls when cmd numbers differ from ia32.")

XFS_IOC_FSGROWFSDATA_32:

  - The relevant struct is xfs_growfs_data.  On x32 this structure
    matches exactly the amd64 layout, so the ioctl cmd number is
    changed.

  - No other data in userspace is involved.

  - Since the x32 structure matches the native layout exactly, it is
    required to add a new case (XFS_IOC_FSGROWFSDATA) which passes the
    arguments onward to the native implementation.  This is patch #2
    in this series.  ("Fix x32 ioctls when cmd numbers differ from
    ia32.")


XFS_IOC_FSGROWFSRT_32:

  - The relevant struct is xfs_growfs_rt.  On x32 this structure matches
    exactly the amd64 layout, so the ioctl cmd number is changed.

  - No other data in userspace is involved.

  - Since the x32 structure matches the native layout exactly, it is
    required to add a new case which passes the arguments onward to
    the native implementation.  This is patch #2 in this series ("Fix
    x32 ioctls when cmd numbers differ from ia32.")

XFS_IOC_GETXFLAGS_32, XFS_IOC_SETXFLAGS_32, XFS_IOC_GETVERSION_32:

  - Trivial, the only compat problem is the ioctl cmd number itself,
    which on x32 will match the ia32 number so it will be handled
    correctly.

  - No change is required.

XFS_IOC_SWAPEXT_32:

  - The relevant struct is xfs_swapext.  On x32 this structure matches
    exactly with the amd64 layout, so the ioctl cmd number is changed.

  - No other data in userspace is involved.

  - Since the x32 structure matches the native layout exactly, it is
    required to add a new case (XFS_IOC_SWAPEXT) which passes the
    arguments onward to the native implementation.  This is patch #2
    in this series ("Fix x32 ioctls when cmd numbers differ from ia32.")

XFS_IOC_FSBULKSTAT_32, XFS_IOC_FSBULKSTAT_SINGLE_32,
XFS_IOC_FSINUMBERS_32:

  - These three are all busted on x32 and patch #1 in this series
    ("Fix bulkstat compat ioctls on x32") should fix them.  See the
    patch thread for details.

XFS_IOC_FD_TO_HANDLE_32, XFS_IOC_PATH_TO_HANDLE_32,
XFS_IOC_PATH_TO_FSHANDLE_32, XFS_IOC_OPEN_BY_HANDLE_32,
XFS_IOC_READLINK_BY_HANDLE_32:

  - For all of these, The relevant struct is xfs_fsop_handlereq.  On
    x32, this structure matches exactly the ia32 layout, so ioctl cmd
    number will match.

  - The current compat implementation reads in the structure according
    to ia32 layout.  Thus it will be read correctly from x32 userspace.

  - The existing implementation then calls the native implementation.
    Since that is correct for both ia32 and amd64 it will also be
    correct for x32.

  - No change is required.

XFS_IOC_ATTRLIST_BY_HANDLE_32:

  - The relevant struct is xfs_fsop_attrlist_handlereq.  On x32, this
    structure matches exactly the ia32 layout, so ioctl cmd number will
    match.  The implementation calls xfs_compat_attrlist_by_handle.

  - Since the xfs_fsop_attrlist_handlereq struct matches ia32, the
    existing compat implementation will read it from x32 userspace
    correctly.

  - Something is strange here, the native implementation has an extra
    copy_to_user which is not present in the compat implementation,
    which writes the attrlist_cursor_kern struct back into the
    xfs_fsop_attrlist_handlereq structure provided by the user.

  - Other than that one difference, the existing compat implementation
    does exactly the same thing as the native implementation.  Since
    those parts are correct for both ia32 and amd64 they will also be
    correct for x32.

  - If any change is required, it may be that this ioctl is currently
    broken on regular ia32 compat.

  - This may be why xfstests xfs/269 is failing for me on i686, since
    that test makes use of this very ioctl.  I will investigate.

XFS_IOC_ATTRMULTI_BY_HANDLE_32:

  - The relevant struct is xfs_fsop_attrmulti_handlereq.  On x32, this
    structure matches exactly the ia32 layout, so ioctl cmd number will
    match.  The implementation calls xfs_compat_attrmulti_by_handle.

  - The main structure contains a pointer to another xfs struct,
    xfs_attr_multiop (this is read as an array of these structs).
    On x32, this structure also matches exactly the ia32 layout
    (so the array does too).

  - Since both these structures match ia32, the existing compat
    implementation will read them in from x32 userspace correctly.

  - The xfs_attr_multiop structure contains two pointers to strings
    (am_attrname, am_attrvalue).  There are no compat issues with
    strings so these will be read and/or written correctly by the
    existing compat implementation.

  - Finally, the implementation writes back to the xfs_attr_multiop
    array.  Since the x32 version matches ia32, this will be correctly
    formatted for x32 userspace.

  - No change is required.

XFS_IOC_FSSETDM_BY_HANDLE_32:

  - The relevant struct is xfs_fsop_setdm_handlereq.  On x32, this
    structure matches exactly the ia32 layout, so ioctl cmd number will
    match.  The implementation calls xfs_compat_fssetdm_by_handle.

  - Since the xfs_fsop_setdm_handlereq struct matches ia32, the existing
    compat implementation will read it from x32 userspace correctly.

  - The existing implementation then reads the pointed-to struct
    fsdmidata.  This structure matches between ia32, x32, and amd64
    layouts; the existing compat implementation just snarfs it in
    directly and this will work as expected on x32.

  - After this point the existing compat implementation is identical to
    the native implementation.  Since that part is correct for both ia32
    and amd64 it will also be correct for x32.

  - No change is required.

And that's a wrap.

Cheers,
  Nick