Attachment 303510 Details for Bug 443043 – patch -- revoke both setuid and setgid on uid/gid changes

[patch] patch -- revoke both setuid and setgid on uid/gid changes

18-bz-443043-knfsd-revoke-both-se.patch (text/plain), 1.67 KB, created by Jeff Layton on 2008-04-23 15:23:30 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jeff Layton

Created: 2008-04-23 15:23:30 UTC

Size: 1.67 KB

patch

obsolete

>BZ#443043: knfsd: revoke both setuid and setgid when uid or gid changes
>
>From: Jeff Layton <jlayton@redhat.com>
>
>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.
>---
>
> fs/nfsd/vfs.c |    7 +++----
> 1 files changed, 3 insertions(+), 4 deletions(-)
>
>
>diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
>index ebd8bf6..e3bae15 100644
>--- a/fs/nfsd/vfs.c
>+++ b/fs/nfsd/vfs.c
>@@ -352,10 +352,9 @@ nfsd_setattr(struct svc_rqst *rqstp, struct svc_fh *fhp, struct iattr *iap,
> 	}
> 
> 	/* Revoke setuid/setgid bit on chown/chgrp */
>-	if ((iap->ia_valid & ATTR_UID) && iap->ia_uid != inode->i_uid)
>-		iap->ia_valid |= ATTR_KILL_SUID;
>-	if ((iap->ia_valid & ATTR_GID) && iap->ia_gid != inode->i_gid)
>-		iap->ia_valid |= ATTR_KILL_SGID;
>+	if (((iap->ia_valid & ATTR_UID) && iap->ia_uid != inode->i_uid) ||
>+	    ((iap->ia_valid & ATTR_GID) && iap->ia_gid != inode->i_gid))
>+		iap->ia_valid |= (ATTR_KILL_SGID | ATTR_KILL_SUID);
> 
> 	/* Change the attributes. */
>

Actions: View | Diff

Attachments on bug 443043: 303510