On Wed, Feb 23, 2022 at 12:24:02PM +0000, Trond Myklebust wrote: > On Wed, 2022-02-23 at 09:44 +0100, Christian Brauner wrote: > > On Sat, Feb 19, 2022 at 05:00:18PM +0000, Trond Myklebust wrote: > > > On Sat, 2022-02-19 at 12:34 +0100, Christian Brauner wrote: > > > > On Sat, Feb 19, 2022 at 08:34:30AM +0000, > > > > suy.fnst@xxxxxxxxxxx wrote: > > > > > Hi NFS folks, > > > > > During our xfstests, we found generic/633 fails like: > > > > > ============================================ > > > > > FSTYP -- nfs > > > > > PLATFORM -- Linux/x86_64 btrfs 5.17.0-rc4-custom #236 SMP > > > > > PREEMPT Sat Feb 19 15:09:03 CST 2022 > > > > > MKFS_OPTIONS -- 127.0.0.1:/nfsscratch > > > > > MOUNT_OPTIONS -- -o vers=4 127.0.0.1:/nfsscratch /mnt/scratch > > > > > > > > > > generic/633 0s ... [failed, exit status 1]- output mismatch > > > > > (see > > > > > /root/xfstests-dev/results//generic/633.out.bad) > > > > > --- tests/generic/633.out 2021-05-23 14:03:08.879999997 > > > > > +0800 > > > > > +++ /root/xfstests-dev/results//generic/633.out.bad 2022- > > > > > 02-19 > > > > > 16:31:28.660000013 +0800 > > > > > @@ -1,2 +1,4 @@ > > > > > QA output created by 633 > > > > > Silence is golden > > > > > +idmapped-mounts.c: 7906: setgid_create - Success - > > > > > failure: > > > > > is_setgid > > > > > +idmapped-mounts.c: 13907: run_test - Success - failure: > > > > > create > > > > > operations in directories with setgid bit set > > > > > ... > > > > > (Run 'diff -u /root/xfstests-dev/tests/generic/633.out > > > > > /root/xfstests-dev/results//generic/633.out.bad' to see the > > > > > entire > > > > > diff) > > > > > Ran: generic/633 > > > > > Failures: generic/633 > > > > > Failed 1 of 1 tests > > > > > ============================================ > > > > > > > > > > The failed test is about setgid inheritance. > > > > > When a file is created with S_ISGID in the directory with > > > > > S_ISGID, > > > > > NFS doesn't strip the setgid bit of the new created file but > > > > > others > > > > > (ext4/xfs/btrfs) do. They call inode_init_owner() which does > > > > > the strip after new_inode(). > > > > > However, NFS has its own logical to handle inode capacities. > > > > > As the test says the behavior can be filesystem type specific, > > > > > I'd report to you NFS guys and ask whether it's a bug or not? > > > > > > > > Thanks for the report. I'm not sure why NFS would have different > > > > rules > > > > for setgid inheritance. So I'm inclined to think that this is > > > > simply > > > > a > > > > bug similar to what we saw in xfs and ceph. But I'll let the NFS > > > > folks > > > > determine that. > > > > > > > > XFS is only special in so far as it has a sysctl that lets it > > > > alter > > > > sgid > > > > inheritance behavior. And it only has that to allow for legacy > > > > irix > > > > semantics iiuc. > > > > > > OK, so how do you expect this 'idmapped-mounts' functionality to > > > work > > > on NFS? I'm not asking about this bug in particular. I'm asking > > > about > > > what this is supposed to do in general. > > > > Just to clarify, the bug has nothing to do with idmapped mounts. The > > idmapped mount testsuite always had a vfs generic part. That vfs > > generic > > part has been made available to all filesystems supporting xfstests a > > few weeks ago. (So far this setgid inheritance bug here has been > > present > > in 3 filesystems.) > > The setgid stuff works just fine with regular use, when the server is > able to determine when to clear the bit. It only breaks with this kind > of test where the server is being lied to by the client's upper layers. I think you misunderstand: it is not possible to create idmapped mounts for a mounted NFS client. In order for a filesystem to support idmapped mounts it must set FS_ALLOW_IDMAP which currently only btrfs, ext4, fat, and xfs do. The failing test does not use idmapped mounts in any way.
