| Message ID | 20181213013000.15716-1-nbowler@draconx.ca (mailing list archive) |
|---|---|
| Headers | show |
| Series | Fixing xfs ioctls on x32 | expand |
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
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
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(-)