[PATCH] fix vfs/utils.c for big-endian systems

[PATCH] fix vfs/utils.c for big-endian systems

[Eric Sandeen]

generic/633 was failing with EINVAL on the fsxgetattr call on s390.
Looks like this is due to a failure to properly endian swap the
arguments to the syscall, so fix that, and the magic_etc compare
in expected_dummy_vfs_caps_uid() as well while we're at it.

Fixes: 0d1af68e ("generic: add fstests for idmapped mounts")
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

diff --git a/src/vfs/utils.c b/src/vfs/utils.c
index c1c7951c..52bb7e42 100644
--- a/src/vfs/utils.c
+++ b/src/vfs/utils.c
@@ -650,7 +650,7 @@ bool expected_dummy_vfs_caps_uid(int fd, uid_t expected_uid)
 	if (ret < 0 || ret == 0)
 		return false;
 
-	if (ns_xattr.magic_etc & VFS_CAP_REVISION_3) {
+	if (le32_to_cpu(ns_xattr.magic_etc) & VFS_CAP_REVISION_3) {
 
 		if (le32_to_cpu(ns_xattr.rootid) != expected_uid) {
 			errno = EINVAL;
@@ -673,10 +673,12 @@ int set_dummy_vfs_caps(int fd, int flags, int rootuid)
 	ns_cap_data.data[(x) >> 5].permitted |= (1 << ((x)&31))
 
 	struct vfs_ns_cap_data ns_xattr;
+	__le32 magic_etc;
 
 	memset(&ns_xattr, 0, sizeof(ns_xattr));
 	__raise_cap_permitted(CAP_NET_RAW, ns_xattr);
-	ns_xattr.magic_etc |= VFS_CAP_REVISION_3 | VFS_CAP_FLAGS_EFFECTIVE;
+	magic_etc = (VFS_CAP_REVISION_3 | VFS_CAP_FLAGS_EFFECTIVE);
+	ns_xattr.magic_etc |= cpu_to_le32(magic_etc);
 	ns_xattr.rootid = cpu_to_le32(rootuid);
 
 	return fsetxattr(fd, "security.capability",

Re: [PATCH] fix vfs/utils.c for big-endian systems

[Darrick J. Wong]

On Mon, Jan 27, 2025 at 03:43:24PM -0600, Eric Sandeen wrote:
> generic/633 was failing with EINVAL on the fsxgetattr call on s390.
> Looks like this is due to a failure to properly endian swap the
> arguments to the syscall, so fix that, and the magic_etc compare
> in expected_dummy_vfs_caps_uid() as well while we're at it.

Hmmm, I see this when running the same test on ext4:

--- /tmp/fstests/tests/generic/633.out  2025-01-16 12:11:43.921449208 -0800
+++ /var/tmp/fstests/generic/633.out.bad        2025-01-27 13:49:27.413943989 -0800
@@ -1,2 +1,4 @@
 QA output created by 633
 Silence is golden
+utils.c: 928: openat_tmpfile_supported - Invalid argument - failure: create
+utils.c: 928: openat_tmpfile_supported - Invalid argument - failure: create

I think this is a separate bug?  And curiously it doesn't trigger on xfs
or btrfs :P

> Fixes: 0d1af68e ("generic: add fstests for idmapped mounts")
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> 
> diff --git a/src/vfs/utils.c b/src/vfs/utils.c
> index c1c7951c..52bb7e42 100644
> --- a/src/vfs/utils.c
> +++ b/src/vfs/utils.c
> @@ -650,7 +650,7 @@ bool expected_dummy_vfs_caps_uid(int fd, uid_t expected_uid)
>  	if (ret < 0 || ret == 0)
>  		return false;
>  
> -	if (ns_xattr.magic_etc & VFS_CAP_REVISION_3) {
> +	if (le32_to_cpu(ns_xattr.magic_etc) & VFS_CAP_REVISION_3) {
>  
>  		if (le32_to_cpu(ns_xattr.rootid) != expected_uid) {
>  			errno = EINVAL;
> @@ -673,10 +673,12 @@ int set_dummy_vfs_caps(int fd, int flags, int rootuid)
>  	ns_cap_data.data[(x) >> 5].permitted |= (1 << ((x)&31))
>  
>  	struct vfs_ns_cap_data ns_xattr;
> +	__le32 magic_etc;
>  
>  	memset(&ns_xattr, 0, sizeof(ns_xattr));
>  	__raise_cap_permitted(CAP_NET_RAW, ns_xattr);
> -	ns_xattr.magic_etc |= VFS_CAP_REVISION_3 | VFS_CAP_FLAGS_EFFECTIVE;
> +	magic_etc = (VFS_CAP_REVISION_3 | VFS_CAP_FLAGS_EFFECTIVE);
> +	ns_xattr.magic_etc |= cpu_to_le32(magic_etc);

Not sure why you wouldn't just pass (VFS_CAP_REVISION_3 |
VFS_CAP_FLAGS_EFFECTIVE) directly to cpu_to_le32 but it doesn't matter.

Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

>  	ns_xattr.rootid = cpu_to_le32(rootuid);
>  
>  	return fsetxattr(fd, "security.capability",
> 
> 

Re: [PATCH] fix vfs/utils.c for big-endian systems

[Christoph Hellwig]

On Mon, Jan 27, 2025 at 03:43:24PM -0600, Eric Sandeen wrote:
> generic/633 was failing with EINVAL on the fsxgetattr call on s390.
> Looks like this is due to a failure to properly endian swap the
> arguments to the syscall, so fix that, and the magic_etc compare
> in expected_dummy_vfs_caps_uid() as well while we're at it.

Looks good:

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

Do we need to enable sparse builds for xfstests?  Or is this
goign to create too much churn?

Re: [PATCH] fix vfs/utils.c for big-endian systems

[Eric Sandeen]

On 1/31/25 2:10 AM, Christoph Hellwig wrote:
> On Mon, Jan 27, 2025 at 03:43:24PM -0600, Eric Sandeen wrote:
>> generic/633 was failing with EINVAL on the fsxgetattr call on s390.
>> Looks like this is due to a failure to properly endian swap the
>> arguments to the syscall, so fix that, and the magic_etc compare
>> in expected_dummy_vfs_caps_uid() as well while we're at it.
> 
> Looks good:
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> 
> Do we need to enable sparse builds for xfstests?  Or is this
> goign to create too much churn?

Yeah I wondered about that. I bet adding the infrastructure would be trivial
but the fallout might be a whole other thing. ;)

-Eric

Re: [PATCH] fix vfs/utils.c for big-endian systems

[Christian Brauner]

On Mon, Jan 27, 2025 at 03:43:24PM -0600, Eric Sandeen wrote:
> generic/633 was failing with EINVAL on the fsxgetattr call on s390.
> Looks like this is due to a failure to properly endian swap the
> arguments to the syscall, so fix that, and the magic_etc compare
> in expected_dummy_vfs_caps_uid() as well while we're at it.
> 
> Fixes: 0d1af68e ("generic: add fstests for idmapped mounts")
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---

Acked-by: Christian Brauner <brauner@kernel.org>