| Message ID | fb7ad928-bbe9-4488-a6b7-a2786782bccd@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | fix vfs/utils.c for big-endian systems | expand |
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", > >
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?
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
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>
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",
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> ---