[RFC PATCH v2 0/3] Fixing xfs ioctls on x32

[RFC PATCH v2 0/3] Fixing xfs ioctls on x32

[Nick Bowler]

v2: Add a new patch which fixes xfstests/269 on both x32 and IA-32
    userspace.  Updated test results.

    Update commit message and comments in bulkstat patch based on
    feedback.

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 and
there's probably something to be fixed here but it seems unrelated
to XFS ioctls and I think that's a story for another day.

Before (i686 w/ 64bit kernel):

  Failures: generic/484 xfs/191-input-validation xfs/269 xfs/495
  Failed 4 of 658 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 658 tests

After (i686 w/ 64bit kernel):

  Failures: generic/484 xfs/191-input-validation xfs/495
  Failed 3 of 658 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/495
  Failed 14 of 658 tests

Nick Bowler (3):
  xfs: Align compat attrlist_by_handle with native implementation.
  xfs: Fix bulkstat compat ioctls on x32 userspace.
  xfs: Fix x32 ioctls when cmd numbers differ from ia32.

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

-- 
2.16.1

[RFC PATCH v2 1/3] xfs: Align compat attrlist_by_handle with native implementation.

[Nick Bowler]

While inspecting the ioctl implementations, I noticed that the compat
implementation of XFS_IOC_ATTRLIST_BY_HANDLE does not do exactly the
same thing as the native implementation.  Specifically, the "cursor"
does not appear to be written out to userspace on the compat path,
like it is on the native path.

This adjusts the compat implementation to copy out the cursor just
like the native implementation does.  The attrlist cursor does not
require any special compat handling.  This fixes xfstests xfs/269
on both IA-32 and x32 userspace, when running on an amd64 kernel.

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

diff --git a/fs/xfs/xfs_ioctl32.c b/fs/xfs/xfs_ioctl32.c
index fba115f4103a..4c34efcbf7e8 100644
--- a/fs/xfs/xfs_ioctl32.c
+++ b/fs/xfs/xfs_ioctl32.c
@@ -336,6 +336,7 @@ xfs_compat_attrlist_by_handle(
 {
 	int			error;
 	attrlist_cursor_kern_t	*cursor;
+	compat_xfs_fsop_attrlist_handlereq_t __user *p = arg;
 	compat_xfs_fsop_attrlist_handlereq_t al_hreq;
 	struct dentry		*dentry;
 	char			*kbuf;
@@ -370,6 +371,11 @@ xfs_compat_attrlist_by_handle(
 	if (error)
 		goto out_kfree;
 
+	if (copy_to_user(&p->pos, cursor, sizeof(attrlist_cursor_kern_t))) {
+		error = -EFAULT;
+		goto out_kfree;
+	}
+
 	if (copy_to_user(compat_ptr(al_hreq.buffer), kbuf, al_hreq.buflen))
 		error = -EFAULT;
 
-- 
2.16.1

Re: [RFC PATCH v2 1/3] xfs: Align compat attrlist_by_handle with native implementation.

[Darrick J. Wong]

On Fri, Dec 14, 2018 at 08:22:57PM -0500, Nick Bowler wrote:
> While inspecting the ioctl implementations, I noticed that the compat
> implementation of XFS_IOC_ATTRLIST_BY_HANDLE does not do exactly the
> same thing as the native implementation.  Specifically, the "cursor"
> does not appear to be written out to userspace on the compat path,
> like it is on the native path.
> 
> This adjusts the compat implementation to copy out the cursor just
> like the native implementation does.  The attrlist cursor does not
> require any special compat handling.  This fixes xfstests xfs/269
> on both IA-32 and x32 userspace, when running on an amd64 kernel.

Craaap, I forgot that when I fixed the native attrlist_by_handle. :(

> Signed-off-by: Nick Bowler <nbowler@draconx.ca>

Fixes: 0facef7fb053b ("xfs: in _attrlist_by_handle, copy the cursor back to userspace")
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> ---
>  fs/xfs/xfs_ioctl32.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/fs/xfs/xfs_ioctl32.c b/fs/xfs/xfs_ioctl32.c
> index fba115f4103a..4c34efcbf7e8 100644
> --- a/fs/xfs/xfs_ioctl32.c
> +++ b/fs/xfs/xfs_ioctl32.c
> @@ -336,6 +336,7 @@ xfs_compat_attrlist_by_handle(
>  {
>  	int			error;
>  	attrlist_cursor_kern_t	*cursor;
> +	compat_xfs_fsop_attrlist_handlereq_t __user *p = arg;
>  	compat_xfs_fsop_attrlist_handlereq_t al_hreq;
>  	struct dentry		*dentry;
>  	char			*kbuf;
> @@ -370,6 +371,11 @@ xfs_compat_attrlist_by_handle(
>  	if (error)
>  		goto out_kfree;
>  
> +	if (copy_to_user(&p->pos, cursor, sizeof(attrlist_cursor_kern_t))) {
> +		error = -EFAULT;
> +		goto out_kfree;
> +	}
> +
>  	if (copy_to_user(compat_ptr(al_hreq.buffer), kbuf, al_hreq.buflen))
>  		error = -EFAULT;
>  
> -- 
> 2.16.1
> 

[RFC PATCH v2 2/3] 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_fsop_bulkreq
struct contains pointers and 32-bit integers so that matches the native
32-bit layout, and that means the ioctl implementation goes into the
regular compat path on x32.

However, the 'ubuffer' member of that struct in turn refers to either
struct xfs_inogrp or xfs_bstat (or an array of these).  On x32, those
structures match the native 64-bit layout.  The compat implementation
writes out the 32-bit version of these structures.  This is not the
expected format for x32 userspace, causing problems.

Fortunately the functions which actually output these xfs_inogrp and
xfs_bstat structures have an easy way to select which output format
is required, so we just need a little tweak to select the right format
on x32.

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

diff --git a/fs/xfs/xfs_ioctl32.c b/fs/xfs/xfs_ioctl32.c
index 4c34efcbf7e8..1cdc75dca779 100644
--- a/fs/xfs/xfs_ioctl32.c
+++ b/fs/xfs/xfs_ioctl32.c
@@ -241,6 +241,32 @@ xfs_compat_ioc_bulkstat(
 	int			done;
 	int			error;
 
+	/*
+	 * Output structure handling functions.  Depending on the command,
+	 * either the xfs_bstat and xfs_inogrp structures are written out
+	 * to userpace memory via bulkreq.ubuffer.  Normally the compat
+	 * functions and structure size are the correct ones to use ...
+	 */
+	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()) {
+		/*
+		 * ... but on x32 the input xfs_fsop_bulkreq has pointers
+		 * which must be handled in the "compat" (32-bit) way, while
+		 * the xfs_bstat and xfs_inogrp structures follow native 64-
+		 * bit layout convention.  So adjust accordingly, otherwise
+		 * the data written out in compat layout will not match what
+		 * x32 userspace expects.
+		 */
+		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 +298,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 v2 2/3] xfs: Fix bulkstat compat ioctls on x32 userspace.

[Darrick J. Wong]

On Fri, Dec 14, 2018 at 08:22:58PM -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_fsop_bulkreq
> struct contains pointers and 32-bit integers so that matches the native
> 32-bit layout, and that means the ioctl implementation goes into the
> regular compat path on x32.
> 
> However, the 'ubuffer' member of that struct in turn refers to either
> struct xfs_inogrp or xfs_bstat (or an array of these).  On x32, those
> structures match the native 64-bit layout.  The compat implementation
> writes out the 32-bit version of these structures.  This is not the
> expected format for x32 userspace, causing problems.
> 
> Fortunately the functions which actually output these xfs_inogrp and
> xfs_bstat structures have an easy way to select which output format
> is required, so we just need a little tweak to select the right format
> on x32.
> 
> Signed-off-by: Nick Bowler <nbowler@draconx.ca>
> ---
>  fs/xfs/xfs_ioctl32.c | 34 ++++++++++++++++++++++++++++++----
>  1 file changed, 30 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/xfs/xfs_ioctl32.c b/fs/xfs/xfs_ioctl32.c
> index 4c34efcbf7e8..1cdc75dca779 100644
> --- a/fs/xfs/xfs_ioctl32.c
> +++ b/fs/xfs/xfs_ioctl32.c
> @@ -241,6 +241,32 @@ xfs_compat_ioc_bulkstat(
>  	int			done;
>  	int			error;
>  
> +	/*
> +	 * Output structure handling functions.  Depending on the command,
> +	 * either the xfs_bstat and xfs_inogrp structures are written out
> +	 * to userpace memory via bulkreq.ubuffer.  Normally the compat
> +	 * functions and structure size are the correct ones to use ...
> +	 */
> +	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()) {
> +		/*
> +		 * ... but on x32 the input xfs_fsop_bulkreq has pointers
> +		 * which must be handled in the "compat" (32-bit) way, while
> +		 * the xfs_bstat and xfs_inogrp structures follow native 64-
> +		 * bit layout convention.  So adjust accordingly, otherwise
> +		 * the data written out in compat layout will not match what
> +		 * x32 userspace expects.
> +		 */
> +		inumbers_func = xfs_inumbers_fmt;
> +		bs_one_func = xfs_bulkstat_one;
> +		bs_one_size = sizeof(xfs_bstat_t);

struct xfs_bstat, not xfs_bstat_t, because we're gradually getting rid
of the structure typedefs.  If I don't hear any other objections in the
next day or so I'll fix this when I pull in this patch.

Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> +	}
> +#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 +298,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 v2 2/3] xfs: Fix bulkstat compat ioctls on x32 userspace.

[Nick Bowler]

On 2018-12-17 Darrick J. Wong <darrick.wong@oracle.com> wrote:
> On Fri, Dec 14, 2018 at 08:22:58PM -0500, Nick Bowler wrote:
[...]
>> diff --git a/fs/xfs/xfs_ioctl32.c b/fs/xfs/xfs_ioctl32.c
>> index 4c34efcbf7e8..1cdc75dca779 100644
>> --- a/fs/xfs/xfs_ioctl32.c
>> +++ b/fs/xfs/xfs_ioctl32.c
>> @@ -241,6 +241,32 @@ xfs_compat_ioc_bulkstat(
>>  	int			done;
>>  	int			error;
>>
>> +	/*
>> +	 * Output structure handling functions.  Depending on the command,
>> +	 * either the xfs_bstat and xfs_inogrp structures are written out
>> +	 * to userpace memory via bulkreq.ubuffer.  Normally the compat
>> +	 * functions and structure size are the correct ones to use ...
>> +	 */
>> +	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()) {
>> +		/*
>> +		 * ... but on x32 the input xfs_fsop_bulkreq has pointers
>> +		 * which must be handled in the "compat" (32-bit) way, while
>> +		 * the xfs_bstat and xfs_inogrp structures follow native 64-
>> +		 * bit layout convention.  So adjust accordingly, otherwise
>> +		 * the data written out in compat layout will not match what
>> +		 * x32 userspace expects.
>> +		 */
>> +		inumbers_func = xfs_inumbers_fmt;
>> +		bs_one_func = xfs_bulkstat_one;
>> +		bs_one_size = sizeof(xfs_bstat_t);
>
> struct xfs_bstat, not xfs_bstat_t, because we're gradually getting rid
> of the structure typedefs.  If I don't hear any other objections in the
> next day or so I'll fix this when I pull in this patch.

Makes sense.  I'll only change this on my end if I have to respin the
series for some other reason then.  I guess the sizeof(compat_xfs_bstat_t)
just before this should be similarly changed as well?

> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

Thanks,
  Nick

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

[Darrick J. Wong]

On Mon, Dec 17, 2018 at 03:06:43PM -0500, Nick Bowler wrote:
> On 2018-12-17 Darrick J. Wong <darrick.wong@oracle.com> wrote:
> > On Fri, Dec 14, 2018 at 08:22:58PM -0500, Nick Bowler wrote:
> [...]
> >> diff --git a/fs/xfs/xfs_ioctl32.c b/fs/xfs/xfs_ioctl32.c
> >> index 4c34efcbf7e8..1cdc75dca779 100644
> >> --- a/fs/xfs/xfs_ioctl32.c
> >> +++ b/fs/xfs/xfs_ioctl32.c
> >> @@ -241,6 +241,32 @@ xfs_compat_ioc_bulkstat(
> >>  	int			done;
> >>  	int			error;
> >>
> >> +	/*
> >> +	 * Output structure handling functions.  Depending on the command,
> >> +	 * either the xfs_bstat and xfs_inogrp structures are written out
> >> +	 * to userpace memory via bulkreq.ubuffer.  Normally the compat
> >> +	 * functions and structure size are the correct ones to use ...
> >> +	 */
> >> +	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()) {
> >> +		/*
> >> +		 * ... but on x32 the input xfs_fsop_bulkreq has pointers
> >> +		 * which must be handled in the "compat" (32-bit) way, while
> >> +		 * the xfs_bstat and xfs_inogrp structures follow native 64-
> >> +		 * bit layout convention.  So adjust accordingly, otherwise
> >> +		 * the data written out in compat layout will not match what
> >> +		 * x32 userspace expects.
> >> +		 */
> >> +		inumbers_func = xfs_inumbers_fmt;
> >> +		bs_one_func = xfs_bulkstat_one;
> >> +		bs_one_size = sizeof(xfs_bstat_t);
> >
> > struct xfs_bstat, not xfs_bstat_t, because we're gradually getting rid
> > of the structure typedefs.  If I don't hear any other objections in the
> > next day or so I'll fix this when I pull in this patch.
> 
> Makes sense.  I'll only change this on my end if I have to respin the
> series for some other reason then.  I guess the sizeof(compat_xfs_bstat_t)
> just before this should be similarly changed as well?

Yep.

--D

> > Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Thanks,
>   Nick

[RFC PATCH v2 3/3] 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 1cdc75dca779..bd9ffad0f65e 100644
--- a/fs/xfs/xfs_ioctl32.c
+++ b/fs/xfs/xfs_ioctl32.c
@@ -579,8 +579,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:
@@ -593,8 +597,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 v2 3/3] xfs: Fix x32 ioctls when cmd numbers differ from ia32.

[Darrick J. Wong]

On Fri, Dec 14, 2018 at 08:22:59PM -0500, Nick Bowler wrote:
> 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>

Looks mostly ok to me, provided they don't drop x32 this release.

(It doesn't sound to me like that's going to happen any time soon...)

Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

(Will try to test all this... /me flexes his cross-arch muscles...)

--D

> ---
>  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 1cdc75dca779..bd9ffad0f65e 100644
> --- a/fs/xfs/xfs_ioctl32.c
> +++ b/fs/xfs/xfs_ioctl32.c
> @@ -579,8 +579,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:
> @@ -593,8 +597,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 v2 0/3] Fixing xfs ioctls on x32

[Nick Bowler]

On 2018-12-14, Nick Bowler <nbowler@draconx.ca> wrote:
> 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.

Update: turns out this bug has already been fixed in glibc 2.27.

Cheers,
  Nick