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

["Darrick J. Wong" <darrick.wong@oracle.com>]

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