[PATCH] xfs: fix bs_xflags copied from wrong compat field

[PATCH] xfs: fix bs_xflags copied from wrong compat field

[Reid D]

From: Reid Downing (b1nv0id) <skibletz@gmail.com>
Subject: [PATCH] xfs: fix bs_xflags copied from wrong compat field

Read bs_xflags from the correct compat field in
xfs_ioctl32_bstat_copyin().

Signed-off-by: Reid Downing (b1nv0id) <skibletz@gmail.com>
---
 fs/xfs/xfs_ioctl32.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/fs/xfs/xfs_ioctl32.c
+++ b/fs/xfs/xfs_ioctl32.c
@@ -124,7 +124,7 @@
            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_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] xfs: fix bs_xflags copied from wrong compat field

[Darrick J. Wong]

On Thu, Oct 15, 2026 at 10:11:57PM -0400, Reid D wrote:
> From: Reid Downing (b1nv0id) <skibletz@gmail.com>
> Subject: [PATCH] xfs: fix bs_xflags copied from wrong compat field
> 
> Read bs_xflags from the correct compat field in
> xfs_ioctl32_bstat_copyin().
> 
> Signed-off-by: Reid Downing (b1nv0id) <skibletz@gmail.com>

Yikes.  Good thing we don't actually /use/ bs_xflags in swapext.

Cc: <stable@vger.kernel.org> # v2.6.29
Fixes: e94fc4a43e5c39 ("[XFS] Add compat handlers for swapext ioctl")
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  fs/xfs/xfs_ioctl32.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> --- a/fs/xfs/xfs_ioctl32.c
> +++ b/fs/xfs/xfs_ioctl32.c
> @@ -124,7 +124,7 @@
>             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_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] xfs: fix bs_xflags copied from wrong compat field

[Eric Sandeen]

On 10/16/26 1:08 AM, Darrick J. Wong wrote:
> On Thu, Oct 15, 2026 at 10:11:57PM -0400, Reid D wrote:
>> From: Reid Downing (b1nv0id) <skibletz@gmail.com>
>> Subject: [PATCH] xfs: fix bs_xflags copied from wrong compat field
>>
>> Read bs_xflags from the correct compat field in
>> xfs_ioctl32_bstat_copyin().
>>
>> Signed-off-by: Reid Downing (b1nv0id) <skibletz@gmail.com>
> 
> Yikes.  Good thing we don't actually /use/ bs_xflags in swapext.

Oh good grief!

Reviewed-by: Eric Sandeen <sandeen@redhat.com>
Brown-paper-bag-on: Eric Sandeen <sandeen@redhat.com>

> Cc: <stable@vger.kernel.org> # v2.6.29
> Fixes: e94fc4a43e5c39 ("[XFS] Add compat handlers for swapext ioctl")
> Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
> 
> --D
> 
>> ---
>>  fs/xfs/xfs_ioctl32.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> --- a/fs/xfs/xfs_ioctl32.c
>> +++ b/fs/xfs/xfs_ioctl32.c
>> @@ -124,7 +124,7 @@
>>             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_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] xfs: fix bs_xflags copied from wrong compat field

[Christoph Hellwig]

I wonder if we should have a way to test these compat abis in
xfstests.  Have test programs (or xfs_io?) compiled with -m32
and compare to native?

Re: [PATCH] xfs: fix bs_xflags copied from wrong compat field

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH] xfs: fix bs_xflags copied from wrong compat field

[Eric Sandeen]

On 10/15/26 9:11 PM, Reid D wrote:
> From: Reid Downing (b1nv0id) <skibletz@gmail.com>
> Subject: [PATCH] xfs: fix bs_xflags copied from wrong compat field
> 
> Read bs_xflags from the correct compat field in
> xfs_ioctl32_bstat_copyin().
> 
> Signed-off-by: Reid Downing (b1nv0id) <skibletz@gmail.com>
> ---
>  fs/xfs/xfs_ioctl32.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> --- a/fs/xfs/xfs_ioctl32.c
> +++ b/fs/xfs/xfs_ioctl32.c
> @@ -124,7 +124,7 @@
>             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)      ||

Ummm we /all/ missed that the line just above is wrong, too. :(

Carlos, looks like this isn't in for-next yet so maybe we can fix it
properly before merge.

-Eric

> -           get_user(bstat->bs_xflags,  &bstat32->bs_size)      ||
> +           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] xfs: fix bs_xflags copied from wrong compat field

[Eric Sandeen]

On 10/27/26 10:18 AM, Christoph Hellwig wrote:
> I wonder if we should have a way to test these compat abis in
> xfstests.  Have test programs (or xfs_io?) compiled with -m32
> and compare to native?
What about 2 things:

A refactor (to eliminate the need to type out matching structure names twice)
and a kunit test?

Something like:

#define BSTAT_GET(f)            (bstat->f = bstat32->f)

xfs_bstat_from_compat(
        struct xfs_bstat                *bstat,
        const struct compat_xfs_bstat   *bstat32)
{
	BSTAT_GET(bs_ino);
	BSTAT_GET(bs_mode);
	BSTAT_GET(bs_nlink);
	...
}

...

xfs_ioctl32_bstat_copyin(
        struct xfs_bstat                *bstat,
        struct compat_xfs_bstat __user  *bstat32)
{
        struct compat_xfs_bstat         c;

        if (copy_from_user(&c, bstat32, sizeof(c)))
                return -EFAULT;
        xfs_bstat_from_compat(bstat, &c);
        return 0;
}

and then a kunit test that populates 32-bit and native structures with
unique values, runs the conversion both ways, and tests that those unique /
sentinel values match?

Not sure how we feel about kunit tests in general, and it could get to be quite
a lot of code to validate all the 32-bit compat conversions.

-Eric

Re: [PATCH] xfs: fix bs_xflags copied from wrong compat field

[Christoph Hellwig]

On Tue, Oct 27, 2026 at 11:26:09AM -0500, Eric Sandeen wrote:
> On 10/27/26 10:18 AM, Christoph Hellwig wrote:
> > I wonder if we should have a way to test these compat abis in
> > xfstests.  Have test programs (or xfs_io?) compiled with -m32
> > and compare to native?
> What about 2 things:
> 
> A refactor (to eliminate the need to type out matching structure names twice)
> and a kunit test?

I don't think kunit is all that helpful for copy to/from user tests..

> Something like:
> 
> #define BSTAT_GET(f)            (bstat->f = bstat32->f)
> 
> xfs_bstat_from_compat(
>         struct xfs_bstat                *bstat,
>         const struct compat_xfs_bstat   *bstat32)
> {
> 	BSTAT_GET(bs_ino);
> 	BSTAT_GET(bs_mode);
> 	BSTAT_GET(bs_nlink);
> 	...
> }

That makes the code so much harder to read for no real gain.