From c4fa6d7c5971baa45eca4f81b8f100299848486a Mon Sep 17 00:00:00 2001 From: Stanislav Kholmanskikh Date: Wed, 11 Dec 2013 14:16:36 +0400 Subject: [PATCH] nfsd: revoking of suid/sgid bits after chown() in a consistent way There is an inconsistency in the handling of SUID/SGID file bits after chown() between NFS and other local file systems. Local file systems (for example, ext3, ext4, xfs, btrfs) revoke SUID/SGID bits after chown() on a regular file even if the owner/group of the file has not been changed: ~# touch file; chmod ug+s file; chmod u+x file ~# ls -l file -rwsr-Sr-- 1 root root 0 Dec 6 04:49 file ~# chown root file; ls -l file -rwxr-Sr-- 1 root root 0 Dec 6 04:49 file but NFS doesn't do that: ~# touch file; chmod ug+s file; chmod u+x file ~# ls -l file -rwsr-Sr-- 1 root root 0 Dec 6 04:49 file ~# chown root file; ls -l file -rwsr-Sr-- 1 root root 0 Dec 6 04:49 file NFS does that only if the owner/group has been changed: ~# touch file; chmod ug+s file; chmod u+x file ~# ls -l file -rwsr-Sr-- 1 root root 0 Dec 6 05:02 file ~# chown bin file; ls -l file -rwxr-Sr-- 1 bin root 0 Dec 6 05:02 file See: http://pubs.opengroup.org/onlinepubs/9699919799/functions/chown.html "If the specified file is a regular file, one or more of the S_IXUSR, S_IXGRP, or S_IXOTH bits of the file mode are set, and the process has appropriate privileges, it is implementation-defined whether the set-user-ID and set-group-ID bits are altered." So both variants are acceptable by POSIX. This patch makes NFS to behave like local file systems. Signed-off-by: Stanislav Kholmanskikh Signed-off-by: J. Bruce Fields --- fs/nfsd/vfs.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c index 7eea63cada1d..c8aa6ff99259 100644 --- a/fs/nfsd/vfs.c +++ b/fs/nfsd/vfs.c @@ -348,8 +348,7 @@ nfsd_sanitize_attrs(struct inode *inode, struct iattr *iap) /* Revoke setuid/setgid on chown */ if (!S_ISDIR(inode->i_mode) && - (((iap->ia_valid & ATTR_UID) && !uid_eq(iap->ia_uid, inode->i_uid)) || - ((iap->ia_valid & ATTR_GID) && !gid_eq(iap->ia_gid, inode->i_gid)))) { + ((iap->ia_valid & ATTR_UID) || (iap->ia_valid & ATTR_GID))) { iap->ia_valid |= ATTR_KILL_PRIV; if (iap->ia_valid & ATTR_MODE) { /* we're setting mode too, just clear the s*id bits */