aboutsummaryrefslogtreecommitdiffstats
path: root/fs/nfsd
diff options
context:
space:
mode:
authorJeff Layton <jlayton@redhat.com>2008-04-16 16:28:47 -0400
committerJ. Bruce Fields <bfields@citi.umich.edu>2008-04-23 16:13:43 -0400
commitca456252db0521e5e88024fa2b67535e9739e030 (patch)
tree3d62ce9e370dfe62e986ab44710262991a2ad2e1 /fs/nfsd
parentdee3209d993f17081d2c58d6470dfc8d6662078b (diff)
knfsd: clear both setuid and setgid whenever a chown is done
Currently, knfsd only clears the setuid bit if the owner of a file is changed on a SETATTR call, and only clears the setgid bit if the group is changed. POSIX says this in the spec for chown(): "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 does not have appropriate privileges, the set-user-ID (S_ISUID) and set-group-ID (S_ISGID) bits of the file mode shall be cleared upon successful return from chown()." If I'm reading this correctly, then knfsd is doing this wrong. It should be clearing both the setuid and setgid bit on any SETATTR that changes the uid or gid. This wasn't really as noticable before, but now that the ATTR_KILL_S*ID bits are a no-op for the NFS client, it's more evident. This patch corrects the nfsd_setattr logic so that this occurs. It also does a bit of cleanup to the function. There is also one small behavioral change. If a SETATTR call comes in that changes the uid/gid and the mode, then we now only clear the setgid bit if the group execute bit isn't set. The setgid bit without a group execute bit signifies mandatory locking and we likely don't want to clear the bit in that case. Since there is no call in POSIX that should generate a SETATTR call like this, then this should rarely happen, but it's worth noting. Signed-off-by: Jeff Layton <jlayton@redhat.com> Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu>
Diffstat (limited to 'fs/nfsd')
-rw-r--r--fs/nfsd/vfs.c27
1 files changed, 14 insertions, 13 deletions
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 1d0406c31a44..a3a291f771f4 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -359,24 +359,25 @@ nfsd_setattr(struct svc_rqst *rqstp, struct svc_fh *fhp, struct iattr *iap,
359 DQUOT_INIT(inode); 359 DQUOT_INIT(inode);
360 } 360 }
361 361
362 /* sanitize the mode change */
362 if (iap->ia_valid & ATTR_MODE) { 363 if (iap->ia_valid & ATTR_MODE) {
363 iap->ia_mode &= S_IALLUGO; 364 iap->ia_mode &= S_IALLUGO;
364 iap->ia_mode |= (inode->i_mode & ~S_IALLUGO); 365 iap->ia_mode |= (inode->i_mode & ~S_IALLUGO);
365 /* if changing uid/gid revoke setuid/setgid in mode */ 366 }
366 if ((iap->ia_valid & ATTR_UID) && iap->ia_uid != inode->i_uid) { 367
367 iap->ia_valid |= ATTR_KILL_PRIV; 368 /* Revoke setuid/setgid on chown */
369 if (((iap->ia_valid & ATTR_UID) && iap->ia_uid != inode->i_uid) ||
370 ((iap->ia_valid & ATTR_GID) && iap->ia_gid != inode->i_gid)) {
371 iap->ia_valid |= ATTR_KILL_PRIV;
372 if (iap->ia_valid & ATTR_MODE) {
373 /* we're setting mode too, just clear the s*id bits */
368 iap->ia_mode &= ~S_ISUID; 374 iap->ia_mode &= ~S_ISUID;
375 if (iap->ia_mode & S_IXGRP)
376 iap->ia_mode &= ~S_ISGID;
377 } else {
378 /* set ATTR_KILL_* bits and let VFS handle it */
379 iap->ia_valid |= (ATTR_KILL_SUID | ATTR_KILL_SGID);
369 } 380 }
370 if ((iap->ia_valid & ATTR_GID) && iap->ia_gid != inode->i_gid)
371 iap->ia_mode &= ~S_ISGID;
372 } else {
373 /*
374 * Revoke setuid/setgid bit on chown/chgrp
375 */
376 if ((iap->ia_valid & ATTR_UID) && iap->ia_uid != inode->i_uid)
377 iap->ia_valid |= ATTR_KILL_SUID | ATTR_KILL_PRIV;
378 if ((iap->ia_valid & ATTR_GID) && iap->ia_gid != inode->i_gid)
379 iap->ia_valid |= ATTR_KILL_SGID;
380 } 381 }
381 382
382 /* Change the attributes. */ 383 /* Change the attributes. */