From: Eugene Teo <eteo@redhat.com> Date: Fri, 26 Sep 2008 19:55:33 +0800 Subject: [fs] open() allows setgid bit when user is not in group Message-id: 20080926115533.GA5759@kernel.sg O-Subject: [RHEL5.3 patch] BZ#463687 kernel: open() call allows setgid bit when user is not in new file's group [v2] Bugzilla: 463687 CVE: CVE-2008-4210 RH-Acked-by: Peter Staubach <staubach@redhat.com> RH-Acked-by: Alan Cox <alan@redhat.com> RH-Acked-by: Eric Sandeen <sandeen@redhat.com> RH-Acked-by: Jiri Pirko <jpirko@redhat.com> RH-Nacked-by: Eric Sandeen <sandeen@redhat.com> This is for bz#463687 (CVE-2008-4210). When creating a file, open()/creat() allows the setgid bit to be set via the mode argument even when, due to the bsdgroups mount option or the file being created in a setgid directory, the new file's group is one which the user is not a member of. The user can then use ftruncate() and memory-mapped I/O to turn the new file into an arbitrary binary and thus gain the privileges of this group, since these operations do not clear the setgid bit. Backport of upstream commit: 7b82dc0e64e93f430182f36b46b79fcee87d3532 and 01de85e057328ecbef36e108673b1e81059d54c1 Brew build: http://porkchop.redhat.com/brewroot/scratch/eteo/task_1497103 Test status: Booted on i686. Tested with reproducers: https://bugzilla.redhat.com/show_bug.cgi?id=463661#c3 https://bugzilla.redhat.com/show_bug.cgi?id=463661#c11 Before patch: -rwxr-s--- 1 test root 735004 Sep 26 07:01 sh* After patch: -rwxr-x--- 1 test root 735004 Sep 26 07:01 sh* Signed-off-by: Eugene Teo <eteo@redhat.com> diff --git a/fs/open.c b/fs/open.c index 80435ef..29b1947 100644 --- a/fs/open.c +++ b/fs/open.c @@ -215,6 +215,9 @@ int do_truncate(struct dentry *dentry, loff_t length, unsigned int time_attrs, newattrs.ia_valid |= ATTR_FILE; } + /* Remove suid/sgid on truncate too */ + newattrs.ia_valid |= should_remove_suid(dentry); + mutex_lock(&dentry->d_inode->i_mutex); err = notify_change(dentry, &newattrs); mutex_unlock(&dentry->d_inode->i_mutex); diff --git a/include/linux/fs.h b/include/linux/fs.h index f08cec8..bd4b676 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -1762,6 +1762,8 @@ extern void iget_failed(struct inode *); extern void clear_inode(struct inode *); extern void destroy_inode(struct inode *); extern struct inode *new_inode(struct super_block *); +extern int __remove_suid(struct dentry *, int); +extern int should_remove_suid(struct dentry *); extern int remove_suid(struct dentry *); extern void remove_dquot_ref(struct super_block *, int, struct list_head *); diff --git a/mm/filemap.c b/mm/filemap.c index 5f9971c..aa2bbcf 100644 --- a/mm/filemap.c +++ b/mm/filemap.c @@ -1914,11 +1914,10 @@ repeat: * if suid or (sgid and xgrp) * remove privs */ -int remove_suid(struct dentry *dentry) +int should_remove_suid(struct dentry *dentry) { mode_t mode = dentry->d_inode->i_mode; int kill = 0; - int result = 0; /* suid always must be killed */ if (unlikely(mode & S_ISUID)) @@ -1931,13 +1930,28 @@ int remove_suid(struct dentry *dentry) if (unlikely((mode & S_ISGID) && (mode & S_IXGRP))) kill |= ATTR_KILL_SGID; - if (unlikely(kill && !capable(CAP_FSETID))) { - struct iattr newattrs; + if (unlikely(kill && !capable(CAP_FSETID))) + return kill; - newattrs.ia_valid = ATTR_FORCE | kill; - result = notify_change(dentry, &newattrs); - } - return result; + return 0; +} + +int __remove_suid(struct dentry *dentry, int kill) +{ + struct iattr newattrs; + + newattrs.ia_valid = ATTR_FORCE | kill; + return notify_change(dentry, &newattrs); +} + +int remove_suid(struct dentry *dentry) +{ + int kill = should_remove_suid(dentry); + + if (unlikely(kill)) + return __remove_suid(dentry, kill); + + return 0; } EXPORT_SYMBOL(remove_suid);