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@xxxxxxxxxx>
> ---
> 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
>
