Thread (11 messages) 11 messages, 5 authors, 2023-05-15
STALE1129d

[PATCH] nfsd: use vfs setgid helper

From: Christian Brauner <brauner@kernel.org>
Date: 2023-05-02 13:36:30
Subsystem: filesystems (vfs and infrastructure), kernel nfsd, sunrpc, and lockd servers, the rest · Maintainers: Alexander Viro, Christian Brauner, Chuck Lever, Jeff Layton, Linus Torvalds

We've aligned setgid behavior over multiple kernel releases. The details
can be found in commit cf619f891971 ("Merge tag 'fs.ovl.setgid.v6.2' of
git://git.kernel.org/pub/scm/linux/kernel/git/vfs/idmapping") and
commit 426b4ca2d6a5 ("Merge tag 'fs.setgid.v6.0' of
git://git.kernel.org/pub/scm/linux/kernel/git/brauner/linux").
Consistent setgid stripping behavior is now encapsulated in the
setattr_should_drop_sgid() helper which is used by all filesystems that
strip setgid bits outside of vfs proper. Usually ATTR_KILL_SGID is
raised in e.g., chown_common() and is subject to the
setattr_should_drop_sgid() check to determine whether the setgid bit can
be retained. Since nfsd is raising ATTR_KILL_SGID unconditionally it
will cause notify_change() to strip it even if the caller had the
necessary privileges to retain it. Ensure that nfsd only raises
ATR_KILL_SGID if the caller lacks the necessary privileges to retain the
setgid bit.

Without this patch the setgid stripping tests in LTP will fail:
As you can see, the problem is S_ISGID (0002000) was dropped on a
non-group-executable file while chown was invoked by super-user, while
[...]
fchown02.c:66: TFAIL: testfile2: wrong mode permissions 0100700, expected 0102700
[...]
chown02.c:57: TFAIL: testfile2: wrong mode permissions 0100700, expected 0102700
With this patch all tests pass.

Reported-by: Sherry Yang <redacted>
Signed-off-by: Christian Brauner <brauner@kernel.org>
---
ubuntu@imp1-vm:~/ltp-install$ sudo ./runltp -d /mnt -s chown02
INFO: ltp-pan reported all tests PASS
LTP Version: 20230127-112-gf41e8a2fa

ubuntu@imp1-vm:~/ltp-install$ sudo ./runltp -d /mnt -s fchown02
INFO: ltp-pan reported all tests PASS
LTP Version: 20230127-112-gf41e8a2fa

ubuntu@imp1-vm:~/src/git/xfstests$ sudo ./check -g perms
FSTYP         -- nfs
PLATFORM      -- Linux/x86_64 imp1-vm 6.3.0-nfs-setgid-3a3cfe624076 #20 SMP PREEMPT_DYNAMIC Tue May  2 12:35:51 UTC 2023
MKFS_OPTIONS  -- 127.0.0.1:/nfsscratch
MOUNT_OPTIONS -- -o vers=3,noacl 127.0.0.1:/nfsscratch /mnt/scratch
Passed all 41 tests
---
 fs/nfsd/vfs.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index bb9d47172162..c4ef24c5ffd0 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -388,7 +388,9 @@ nfsd_sanitize_attrs(struct inode *inode, struct iattr *iap)
 				iap->ia_mode &= ~S_ISGID;
 		} else {
 			/* set ATTR_KILL_* bits and let VFS handle it */
-			iap->ia_valid |= (ATTR_KILL_SUID | ATTR_KILL_SGID);
+			iap->ia_valid |= ATTR_KILL_SUID;
+			iap->ia_valid |=
+				setattr_should_drop_sgid(&nop_mnt_idmap, inode);
 		}
 	}
 }
-- 
2.34.1
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help