Hi, Christian, Thank you so much for your explanation. > > I tested on kernel 5.14.0-162.6.1.el9_1.x86_64, and it failed with > "no_root_squash" set. > > But after I apply commit 1639a49ccdce58ea248841ed9b23babcce6dbb0b > onto > > kernel 5.14.0-162.6.1.el9_1.x86_64, the case will pass. > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/com > > mit/?h=1639a49ccdce58ea248841ed9b23babcce6dbb0b > > Ah, good. That's crucial information as no_root_squash did work before and it > would've been a regression if it suddenly would leave the setgid bit set. If I don't apply this patch, the setgid bit will not be stripped. So is this behavior an NFS bug? Does NFS need to strip SGID bit when the "no_root_squash" set or "root_squash" set? Thanks, ★☆★☆★☆★☆FNSTオンラインへようこそ★☆★☆★☆★☆ FNST最新情報盛りたくさん！ http://online.fnst.cn.fujitsu.com/fnst-news ★☆★☆★☆★☆★☆★☆★☆★☆★☆★☆★☆★☆★☆ > -----Original Message----- > From: Christian Brauner <brauner@xxxxxxxxxx> > Sent: Tuesday, January 3, 2023 6:35 PM > To: Cui, Yue/崔 悦 <cuiyue-fnst@xxxxxxxxxxx> > Cc: Christian Brauner <christian@xxxxxxxxxx>; linux-nfs@xxxxxxxxxxxxxxx > Subject: Re: nfs setgid inheritance test > > On Tue, Jan 03, 2023 at 02:58:29AM +0000, cuiyue-fnst@xxxxxxxxxxx wrote: > > Hello Christian, > > > > > > Thank you for your response. > > > > > > > > > Afaict, nothing has changed and the test should still be skipped. > > > > > I'm not sure I ever send a patch to skip this test specifically > > > > > for nfs though. I might just not have gotten around to that. > > > > > > > > > > Can you please also send the exact steps for reproducing this issue? > > > > > > > > The reproducing steps is as follows: > > > > > > > > Client & Server: > > > > 1. Install xfstests > > > > 2. # yum install libcap-devel > > > > > > > > Server: > > > > 1. Set exports file. > > > > # echo "/nfstest > > > *(rw,insecure,no_subtree_check,no_root_squash,fsid=1) > > > > /nfsscratch > > > *(rw,insecure,no_subtree_check,no_root_squash,fsid=2)" >/etc/exports > > > > 2. Restart services. > > > > # systemctl restart rpcbind.service # systemctl restart > > > > nfs-server.service # systemctl restart rpc-statd.service > > > > > > > > Client: > > > > 1. Create mount point > > > > # mkdir -p /mnt/test > > > > # mkdir -p /mnt/scratch > > > > 2. Cofigure NFS parameters. > > > > # echo "FSTYP=nfs > > > > TEST_DEV=server_IP:/nfstest > > > > TEST_DIR=/mnt/test > > > > SCRATCH_DEV=server_IP:/nfsscratch > > > > SCRATCH_MNT=/mnt/scratch > > > > export KEEP_DMESG=yes > > > > NFS_MOUNT_OPTIONS=\"-o vers=3\"">/var/lib/xfstests/local.config > > > > 3. Test > > > > # ./check -d generic/633 > > > > > > The tests should pass with "no_root_squash" set. The root cause of > > > the original issue was that files created by root are squashed to > > > 65534 which breaks setgid inheritance rules for S_ISGID directories. > > > > > > But without root squashing the tests should succeed. If I reproduce > > > this exactly with your instructions on a v6.2-rc1 kernel I get a success as > expected. > > > > > > I don't think you've told me What kernel you are testing this on? > > > > Sorry, I didn't make it clearly before. > > No worries. > > > I tested on kernel 5.14.0-162.6.1.el9_1.x86_64, and it failed with > "no_root_squash" set. > > But after I apply commit 1639a49ccdce58ea248841ed9b23babcce6dbb0b > onto > > kernel 5.14.0-162.6.1.el9_1.x86_64, the case will pass. > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/com > > mit/?h=1639a49ccdce58ea248841ed9b23babcce6dbb0b > > Ah, good. That's crucial information as no_root_squash did work before and it > would've been a regression if it suddenly would leave the setgid bit set. > > > This patch moves S_ISGID stripping into the vfs, so NFS can solve the setgid > inheritance problem. > > > > But although the test can succeed, when the root is squashed to nobody, is it > still suitable to use generic/633 to test? > > No, when root squashing is enabled the test shouldn't run. I've mentioned this in > my earlier mail. > > Just one example, when you create a new file in a setgid directory then the new > file will inherit the gid of the directory it has been created in. But with root > squashing that's no longer the case for the root user since root squashing > changes the {g,u}id that a file is created as. It essentially idmaps {g,u}id 0 to > 655345. That means reasoning about setgid inheritance rules as the root user > doesn't work in the tests anymore. If that is a desirable thing then xfstests > should gain a new nfs specific test for this case.
