| Message ID | 20170906085453.22382-2-juerg.haefliger@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | Fix for CVE-2016-7097 | expand |
On 09/06/17 10:54, Juerg Haefliger wrote: > From: Jan Kara <jack@suse.cz> > > commit 073931017b49d9458aa351605b43a7e34598caef upstream. > > When file permissions are modified via chmod(2) and the user is not in > the owning group or capable of CAP_FSETID, the setgid bit is cleared in > inode_change_ok(). Setting a POSIX ACL via setxattr(2) sets the file > permissions as well as the new ACL, but doesn't clear the setgid bit in > a similar way; this allows to bypass the check in chmod(2). Fix that. > > References: CVE-2016-7097 > Reviewed-by: Christoph Hellwig <hch@lst.de> > Reviewed-by: Jeff Layton <jlayton@redhat.com> > Signed-off-by: Jan Kara <jack@suse.cz> > Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com> > [bwh: Backported to 3.16: > - Drop changes to orangefs > - Adjust context > - Update ext3 as well] > Signed-off-by: Ben Hutchings <ben@decadent.org.uk> > > CVE-2016-7097 > Should we add here the sha1 of the 3.16 backport commit since the original SOB comes from it? Probably: (backported from f2ba3e2310b3967720b83126db8684c69ce41894 3.16.y) I think we can add that while applying the patch. Otherwise it looks a sane backport and a nice combination of the patches from 3.2 and 3.16 :-). Kleber > [juergh: Backported to 3.13: > - Drop changes to ceph > - Use capable() instead of capable_wrt_inode_uidgid() > - Update generic_acl.c as well > - In gfs2, jfs, and xfs, take care to avoid leaking the allocated ACL if > posix_acl_update_mode() determines it's not needed] > Signed-off-by: Juerg Haefliger <juerg.haefliger@canonical.com> > --- > fs/9p/acl.c | 40 +++++++++++++++++----------------------- > fs/btrfs/acl.c | 6 ++---- > fs/ext2/acl.c | 12 ++++-------- > fs/ext3/acl.c | 12 ++++-------- > fs/ext4/acl.c | 12 ++++-------- > fs/f2fs/acl.c | 6 ++---- > fs/generic_acl.c | 15 ++++++++------- > fs/gfs2/acl.c | 16 +++++++--------- > fs/hfsplus/posix_acl.c | 4 ++-- > fs/jffs2/acl.c | 9 ++++----- > fs/jfs/xattr.c | 6 ++++-- > fs/ocfs2/acl.c | 9 +++------ > fs/posix_acl.c | 30 ++++++++++++++++++++++++++++++ > fs/reiserfs/xattr_acl.c | 8 ++------ > fs/xfs/xfs_acl.c | 17 +++++++---------- > include/linux/posix_acl.h | 1 + > 16 files changed, 101 insertions(+), 102 deletions(-) > > diff --git a/fs/9p/acl.c b/fs/9p/acl.c > index 7af425f53bee..9686c1f17653 100644 > --- a/fs/9p/acl.c > +++ b/fs/9p/acl.c > @@ -320,32 +320,26 @@ static int v9fs_xattr_set_acl(struct dentry *dentry, const char *name, > case ACL_TYPE_ACCESS: > name = POSIX_ACL_XATTR_ACCESS; > if (acl) { > - umode_t mode = inode->i_mode; > - retval = posix_acl_equiv_mode(acl, &mode); > - if (retval < 0) > + struct iattr iattr; > + > + retval = posix_acl_update_mode(inode, &iattr.ia_mode, &acl); > + if (retval) > goto err_out; > - else { > - struct iattr iattr; > - if (retval == 0) { > - /* > - * ACL can be represented > - * by the mode bits. So don't > - * update ACL. > - */ > - acl = NULL; > - value = NULL; > - size = 0; > - } > - /* Updte the mode bits */ > - iattr.ia_mode = ((mode & S_IALLUGO) | > - (inode->i_mode & ~S_IALLUGO)); > - iattr.ia_valid = ATTR_MODE; > - /* FIXME should we update ctime ? > - * What is the following setxattr update the > - * mode ? > + if (!acl) { > + /* > + * ACL can be represented > + * by the mode bits. So don't > + * update ACL. > */ > - v9fs_vfs_setattr_dotl(dentry, &iattr); > + value = NULL; > + size = 0; > } > + iattr.ia_valid = ATTR_MODE; > + /* FIXME should we update ctime ? > + * What is the following setxattr update the > + * mode ? > + */ > + v9fs_vfs_setattr_dotl(dentry, &iattr); > } > break; > case ACL_TYPE_DEFAULT: > diff --git a/fs/btrfs/acl.c b/fs/btrfs/acl.c > index 0890c83643e9..d6d53e5e7945 100644 > --- a/fs/btrfs/acl.c > +++ b/fs/btrfs/acl.c > @@ -118,11 +118,9 @@ static int btrfs_set_acl(struct btrfs_trans_handle *trans, > case ACL_TYPE_ACCESS: > name = POSIX_ACL_XATTR_ACCESS; > if (acl) { > - ret = posix_acl_equiv_mode(acl, &inode->i_mode); > - if (ret < 0) > + ret = posix_acl_update_mode(inode, &inode->i_mode, &acl); > + if (ret) > return ret; > - if (ret == 0) > - acl = NULL; > } > ret = 0; > break; > diff --git a/fs/ext2/acl.c b/fs/ext2/acl.c > index 110b6b371a4e..48c3c2d7d261 100644 > --- a/fs/ext2/acl.c > +++ b/fs/ext2/acl.c > @@ -206,15 +206,11 @@ ext2_set_acl(struct inode *inode, int type, struct posix_acl *acl) > case ACL_TYPE_ACCESS: > name_index = EXT2_XATTR_INDEX_POSIX_ACL_ACCESS; > if (acl) { > - error = posix_acl_equiv_mode(acl, &inode->i_mode); > - if (error < 0) > + error = posix_acl_update_mode(inode, &inode->i_mode, &acl); > + if (error) > return error; > - else { > - inode->i_ctime = CURRENT_TIME_SEC; > - mark_inode_dirty(inode); > - if (error == 0) > - acl = NULL; > - } > + inode->i_ctime = CURRENT_TIME_SEC; > + mark_inode_dirty(inode); > } > break; > > diff --git a/fs/ext3/acl.c b/fs/ext3/acl.c > index dbb5ad59a7fc..bb2f60a62d82 100644 > --- a/fs/ext3/acl.c > +++ b/fs/ext3/acl.c > @@ -205,15 +205,11 @@ ext3_set_acl(handle_t *handle, struct inode *inode, int type, > case ACL_TYPE_ACCESS: > name_index = EXT3_XATTR_INDEX_POSIX_ACL_ACCESS; > if (acl) { > - error = posix_acl_equiv_mode(acl, &inode->i_mode); > - if (error < 0) > + error = posix_acl_update_mode(inode, &inode->i_mode, &acl); > + if (error) > return error; > - else { > - inode->i_ctime = CURRENT_TIME_SEC; > - ext3_mark_inode_dirty(handle, inode); > - if (error == 0) > - acl = NULL; > - } > + inode->i_ctime = CURRENT_TIME_SEC; > + ext3_mark_inode_dirty(handle, inode); > } > break; > > diff --git a/fs/ext4/acl.c b/fs/ext4/acl.c > index 39a54a0e9fe4..c844f1bfb451 100644 > --- a/fs/ext4/acl.c > +++ b/fs/ext4/acl.c > @@ -211,15 +211,11 @@ ext4_set_acl(handle_t *handle, struct inode *inode, int type, > case ACL_TYPE_ACCESS: > name_index = EXT4_XATTR_INDEX_POSIX_ACL_ACCESS; > if (acl) { > - error = posix_acl_equiv_mode(acl, &inode->i_mode); > - if (error < 0) > + error = posix_acl_update_mode(inode, &inode->i_mode, &acl); > + if (error) > return error; > - else { > - inode->i_ctime = ext4_current_time(inode); > - ext4_mark_inode_dirty(handle, inode); > - if (error == 0) > - acl = NULL; > - } > + inode->i_ctime = ext4_current_time(inode); > + ext4_mark_inode_dirty(handle, inode); > } > break; > > diff --git a/fs/f2fs/acl.c b/fs/f2fs/acl.c > index d0fc287efeff..0eb2d66827ad 100644 > --- a/fs/f2fs/acl.c > +++ b/fs/f2fs/acl.c > @@ -224,12 +224,10 @@ static int f2fs_set_acl(struct inode *inode, int type, > case ACL_TYPE_ACCESS: > name_index = F2FS_XATTR_INDEX_POSIX_ACL_ACCESS; > if (acl) { > - error = posix_acl_equiv_mode(acl, &inode->i_mode); > - if (error < 0) > + error = posix_acl_update_mode(inode, &inode->i_mode, &acl); > + if (error) > return error; > set_acl_inode(fi, inode->i_mode); > - if (error == 0) > - acl = NULL; > } > break; > > diff --git a/fs/generic_acl.c b/fs/generic_acl.c > index b3f3676796d3..67319f168b42 100644 > --- a/fs/generic_acl.c > +++ b/fs/generic_acl.c > @@ -86,16 +86,17 @@ generic_acl_set(struct dentry *dentry, const char *name, const void *value, > if (error) > goto failed; > switch (type) { > - case ACL_TYPE_ACCESS: > - error = posix_acl_equiv_mode(acl, &inode->i_mode); > - if (error < 0) > + case ACL_TYPE_ACCESS: { > + struct posix_acl *saved_acl = acl; > + > + error = posix_acl_update_mode(inode, &inode->i_mode, &acl); > + if (acl == NULL) > + posix_acl_release(saved_acl); > + if (error) > goto failed; > inode->i_ctime = CURRENT_TIME; > - if (error == 0) { > - posix_acl_release(acl); > - acl = NULL; > - } > break; > + } > case ACL_TYPE_DEFAULT: > if (!S_ISDIR(inode->i_mode)) { > error = -EINVAL; > diff --git a/fs/gfs2/acl.c b/fs/gfs2/acl.c > index f69ac0af5496..015809a066b5 100644 > --- a/fs/gfs2/acl.c > +++ b/fs/gfs2/acl.c > @@ -267,16 +267,14 @@ static int gfs2_xattr_system_set(struct dentry *dentry, const char *name, > goto out_release; > > if (type == ACL_TYPE_ACCESS) { > - umode_t mode = inode->i_mode; > - error = posix_acl_equiv_mode(acl, &mode); > + struct posix_acl *saved_acl = acl; > + umode_t mode; > > - if (error <= 0) { > - posix_acl_release(acl); > - acl = NULL; > - > - if (error < 0) > - return error; > - } > + error = posix_acl_update_mode(inode, &mode, &acl); > + if (error || acl == NULL) > + posix_acl_release(saved_acl); > + if (error) > + return error; > > error = gfs2_set_mode(inode, mode); > if (error) > diff --git a/fs/hfsplus/posix_acl.c b/fs/hfsplus/posix_acl.c > index b609cc14c72e..9f7cc491ffb1 100644 > --- a/fs/hfsplus/posix_acl.c > +++ b/fs/hfsplus/posix_acl.c > @@ -72,8 +72,8 @@ static int hfsplus_set_posix_acl(struct inode *inode, > case ACL_TYPE_ACCESS: > xattr_name = POSIX_ACL_XATTR_ACCESS; > if (acl) { > - err = posix_acl_equiv_mode(acl, &inode->i_mode); > - if (err < 0) > + err = posix_acl_update_mode(inode, &inode->i_mode, &acl); > + if (err) > return err; > } > err = 0; > diff --git a/fs/jffs2/acl.c b/fs/jffs2/acl.c > index 223283c30111..9335b8d3cf52 100644 > --- a/fs/jffs2/acl.c > +++ b/fs/jffs2/acl.c > @@ -243,9 +243,10 @@ static int jffs2_set_acl(struct inode *inode, int type, struct posix_acl *acl) > case ACL_TYPE_ACCESS: > xprefix = JFFS2_XPREFIX_ACL_ACCESS; > if (acl) { > - umode_t mode = inode->i_mode; > - rc = posix_acl_equiv_mode(acl, &mode); > - if (rc < 0) > + umode_t mode; > + > + rc = posix_acl_update_mode(inode, &mode, &acl); > + if (rc) > return rc; > if (inode->i_mode != mode) { > struct iattr attr; > @@ -257,8 +258,6 @@ static int jffs2_set_acl(struct inode *inode, int type, struct posix_acl *acl) > if (rc < 0) > return rc; > } > - if (rc == 0) > - acl = NULL; > } > break; > case ACL_TYPE_DEFAULT: > diff --git a/fs/jfs/xattr.c b/fs/jfs/xattr.c > index d3472f4cd530..6910662a8bf5 100644 > --- a/fs/jfs/xattr.c > +++ b/fs/jfs/xattr.c > @@ -693,9 +693,11 @@ static int can_set_system_xattr(struct inode *inode, const char *name, > return rc; > } > if (acl) { > - rc = posix_acl_equiv_mode(acl, &inode->i_mode); > + struct posix_acl *dummy = acl; > + > + rc = posix_acl_update_mode(inode, &inode->i_mode, &dummy); > posix_acl_release(acl); > - if (rc < 0) { > + if (rc) { > printk(KERN_ERR > "posix_acl_equiv_mode returned %d\n", > rc); > diff --git a/fs/ocfs2/acl.c b/fs/ocfs2/acl.c > index b4f788e0ca31..b16bb5c70bc8 100644 > --- a/fs/ocfs2/acl.c > +++ b/fs/ocfs2/acl.c > @@ -270,14 +270,11 @@ static int ocfs2_set_acl(handle_t *handle, > case ACL_TYPE_ACCESS: > name_index = OCFS2_XATTR_INDEX_POSIX_ACL_ACCESS; > if (acl) { > - umode_t mode = inode->i_mode; > - ret = posix_acl_equiv_mode(acl, &mode); > - if (ret < 0) > + umode_t mode; > + ret = posix_acl_update_mode(inode, &mode, &acl); > + if (ret) > return ret; > else { > - if (ret == 0) > - acl = NULL; > - > ret = ocfs2_acl_set_mode(inode, di_bh, > handle, mode); > if (ret) > diff --git a/fs/posix_acl.c b/fs/posix_acl.c > index 3542f1f814e2..8161e5c9dc31 100644 > --- a/fs/posix_acl.c > +++ b/fs/posix_acl.c > @@ -407,6 +407,36 @@ posix_acl_create(struct posix_acl **acl, gfp_t gfp, umode_t *mode_p) > } > EXPORT_SYMBOL(posix_acl_create); > > +/** > + * posix_acl_update_mode - update mode in set_acl > + * > + * Update the file mode when setting an ACL: compute the new file permission > + * bits based on the ACL. In addition, if the ACL is equivalent to the new > + * file mode, set *acl to NULL to indicate that no ACL should be set. > + * > + * As with chmod, clear the setgit bit if the caller is not in the owning group > + * or capable of CAP_FSETID (see inode_change_ok). > + * > + * Called from set_acl inode operations. > + */ > +int posix_acl_update_mode(struct inode *inode, umode_t *mode_p, > + struct posix_acl **acl) > +{ > + umode_t mode = inode->i_mode; > + int error; > + > + error = posix_acl_equiv_mode(*acl, &mode); > + if (error < 0) > + return error; > + if (error == 0) > + *acl = NULL; > + if (!in_group_p(inode->i_gid) && !capable(CAP_FSETID)) > + mode &= ~S_ISGID; > + *mode_p = mode; > + return 0; > +} > +EXPORT_SYMBOL(posix_acl_update_mode); > + > int > posix_acl_chmod(struct posix_acl **acl, gfp_t gfp, umode_t mode) > { > diff --git a/fs/reiserfs/xattr_acl.c b/fs/reiserfs/xattr_acl.c > index 06c04f73da65..a86ad7ec7957 100644 > --- a/fs/reiserfs/xattr_acl.c > +++ b/fs/reiserfs/xattr_acl.c > @@ -288,13 +288,9 @@ reiserfs_set_acl(struct reiserfs_transaction_handle *th, struct inode *inode, > case ACL_TYPE_ACCESS: > name = POSIX_ACL_XATTR_ACCESS; > if (acl) { > - error = posix_acl_equiv_mode(acl, &inode->i_mode); > - if (error < 0) > + error = posix_acl_update_mode(inode, &inode->i_mode, &acl); > + if (error) > return error; > - else { > - if (error == 0) > - acl = NULL; > - } > } > break; > case ACL_TYPE_DEFAULT: > diff --git a/fs/xfs/xfs_acl.c b/fs/xfs/xfs_acl.c > index 370eb3e121d1..89ac0522b38d 100644 > --- a/fs/xfs/xfs_acl.c > +++ b/fs/xfs/xfs_acl.c > @@ -402,17 +402,14 @@ xfs_xattr_acl_set(struct dentry *dentry, const char *name, > goto out_release; > > if (type == ACL_TYPE_ACCESS) { > - umode_t mode = inode->i_mode; > - error = posix_acl_equiv_mode(acl, &mode); > - > - if (error <= 0) { > - posix_acl_release(acl); > - acl = NULL; > - > - if (error < 0) > - return error; > - } > + struct posix_acl *saved_acl = acl; > + umode_t mode; > > + error = posix_acl_update_mode(inode, &mode, &acl); > + if (error || acl == NULL) > + posix_acl_release(saved_acl); > + if (error) > + return error; > error = xfs_set_mode(inode, mode); > if (error) > goto out_release; > diff --git a/include/linux/posix_acl.h b/include/linux/posix_acl.h > index 7931efe71175..2ae0bba45f12 100644 > --- a/include/linux/posix_acl.h > +++ b/include/linux/posix_acl.h > @@ -90,6 +90,7 @@ extern struct posix_acl *posix_acl_from_mode(umode_t, gfp_t); > extern int posix_acl_equiv_mode(const struct posix_acl *, umode_t *); > extern int posix_acl_create(struct posix_acl **, gfp_t, umode_t *); > extern int posix_acl_chmod(struct posix_acl **, gfp_t, umode_t); > +extern int posix_acl_update_mode(struct inode *, umode_t *, struct posix_acl **); > > extern struct posix_acl *get_posix_acl(struct inode *, int); > extern int set_posix_acl(struct inode *, int, struct posix_acl *); >
On 09/06/17 14:59, Kleber Souza wrote: > On 09/06/17 10:54, Juerg Haefliger wrote: >> From: Jan Kara <jack@suse.cz> >> >> commit 073931017b49d9458aa351605b43a7e34598caef upstream. >> >> When file permissions are modified via chmod(2) and the user is not in >> the owning group or capable of CAP_FSETID, the setgid bit is cleared in >> inode_change_ok(). Setting a POSIX ACL via setxattr(2) sets the file >> permissions as well as the new ACL, but doesn't clear the setgid bit in >> a similar way; this allows to bypass the check in chmod(2). Fix that. >> >> References: CVE-2016-7097 >> Reviewed-by: Christoph Hellwig <hch@lst.de> >> Reviewed-by: Jeff Layton <jlayton@redhat.com> >> Signed-off-by: Jan Kara <jack@suse.cz> >> Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com> >> [bwh: Backported to 3.16: >> - Drop changes to orangefs >> - Adjust context >> - Update ext3 as well] >> Signed-off-by: Ben Hutchings <ben@decadent.org.uk> >> >> CVE-2016-7097 >> > > Should we add here the sha1 of the 3.16 backport commit since the > original SOB comes from it? > > Probably: > (backported from f2ba3e2310b3967720b83126db8684c69ce41894 3.16.y) > > I think we can add that while applying the patch. > > Otherwise it looks a sane backport and a nice combination of the patches > from 3.2 and 3.16 :-). Now with the actual ACK: Acked-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com> > > > Kleber > >> [juergh: Backported to 3.13: >> - Drop changes to ceph >> - Use capable() instead of capable_wrt_inode_uidgid() >> - Update generic_acl.c as well >> - In gfs2, jfs, and xfs, take care to avoid leaking the allocated ACL if >> posix_acl_update_mode() determines it's not needed] >> Signed-off-by: Juerg Haefliger <juerg.haefliger@canonical.com> >> --- >> fs/9p/acl.c | 40 +++++++++++++++++----------------------- >> fs/btrfs/acl.c | 6 ++---- >> fs/ext2/acl.c | 12 ++++-------- >> fs/ext3/acl.c | 12 ++++-------- >> fs/ext4/acl.c | 12 ++++-------- >> fs/f2fs/acl.c | 6 ++---- >> fs/generic_acl.c | 15 ++++++++------- >> fs/gfs2/acl.c | 16 +++++++--------- >> fs/hfsplus/posix_acl.c | 4 ++-- >> fs/jffs2/acl.c | 9 ++++----- >> fs/jfs/xattr.c | 6 ++++-- >> fs/ocfs2/acl.c | 9 +++------ >> fs/posix_acl.c | 30 ++++++++++++++++++++++++++++++ >> fs/reiserfs/xattr_acl.c | 8 ++------ >> fs/xfs/xfs_acl.c | 17 +++++++---------- >> include/linux/posix_acl.h | 1 + >> 16 files changed, 101 insertions(+), 102 deletions(-) >> >> diff --git a/fs/9p/acl.c b/fs/9p/acl.c >> index 7af425f53bee..9686c1f17653 100644 >> --- a/fs/9p/acl.c >> +++ b/fs/9p/acl.c >> @@ -320,32 +320,26 @@ static int v9fs_xattr_set_acl(struct dentry *dentry, const char *name, >> case ACL_TYPE_ACCESS: >> name = POSIX_ACL_XATTR_ACCESS; >> if (acl) { >> - umode_t mode = inode->i_mode; >> - retval = posix_acl_equiv_mode(acl, &mode); >> - if (retval < 0) >> + struct iattr iattr; >> + >> + retval = posix_acl_update_mode(inode, &iattr.ia_mode, &acl); >> + if (retval) >> goto err_out; >> - else { >> - struct iattr iattr; >> - if (retval == 0) { >> - /* >> - * ACL can be represented >> - * by the mode bits. So don't >> - * update ACL. >> - */ >> - acl = NULL; >> - value = NULL; >> - size = 0; >> - } >> - /* Updte the mode bits */ >> - iattr.ia_mode = ((mode & S_IALLUGO) | >> - (inode->i_mode & ~S_IALLUGO)); >> - iattr.ia_valid = ATTR_MODE; >> - /* FIXME should we update ctime ? >> - * What is the following setxattr update the >> - * mode ? >> + if (!acl) { >> + /* >> + * ACL can be represented >> + * by the mode bits. So don't >> + * update ACL. >> */ >> - v9fs_vfs_setattr_dotl(dentry, &iattr); >> + value = NULL; >> + size = 0; >> } >> + iattr.ia_valid = ATTR_MODE; >> + /* FIXME should we update ctime ? >> + * What is the following setxattr update the >> + * mode ? >> + */ >> + v9fs_vfs_setattr_dotl(dentry, &iattr); >> } >> break; >> case ACL_TYPE_DEFAULT: >> diff --git a/fs/btrfs/acl.c b/fs/btrfs/acl.c >> index 0890c83643e9..d6d53e5e7945 100644 >> --- a/fs/btrfs/acl.c >> +++ b/fs/btrfs/acl.c >> @@ -118,11 +118,9 @@ static int btrfs_set_acl(struct btrfs_trans_handle *trans, >> case ACL_TYPE_ACCESS: >> name = POSIX_ACL_XATTR_ACCESS; >> if (acl) { >> - ret = posix_acl_equiv_mode(acl, &inode->i_mode); >> - if (ret < 0) >> + ret = posix_acl_update_mode(inode, &inode->i_mode, &acl); >> + if (ret) >> return ret; >> - if (ret == 0) >> - acl = NULL; >> } >> ret = 0; >> break; >> diff --git a/fs/ext2/acl.c b/fs/ext2/acl.c >> index 110b6b371a4e..48c3c2d7d261 100644 >> --- a/fs/ext2/acl.c >> +++ b/fs/ext2/acl.c >> @@ -206,15 +206,11 @@ ext2_set_acl(struct inode *inode, int type, struct posix_acl *acl) >> case ACL_TYPE_ACCESS: >> name_index = EXT2_XATTR_INDEX_POSIX_ACL_ACCESS; >> if (acl) { >> - error = posix_acl_equiv_mode(acl, &inode->i_mode); >> - if (error < 0) >> + error = posix_acl_update_mode(inode, &inode->i_mode, &acl); >> + if (error) >> return error; >> - else { >> - inode->i_ctime = CURRENT_TIME_SEC; >> - mark_inode_dirty(inode); >> - if (error == 0) >> - acl = NULL; >> - } >> + inode->i_ctime = CURRENT_TIME_SEC; >> + mark_inode_dirty(inode); >> } >> break; >> >> diff --git a/fs/ext3/acl.c b/fs/ext3/acl.c >> index dbb5ad59a7fc..bb2f60a62d82 100644 >> --- a/fs/ext3/acl.c >> +++ b/fs/ext3/acl.c >> @@ -205,15 +205,11 @@ ext3_set_acl(handle_t *handle, struct inode *inode, int type, >> case ACL_TYPE_ACCESS: >> name_index = EXT3_XATTR_INDEX_POSIX_ACL_ACCESS; >> if (acl) { >> - error = posix_acl_equiv_mode(acl, &inode->i_mode); >> - if (error < 0) >> + error = posix_acl_update_mode(inode, &inode->i_mode, &acl); >> + if (error) >> return error; >> - else { >> - inode->i_ctime = CURRENT_TIME_SEC; >> - ext3_mark_inode_dirty(handle, inode); >> - if (error == 0) >> - acl = NULL; >> - } >> + inode->i_ctime = CURRENT_TIME_SEC; >> + ext3_mark_inode_dirty(handle, inode); >> } >> break; >> >> diff --git a/fs/ext4/acl.c b/fs/ext4/acl.c >> index 39a54a0e9fe4..c844f1bfb451 100644 >> --- a/fs/ext4/acl.c >> +++ b/fs/ext4/acl.c >> @@ -211,15 +211,11 @@ ext4_set_acl(handle_t *handle, struct inode *inode, int type, >> case ACL_TYPE_ACCESS: >> name_index = EXT4_XATTR_INDEX_POSIX_ACL_ACCESS; >> if (acl) { >> - error = posix_acl_equiv_mode(acl, &inode->i_mode); >> - if (error < 0) >> + error = posix_acl_update_mode(inode, &inode->i_mode, &acl); >> + if (error) >> return error; >> - else { >> - inode->i_ctime = ext4_current_time(inode); >> - ext4_mark_inode_dirty(handle, inode); >> - if (error == 0) >> - acl = NULL; >> - } >> + inode->i_ctime = ext4_current_time(inode); >> + ext4_mark_inode_dirty(handle, inode); >> } >> break; >> >> diff --git a/fs/f2fs/acl.c b/fs/f2fs/acl.c >> index d0fc287efeff..0eb2d66827ad 100644 >> --- a/fs/f2fs/acl.c >> +++ b/fs/f2fs/acl.c >> @@ -224,12 +224,10 @@ static int f2fs_set_acl(struct inode *inode, int type, >> case ACL_TYPE_ACCESS: >> name_index = F2FS_XATTR_INDEX_POSIX_ACL_ACCESS; >> if (acl) { >> - error = posix_acl_equiv_mode(acl, &inode->i_mode); >> - if (error < 0) >> + error = posix_acl_update_mode(inode, &inode->i_mode, &acl); >> + if (error) >> return error; >> set_acl_inode(fi, inode->i_mode); >> - if (error == 0) >> - acl = NULL; >> } >> break; >> >> diff --git a/fs/generic_acl.c b/fs/generic_acl.c >> index b3f3676796d3..67319f168b42 100644 >> --- a/fs/generic_acl.c >> +++ b/fs/generic_acl.c >> @@ -86,16 +86,17 @@ generic_acl_set(struct dentry *dentry, const char *name, const void *value, >> if (error) >> goto failed; >> switch (type) { >> - case ACL_TYPE_ACCESS: >> - error = posix_acl_equiv_mode(acl, &inode->i_mode); >> - if (error < 0) >> + case ACL_TYPE_ACCESS: { >> + struct posix_acl *saved_acl = acl; >> + >> + error = posix_acl_update_mode(inode, &inode->i_mode, &acl); >> + if (acl == NULL) >> + posix_acl_release(saved_acl); >> + if (error) >> goto failed; >> inode->i_ctime = CURRENT_TIME; >> - if (error == 0) { >> - posix_acl_release(acl); >> - acl = NULL; >> - } >> break; >> + } >> case ACL_TYPE_DEFAULT: >> if (!S_ISDIR(inode->i_mode)) { >> error = -EINVAL; >> diff --git a/fs/gfs2/acl.c b/fs/gfs2/acl.c >> index f69ac0af5496..015809a066b5 100644 >> --- a/fs/gfs2/acl.c >> +++ b/fs/gfs2/acl.c >> @@ -267,16 +267,14 @@ static int gfs2_xattr_system_set(struct dentry *dentry, const char *name, >> goto out_release; >> >> if (type == ACL_TYPE_ACCESS) { >> - umode_t mode = inode->i_mode; >> - error = posix_acl_equiv_mode(acl, &mode); >> + struct posix_acl *saved_acl = acl; >> + umode_t mode; >> >> - if (error <= 0) { >> - posix_acl_release(acl); >> - acl = NULL; >> - >> - if (error < 0) >> - return error; >> - } >> + error = posix_acl_update_mode(inode, &mode, &acl); >> + if (error || acl == NULL) >> + posix_acl_release(saved_acl); >> + if (error) >> + return error; >> >> error = gfs2_set_mode(inode, mode); >> if (error) >> diff --git a/fs/hfsplus/posix_acl.c b/fs/hfsplus/posix_acl.c >> index b609cc14c72e..9f7cc491ffb1 100644 >> --- a/fs/hfsplus/posix_acl.c >> +++ b/fs/hfsplus/posix_acl.c >> @@ -72,8 +72,8 @@ static int hfsplus_set_posix_acl(struct inode *inode, >> case ACL_TYPE_ACCESS: >> xattr_name = POSIX_ACL_XATTR_ACCESS; >> if (acl) { >> - err = posix_acl_equiv_mode(acl, &inode->i_mode); >> - if (err < 0) >> + err = posix_acl_update_mode(inode, &inode->i_mode, &acl); >> + if (err) >> return err; >> } >> err = 0; >> diff --git a/fs/jffs2/acl.c b/fs/jffs2/acl.c >> index 223283c30111..9335b8d3cf52 100644 >> --- a/fs/jffs2/acl.c >> +++ b/fs/jffs2/acl.c >> @@ -243,9 +243,10 @@ static int jffs2_set_acl(struct inode *inode, int type, struct posix_acl *acl) >> case ACL_TYPE_ACCESS: >> xprefix = JFFS2_XPREFIX_ACL_ACCESS; >> if (acl) { >> - umode_t mode = inode->i_mode; >> - rc = posix_acl_equiv_mode(acl, &mode); >> - if (rc < 0) >> + umode_t mode; >> + >> + rc = posix_acl_update_mode(inode, &mode, &acl); >> + if (rc) >> return rc; >> if (inode->i_mode != mode) { >> struct iattr attr; >> @@ -257,8 +258,6 @@ static int jffs2_set_acl(struct inode *inode, int type, struct posix_acl *acl) >> if (rc < 0) >> return rc; >> } >> - if (rc == 0) >> - acl = NULL; >> } >> break; >> case ACL_TYPE_DEFAULT: >> diff --git a/fs/jfs/xattr.c b/fs/jfs/xattr.c >> index d3472f4cd530..6910662a8bf5 100644 >> --- a/fs/jfs/xattr.c >> +++ b/fs/jfs/xattr.c >> @@ -693,9 +693,11 @@ static int can_set_system_xattr(struct inode *inode, const char *name, >> return rc; >> } >> if (acl) { >> - rc = posix_acl_equiv_mode(acl, &inode->i_mode); >> + struct posix_acl *dummy = acl; >> + >> + rc = posix_acl_update_mode(inode, &inode->i_mode, &dummy); >> posix_acl_release(acl); >> - if (rc < 0) { >> + if (rc) { >> printk(KERN_ERR >> "posix_acl_equiv_mode returned %d\n", >> rc); >> diff --git a/fs/ocfs2/acl.c b/fs/ocfs2/acl.c >> index b4f788e0ca31..b16bb5c70bc8 100644 >> --- a/fs/ocfs2/acl.c >> +++ b/fs/ocfs2/acl.c >> @@ -270,14 +270,11 @@ static int ocfs2_set_acl(handle_t *handle, >> case ACL_TYPE_ACCESS: >> name_index = OCFS2_XATTR_INDEX_POSIX_ACL_ACCESS; >> if (acl) { >> - umode_t mode = inode->i_mode; >> - ret = posix_acl_equiv_mode(acl, &mode); >> - if (ret < 0) >> + umode_t mode; >> + ret = posix_acl_update_mode(inode, &mode, &acl); >> + if (ret) >> return ret; >> else { >> - if (ret == 0) >> - acl = NULL; >> - >> ret = ocfs2_acl_set_mode(inode, di_bh, >> handle, mode); >> if (ret) >> diff --git a/fs/posix_acl.c b/fs/posix_acl.c >> index 3542f1f814e2..8161e5c9dc31 100644 >> --- a/fs/posix_acl.c >> +++ b/fs/posix_acl.c >> @@ -407,6 +407,36 @@ posix_acl_create(struct posix_acl **acl, gfp_t gfp, umode_t *mode_p) >> } >> EXPORT_SYMBOL(posix_acl_create); >> >> +/** >> + * posix_acl_update_mode - update mode in set_acl >> + * >> + * Update the file mode when setting an ACL: compute the new file permission >> + * bits based on the ACL. In addition, if the ACL is equivalent to the new >> + * file mode, set *acl to NULL to indicate that no ACL should be set. >> + * >> + * As with chmod, clear the setgit bit if the caller is not in the owning group >> + * or capable of CAP_FSETID (see inode_change_ok). >> + * >> + * Called from set_acl inode operations. >> + */ >> +int posix_acl_update_mode(struct inode *inode, umode_t *mode_p, >> + struct posix_acl **acl) >> +{ >> + umode_t mode = inode->i_mode; >> + int error; >> + >> + error = posix_acl_equiv_mode(*acl, &mode); >> + if (error < 0) >> + return error; >> + if (error == 0) >> + *acl = NULL; >> + if (!in_group_p(inode->i_gid) && !capable(CAP_FSETID)) >> + mode &= ~S_ISGID; >> + *mode_p = mode; >> + return 0; >> +} >> +EXPORT_SYMBOL(posix_acl_update_mode); >> + >> int >> posix_acl_chmod(struct posix_acl **acl, gfp_t gfp, umode_t mode) >> { >> diff --git a/fs/reiserfs/xattr_acl.c b/fs/reiserfs/xattr_acl.c >> index 06c04f73da65..a86ad7ec7957 100644 >> --- a/fs/reiserfs/xattr_acl.c >> +++ b/fs/reiserfs/xattr_acl.c >> @@ -288,13 +288,9 @@ reiserfs_set_acl(struct reiserfs_transaction_handle *th, struct inode *inode, >> case ACL_TYPE_ACCESS: >> name = POSIX_ACL_XATTR_ACCESS; >> if (acl) { >> - error = posix_acl_equiv_mode(acl, &inode->i_mode); >> - if (error < 0) >> + error = posix_acl_update_mode(inode, &inode->i_mode, &acl); >> + if (error) >> return error; >> - else { >> - if (error == 0) >> - acl = NULL; >> - } >> } >> break; >> case ACL_TYPE_DEFAULT: >> diff --git a/fs/xfs/xfs_acl.c b/fs/xfs/xfs_acl.c >> index 370eb3e121d1..89ac0522b38d 100644 >> --- a/fs/xfs/xfs_acl.c >> +++ b/fs/xfs/xfs_acl.c >> @@ -402,17 +402,14 @@ xfs_xattr_acl_set(struct dentry *dentry, const char *name, >> goto out_release; >> >> if (type == ACL_TYPE_ACCESS) { >> - umode_t mode = inode->i_mode; >> - error = posix_acl_equiv_mode(acl, &mode); >> - >> - if (error <= 0) { >> - posix_acl_release(acl); >> - acl = NULL; >> - >> - if (error < 0) >> - return error; >> - } >> + struct posix_acl *saved_acl = acl; >> + umode_t mode; >> >> + error = posix_acl_update_mode(inode, &mode, &acl); >> + if (error || acl == NULL) >> + posix_acl_release(saved_acl); >> + if (error) >> + return error; >> error = xfs_set_mode(inode, mode); >> if (error) >> goto out_release; >> diff --git a/include/linux/posix_acl.h b/include/linux/posix_acl.h >> index 7931efe71175..2ae0bba45f12 100644 >> --- a/include/linux/posix_acl.h >> +++ b/include/linux/posix_acl.h >> @@ -90,6 +90,7 @@ extern struct posix_acl *posix_acl_from_mode(umode_t, gfp_t); >> extern int posix_acl_equiv_mode(const struct posix_acl *, umode_t *); >> extern int posix_acl_create(struct posix_acl **, gfp_t, umode_t *); >> extern int posix_acl_chmod(struct posix_acl **, gfp_t, umode_t); >> +extern int posix_acl_update_mode(struct inode *, umode_t *, struct posix_acl **); >> >> extern struct posix_acl *get_posix_acl(struct inode *, int); >> extern int set_posix_acl(struct inode *, int, struct posix_acl *); >>
On Wed, Sep 06, 2017 at 10:54:53AM +0200, Juerg Haefliger wrote: > From: Jan Kara <jack@suse.cz> > > commit 073931017b49d9458aa351605b43a7e34598caef upstream. > > When file permissions are modified via chmod(2) and the user is not in > the owning group or capable of CAP_FSETID, the setgid bit is cleared in > inode_change_ok(). Setting a POSIX ACL via setxattr(2) sets the file > permissions as well as the new ACL, but doesn't clear the setgid bit in > a similar way; this allows to bypass the check in chmod(2). Fix that. > > References: CVE-2016-7097 > Reviewed-by: Christoph Hellwig <hch@lst.de> > Reviewed-by: Jeff Layton <jlayton@redhat.com> > Signed-off-by: Jan Kara <jack@suse.cz> > Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com> > [bwh: Backported to 3.16: > - Drop changes to orangefs > - Adjust context > - Update ext3 as well] > Signed-off-by: Ben Hutchings <ben@decadent.org.uk> > > CVE-2016-7097 > > [juergh: Backported to 3.13: > - Drop changes to ceph > - Use capable() instead of capable_wrt_inode_uidgid() We have capable_wrt_inode_uidgid in trusty. Why didn't you use it? Cascardo.
On 09/06/2017 03:40 PM, Thadeu Lima de Souza Cascardo wrote: > On Wed, Sep 06, 2017 at 10:54:53AM +0200, Juerg Haefliger wrote: >> From: Jan Kara <jack@suse.cz> >> >> commit 073931017b49d9458aa351605b43a7e34598caef upstream. >> >> When file permissions are modified via chmod(2) and the user is not in >> the owning group or capable of CAP_FSETID, the setgid bit is cleared in >> inode_change_ok(). Setting a POSIX ACL via setxattr(2) sets the file >> permissions as well as the new ACL, but doesn't clear the setgid bit in >> a similar way; this allows to bypass the check in chmod(2). Fix that. >> >> References: CVE-2016-7097 >> Reviewed-by: Christoph Hellwig <hch@lst.de> >> Reviewed-by: Jeff Layton <jlayton@redhat.com> >> Signed-off-by: Jan Kara <jack@suse.cz> >> Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com> >> [bwh: Backported to 3.16: >> - Drop changes to orangefs >> - Adjust context >> - Update ext3 as well] >> Signed-off-by: Ben Hutchings <ben@decadent.org.uk> >> >> CVE-2016-7097 >> >> [juergh: Backported to 3.13: >> - Drop changes to ceph >> - Use capable() instead of capable_wrt_inode_uidgid() > > We have capable_wrt_inode_uidgid in trusty. Why didn't you use it? Because I was looking at upstream 3.13 and not trusty 3.13. Duh. ...Juerg > Cascardo. >
diff --git a/fs/9p/acl.c b/fs/9p/acl.c index 7af425f53bee..9686c1f17653 100644 --- a/fs/9p/acl.c +++ b/fs/9p/acl.c @@ -320,32 +320,26 @@ static int v9fs_xattr_set_acl(struct dentry *dentry, const char *name, case ACL_TYPE_ACCESS: name = POSIX_ACL_XATTR_ACCESS; if (acl) { - umode_t mode = inode->i_mode; - retval = posix_acl_equiv_mode(acl, &mode); - if (retval < 0) + struct iattr iattr; + + retval = posix_acl_update_mode(inode, &iattr.ia_mode, &acl); + if (retval) goto err_out; - else { - struct iattr iattr; - if (retval == 0) { - /* - * ACL can be represented - * by the mode bits. So don't - * update ACL. - */ - acl = NULL; - value = NULL; - size = 0; - } - /* Updte the mode bits */ - iattr.ia_mode = ((mode & S_IALLUGO) | - (inode->i_mode & ~S_IALLUGO)); - iattr.ia_valid = ATTR_MODE; - /* FIXME should we update ctime ? - * What is the following setxattr update the - * mode ? + if (!acl) { + /* + * ACL can be represented + * by the mode bits. So don't + * update ACL. */ - v9fs_vfs_setattr_dotl(dentry, &iattr); + value = NULL; + size = 0; } + iattr.ia_valid = ATTR_MODE; + /* FIXME should we update ctime ? + * What is the following setxattr update the + * mode ? + */ + v9fs_vfs_setattr_dotl(dentry, &iattr); } break; case ACL_TYPE_DEFAULT: diff --git a/fs/btrfs/acl.c b/fs/btrfs/acl.c index 0890c83643e9..d6d53e5e7945 100644 --- a/fs/btrfs/acl.c +++ b/fs/btrfs/acl.c @@ -118,11 +118,9 @@ static int btrfs_set_acl(struct btrfs_trans_handle *trans, case ACL_TYPE_ACCESS: name = POSIX_ACL_XATTR_ACCESS; if (acl) { - ret = posix_acl_equiv_mode(acl, &inode->i_mode); - if (ret < 0) + ret = posix_acl_update_mode(inode, &inode->i_mode, &acl); + if (ret) return ret; - if (ret == 0) - acl = NULL; } ret = 0; break; diff --git a/fs/ext2/acl.c b/fs/ext2/acl.c index 110b6b371a4e..48c3c2d7d261 100644 --- a/fs/ext2/acl.c +++ b/fs/ext2/acl.c @@ -206,15 +206,11 @@ ext2_set_acl(struct inode *inode, int type, struct posix_acl *acl) case ACL_TYPE_ACCESS: name_index = EXT2_XATTR_INDEX_POSIX_ACL_ACCESS; if (acl) { - error = posix_acl_equiv_mode(acl, &inode->i_mode); - if (error < 0) + error = posix_acl_update_mode(inode, &inode->i_mode, &acl); + if (error) return error; - else { - inode->i_ctime = CURRENT_TIME_SEC; - mark_inode_dirty(inode); - if (error == 0) - acl = NULL; - } + inode->i_ctime = CURRENT_TIME_SEC; + mark_inode_dirty(inode); } break; diff --git a/fs/ext3/acl.c b/fs/ext3/acl.c index dbb5ad59a7fc..bb2f60a62d82 100644 --- a/fs/ext3/acl.c +++ b/fs/ext3/acl.c @@ -205,15 +205,11 @@ ext3_set_acl(handle_t *handle, struct inode *inode, int type, case ACL_TYPE_ACCESS: name_index = EXT3_XATTR_INDEX_POSIX_ACL_ACCESS; if (acl) { - error = posix_acl_equiv_mode(acl, &inode->i_mode); - if (error < 0) + error = posix_acl_update_mode(inode, &inode->i_mode, &acl); + if (error) return error; - else { - inode->i_ctime = CURRENT_TIME_SEC; - ext3_mark_inode_dirty(handle, inode); - if (error == 0) - acl = NULL; - } + inode->i_ctime = CURRENT_TIME_SEC; + ext3_mark_inode_dirty(handle, inode); } break; diff --git a/fs/ext4/acl.c b/fs/ext4/acl.c index 39a54a0e9fe4..c844f1bfb451 100644 --- a/fs/ext4/acl.c +++ b/fs/ext4/acl.c @@ -211,15 +211,11 @@ ext4_set_acl(handle_t *handle, struct inode *inode, int type, case ACL_TYPE_ACCESS: name_index = EXT4_XATTR_INDEX_POSIX_ACL_ACCESS; if (acl) { - error = posix_acl_equiv_mode(acl, &inode->i_mode); - if (error < 0) + error = posix_acl_update_mode(inode, &inode->i_mode, &acl); + if (error) return error; - else { - inode->i_ctime = ext4_current_time(inode); - ext4_mark_inode_dirty(handle, inode); - if (error == 0) - acl = NULL; - } + inode->i_ctime = ext4_current_time(inode); + ext4_mark_inode_dirty(handle, inode); } break; diff --git a/fs/f2fs/acl.c b/fs/f2fs/acl.c index d0fc287efeff..0eb2d66827ad 100644 --- a/fs/f2fs/acl.c +++ b/fs/f2fs/acl.c @@ -224,12 +224,10 @@ static int f2fs_set_acl(struct inode *inode, int type, case ACL_TYPE_ACCESS: name_index = F2FS_XATTR_INDEX_POSIX_ACL_ACCESS; if (acl) { - error = posix_acl_equiv_mode(acl, &inode->i_mode); - if (error < 0) + error = posix_acl_update_mode(inode, &inode->i_mode, &acl); + if (error) return error; set_acl_inode(fi, inode->i_mode); - if (error == 0) - acl = NULL; } break; diff --git a/fs/generic_acl.c b/fs/generic_acl.c index b3f3676796d3..67319f168b42 100644 --- a/fs/generic_acl.c +++ b/fs/generic_acl.c @@ -86,16 +86,17 @@ generic_acl_set(struct dentry *dentry, const char *name, const void *value, if (error) goto failed; switch (type) { - case ACL_TYPE_ACCESS: - error = posix_acl_equiv_mode(acl, &inode->i_mode); - if (error < 0) + case ACL_TYPE_ACCESS: { + struct posix_acl *saved_acl = acl; + + error = posix_acl_update_mode(inode, &inode->i_mode, &acl); + if (acl == NULL) + posix_acl_release(saved_acl); + if (error) goto failed; inode->i_ctime = CURRENT_TIME; - if (error == 0) { - posix_acl_release(acl); - acl = NULL; - } break; + } case ACL_TYPE_DEFAULT: if (!S_ISDIR(inode->i_mode)) { error = -EINVAL; diff --git a/fs/gfs2/acl.c b/fs/gfs2/acl.c index f69ac0af5496..015809a066b5 100644 --- a/fs/gfs2/acl.c +++ b/fs/gfs2/acl.c @@ -267,16 +267,14 @@ static int gfs2_xattr_system_set(struct dentry *dentry, const char *name, goto out_release; if (type == ACL_TYPE_ACCESS) { - umode_t mode = inode->i_mode; - error = posix_acl_equiv_mode(acl, &mode); + struct posix_acl *saved_acl = acl; + umode_t mode; - if (error <= 0) { - posix_acl_release(acl); - acl = NULL; - - if (error < 0) - return error; - } + error = posix_acl_update_mode(inode, &mode, &acl); + if (error || acl == NULL) + posix_acl_release(saved_acl); + if (error) + return error; error = gfs2_set_mode(inode, mode); if (error) diff --git a/fs/hfsplus/posix_acl.c b/fs/hfsplus/posix_acl.c index b609cc14c72e..9f7cc491ffb1 100644 --- a/fs/hfsplus/posix_acl.c +++ b/fs/hfsplus/posix_acl.c @@ -72,8 +72,8 @@ static int hfsplus_set_posix_acl(struct inode *inode, case ACL_TYPE_ACCESS: xattr_name = POSIX_ACL_XATTR_ACCESS; if (acl) { - err = posix_acl_equiv_mode(acl, &inode->i_mode); - if (err < 0) + err = posix_acl_update_mode(inode, &inode->i_mode, &acl); + if (err) return err; } err = 0; diff --git a/fs/jffs2/acl.c b/fs/jffs2/acl.c index 223283c30111..9335b8d3cf52 100644 --- a/fs/jffs2/acl.c +++ b/fs/jffs2/acl.c @@ -243,9 +243,10 @@ static int jffs2_set_acl(struct inode *inode, int type, struct posix_acl *acl) case ACL_TYPE_ACCESS: xprefix = JFFS2_XPREFIX_ACL_ACCESS; if (acl) { - umode_t mode = inode->i_mode; - rc = posix_acl_equiv_mode(acl, &mode); - if (rc < 0) + umode_t mode; + + rc = posix_acl_update_mode(inode, &mode, &acl); + if (rc) return rc; if (inode->i_mode != mode) { struct iattr attr; @@ -257,8 +258,6 @@ static int jffs2_set_acl(struct inode *inode, int type, struct posix_acl *acl) if (rc < 0) return rc; } - if (rc == 0) - acl = NULL; } break; case ACL_TYPE_DEFAULT: diff --git a/fs/jfs/xattr.c b/fs/jfs/xattr.c index d3472f4cd530..6910662a8bf5 100644 --- a/fs/jfs/xattr.c +++ b/fs/jfs/xattr.c @@ -693,9 +693,11 @@ static int can_set_system_xattr(struct inode *inode, const char *name, return rc; } if (acl) { - rc = posix_acl_equiv_mode(acl, &inode->i_mode); + struct posix_acl *dummy = acl; + + rc = posix_acl_update_mode(inode, &inode->i_mode, &dummy); posix_acl_release(acl); - if (rc < 0) { + if (rc) { printk(KERN_ERR "posix_acl_equiv_mode returned %d\n", rc); diff --git a/fs/ocfs2/acl.c b/fs/ocfs2/acl.c index b4f788e0ca31..b16bb5c70bc8 100644 --- a/fs/ocfs2/acl.c +++ b/fs/ocfs2/acl.c @@ -270,14 +270,11 @@ static int ocfs2_set_acl(handle_t *handle, case ACL_TYPE_ACCESS: name_index = OCFS2_XATTR_INDEX_POSIX_ACL_ACCESS; if (acl) { - umode_t mode = inode->i_mode; - ret = posix_acl_equiv_mode(acl, &mode); - if (ret < 0) + umode_t mode; + ret = posix_acl_update_mode(inode, &mode, &acl); + if (ret) return ret; else { - if (ret == 0) - acl = NULL; - ret = ocfs2_acl_set_mode(inode, di_bh, handle, mode); if (ret) diff --git a/fs/posix_acl.c b/fs/posix_acl.c index 3542f1f814e2..8161e5c9dc31 100644 --- a/fs/posix_acl.c +++ b/fs/posix_acl.c @@ -407,6 +407,36 @@ posix_acl_create(struct posix_acl **acl, gfp_t gfp, umode_t *mode_p) } EXPORT_SYMBOL(posix_acl_create); +/** + * posix_acl_update_mode - update mode in set_acl + * + * Update the file mode when setting an ACL: compute the new file permission + * bits based on the ACL. In addition, if the ACL is equivalent to the new + * file mode, set *acl to NULL to indicate that no ACL should be set. + * + * As with chmod, clear the setgit bit if the caller is not in the owning group + * or capable of CAP_FSETID (see inode_change_ok). + * + * Called from set_acl inode operations. + */ +int posix_acl_update_mode(struct inode *inode, umode_t *mode_p, + struct posix_acl **acl) +{ + umode_t mode = inode->i_mode; + int error; + + error = posix_acl_equiv_mode(*acl, &mode); + if (error < 0) + return error; + if (error == 0) + *acl = NULL; + if (!in_group_p(inode->i_gid) && !capable(CAP_FSETID)) + mode &= ~S_ISGID; + *mode_p = mode; + return 0; +} +EXPORT_SYMBOL(posix_acl_update_mode); + int posix_acl_chmod(struct posix_acl **acl, gfp_t gfp, umode_t mode) { diff --git a/fs/reiserfs/xattr_acl.c b/fs/reiserfs/xattr_acl.c index 06c04f73da65..a86ad7ec7957 100644 --- a/fs/reiserfs/xattr_acl.c +++ b/fs/reiserfs/xattr_acl.c @@ -288,13 +288,9 @@ reiserfs_set_acl(struct reiserfs_transaction_handle *th, struct inode *inode, case ACL_TYPE_ACCESS: name = POSIX_ACL_XATTR_ACCESS; if (acl) { - error = posix_acl_equiv_mode(acl, &inode->i_mode); - if (error < 0) + error = posix_acl_update_mode(inode, &inode->i_mode, &acl); + if (error) return error; - else { - if (error == 0) - acl = NULL; - } } break; case ACL_TYPE_DEFAULT: diff --git a/fs/xfs/xfs_acl.c b/fs/xfs/xfs_acl.c index 370eb3e121d1..89ac0522b38d 100644 --- a/fs/xfs/xfs_acl.c +++ b/fs/xfs/xfs_acl.c @@ -402,17 +402,14 @@ xfs_xattr_acl_set(struct dentry *dentry, const char *name, goto out_release; if (type == ACL_TYPE_ACCESS) { - umode_t mode = inode->i_mode; - error = posix_acl_equiv_mode(acl, &mode); - - if (error <= 0) { - posix_acl_release(acl); - acl = NULL; - - if (error < 0) - return error; - } + struct posix_acl *saved_acl = acl; + umode_t mode; + error = posix_acl_update_mode(inode, &mode, &acl); + if (error || acl == NULL) + posix_acl_release(saved_acl); + if (error) + return error; error = xfs_set_mode(inode, mode); if (error) goto out_release; diff --git a/include/linux/posix_acl.h b/include/linux/posix_acl.h index 7931efe71175..2ae0bba45f12 100644 --- a/include/linux/posix_acl.h +++ b/include/linux/posix_acl.h @@ -90,6 +90,7 @@ extern struct posix_acl *posix_acl_from_mode(umode_t, gfp_t); extern int posix_acl_equiv_mode(const struct posix_acl *, umode_t *); extern int posix_acl_create(struct posix_acl **, gfp_t, umode_t *); extern int posix_acl_chmod(struct posix_acl **, gfp_t, umode_t); +extern int posix_acl_update_mode(struct inode *, umode_t *, struct posix_acl **); extern struct posix_acl *get_posix_acl(struct inode *, int); extern int set_posix_acl(struct inode *, int, struct posix_acl *);