[PATCH v2] xfs: fix wrong compat fields in xfs_ioctl32_bstat_copyin()

[PATCH v2] xfs: fix wrong compat fields in xfs_ioctl32_bstat_copyin()

[Reid Downing]

bs_blocks and bs_xflags were both read from bstat32->bs_size.
Read each one from its own field instead.

Fixes: e94fc4a43e5c39 ("[XFS] Add compat handlers for swapext ioctl")
Cc: <stable@vger.kernel.org> # v2.6.29
Signed-off-by: Reid Downing <skibletz@gmail.com>
---
v2:
 - Also fix bs_blocks (spotted by Eric Sandeen)

 fs/xfs/xfs_ioctl32.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/xfs/xfs_ioctl32.c b/fs/xfs/xfs_ioctl32.c
index f6875a2cc70..77bc936bf0d 100644
--- a/fs/xfs/xfs_ioctl32.c
+++ b/fs/xfs/xfs_ioctl32.c
@@ -143,8 +143,8 @@ xfs_ioctl32_bstat_copyin(
 	    xfs_ioctl32_bstime_copyin(&bstat->bs_atime, &bstat32->bs_atime) ||
 	    xfs_ioctl32_bstime_copyin(&bstat->bs_mtime, &bstat32->bs_mtime) ||
 	    xfs_ioctl32_bstime_copyin(&bstat->bs_ctime, &bstat32->bs_ctime) ||
-	    get_user(bstat->bs_blocks,	&bstat32->bs_size)	||
-	    get_user(bstat->bs_xflags,	&bstat32->bs_size)	||
+	    get_user(bstat->bs_blocks,	&bstat32->bs_blocks)	||
+	    get_user(bstat->bs_xflags,	&bstat32->bs_xflags)	||
 	    get_user(bstat->bs_extsize,	&bstat32->bs_extsize)	||
 	    get_user(bstat->bs_extents,	&bstat32->bs_extents)	||
 	    get_user(bstat->bs_gen,	&bstat32->bs_gen)	||
-- 
2.53.0

Re: [PATCH v2] xfs: fix wrong compat fields in xfs_ioctl32_bstat_copyin()

[David Laight]

On Tue, 27 Oct 2026 12:17:48 -0400
Reid Downing <skibletz@gmail.com> wrote:

> bs_blocks and bs_xflags were both read from bstat32->bs_size.
> Read each one from its own field instead.

It would be much better to do a single copy_from_user() of the entire
compat structure and copy the fields to the standard one.

The function would need to be 'noinline_for_stack'.

David
 
> 
> Fixes: e94fc4a43e5c39 ("[XFS] Add compat handlers for swapext ioctl")
> Cc: <stable@vger.kernel.org> # v2.6.29
> Signed-off-by: Reid Downing <skibletz@gmail.com>
> ---
> v2:
>  - Also fix bs_blocks (spotted by Eric Sandeen)
> 
>  fs/xfs/xfs_ioctl32.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/xfs/xfs_ioctl32.c b/fs/xfs/xfs_ioctl32.c
> index f6875a2cc70..77bc936bf0d 100644
> --- a/fs/xfs/xfs_ioctl32.c
> +++ b/fs/xfs/xfs_ioctl32.c
> @@ -143,8 +143,8 @@ xfs_ioctl32_bstat_copyin(
>  	    xfs_ioctl32_bstime_copyin(&bstat->bs_atime, &bstat32->bs_atime) ||
>  	    xfs_ioctl32_bstime_copyin(&bstat->bs_mtime, &bstat32->bs_mtime) ||
>  	    xfs_ioctl32_bstime_copyin(&bstat->bs_ctime, &bstat32->bs_ctime) ||
> -	    get_user(bstat->bs_blocks,	&bstat32->bs_size)	||
> -	    get_user(bstat->bs_xflags,	&bstat32->bs_size)	||
> +	    get_user(bstat->bs_blocks,	&bstat32->bs_blocks)	||
> +	    get_user(bstat->bs_xflags,	&bstat32->bs_xflags)	||
>  	    get_user(bstat->bs_extsize,	&bstat32->bs_extsize)	||
>  	    get_user(bstat->bs_extents,	&bstat32->bs_extents)	||
>  	    get_user(bstat->bs_gen,	&bstat32->bs_gen)	||

Re: [PATCH v2] xfs: fix wrong compat fields in xfs_ioctl32_bstat_copyin()

[Reid Downing]

Hi David, Eric, Carlos,

Thank you all for the careful review, and Eric, thanks for catching
the second line.

I looked at the refactor David suggested and Eric sketched, and I
agree it is the better way to write this function. I'm not confident
enough in the 32-bit compat code to send it myself, and I only want to
send changes I can fully vouch for. If the refactor should go in, I'd
be glad to see it done.

Reid

Re: [PATCH v2] xfs: fix wrong compat fields in xfs_ioctl32_bstat_copyin()

[Christoph Hellwig]

On Tue, Oct 27, 2026 at 04:49:11PM +0000, David Laight wrote:
> On Tue, 27 Oct 2026 12:17:48 -0400
> Reid Downing <skibletz@gmail.com> wrote:
> 
> > bs_blocks and bs_xflags were both read from bstat32->bs_size.
> > Read each one from its own field instead.
> 
> It would be much better to do a single copy_from_user() of the entire
> compat structure and copy the fields to the standard one.

Care to explain why it would be "much better" to do two copies instead of
one?  The only benefits from copy_from/to_user over the current
get_put/user is avoidind the conctant user access disabling/enabling,
but the scoped uaccess thing would do the same.

Re: [PATCH v2] xfs: fix wrong compat fields in xfs_ioctl32_bstat_copyin()

[David Laight]

On Wed, 28 Oct 2026 08:18:40 -0700
Christoph Hellwig <hch@infradead.org> wrote:

> On Tue, Oct 27, 2026 at 04:49:11PM +0000, David Laight wrote:
> > On Tue, 27 Oct 2026 12:17:48 -0400
> > Reid Downing <skibletz@gmail.com> wrote:
> >   
> > > bs_blocks and bs_xflags were both read from bstat32->bs_size.
> > > Read each one from its own field instead.  
> > 
> > It would be much better to do a single copy_from_user() of the entire
> > compat structure and copy the fields to the standard one.  
> 
> Care to explain why it would be "much better" to do two copies instead of
> one?  The only benefits from copy_from/to_user over the current
> get_put/user is avoidind the conctant user access disabling/enabling,

That is a massive hit.
The current code also has all the conditionals from access_ok().

> but the scoped uaccess thing would do the same.

True, but unnecessarily 'technical' for this bit of code.
You also save all the exception table entries - so the code size
would be rather smaller.

David