From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934359AbdA0LcG (ORCPT ); Fri, 27 Jan 2017 06:32:06 -0500 Received: from mx2.suse.de ([195.135.220.15]:36794 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933731AbdA0LNL (ORCPT ); Fri, 27 Jan 2017 06:13:11 -0500 X-Amavis-Alert: BAD HEADER SECTION, Duplicate header field: "References" From: Jiri Slaby To: stable@vger.kernel.org Cc: linux-kernel@vger.kernel.org, Jan Kara , Andreas Gruenbacher , Jiri Slaby Subject: [PATCH 3.12 194/235] posix_acl: Clear SGID bit when setting file permissions Date: Fri, 27 Jan 2017 11:55:27 +0100 Message-Id: X-Mailer: git-send-email 2.11.0 In-Reply-To: <5b46dc789ca2be4046e4e40a131858d386cac741.1485514374.git.jslaby@suse.cz> References: <5b46dc789ca2be4046e4e40a131858d386cac741.1485514374.git.jslaby@suse.cz> In-Reply-To: References: Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org From: Jan Kara 3.12-stable review patch. If anyone has any objections, please let me know. =============== 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. Reviewed-by: Christoph Hellwig Reviewed-by: Jeff Layton Signed-off-by: Jan Kara Signed-off-by: Andreas Gruenbacher Signed-off-by: Jiri Slaby --- fs/9p/acl.c | 40 +++++++++++++++++----------------------- fs/btrfs/acl.c | 6 ++---- fs/ext2/acl.c | 12 ++++-------- fs/ext3/acl.c | 10 +++------- fs/ext4/acl.c | 12 ++++-------- fs/f2fs/acl.c | 6 ++---- fs/gfs2/acl.c | 14 ++++++-------- fs/hfsplus/posix_acl.c | 4 ++-- fs/hfsplus/xattr.c | 5 +++-- fs/jffs2/acl.c | 9 ++++----- fs/jfs/xattr.c | 5 +++-- fs/ocfs2/acl.c | 20 +++++++------------- fs/posix_acl.c | 31 +++++++++++++++++++++++++++++++ fs/reiserfs/xattr_acl.c | 8 ++------ fs/xfs/xfs_acl.c | 15 +++++++-------- include/linux/posix_acl.h | 1 + 16 files changed, 98 insertions(+), 100 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..2f994bbf73a7 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); + error = posix_acl_update_mode(inode, &inode->i_mode, &acl); if (error < 0) 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 b7826ec1b470..f4fefc57ff56 100644 --- a/fs/f2fs/acl.c +++ b/fs/f2fs/acl.c @@ -223,12 +223,10 @@ static int f2fs_set_acl(struct inode *inode, int type, struct posix_acl *acl) 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/gfs2/acl.c b/fs/gfs2/acl.c index f69ac0af5496..a61b0c2b57ab 100644 --- a/fs/gfs2/acl.c +++ b/fs/gfs2/acl.c @@ -268,15 +268,13 @@ static int gfs2_xattr_system_set(struct dentry *dentry, const char *name, if (type == ACL_TYPE_ACCESS) { umode_t mode = inode->i_mode; - error = posix_acl_equiv_mode(acl, &mode); + struct posix_acl *old_acl = acl; - if (error <= 0) { - posix_acl_release(acl); - acl = NULL; - - if (error < 0) - return error; - } + error = posix_acl_update_mode(inode, &mode, &acl); + if (error < 0) + goto out_release; + if (!acl) + posix_acl_release(old_acl); 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/hfsplus/xattr.c b/fs/hfsplus/xattr.c index bd8471fb9a6a..889be3fef4bc 100644 --- a/fs/hfsplus/xattr.c +++ b/fs/hfsplus/xattr.c @@ -69,8 +69,9 @@ static int can_set_system_xattr(struct inode *inode, const char *name, if (IS_ERR(acl)) return PTR_ERR(acl); if (acl) { - err = posix_acl_equiv_mode(acl, &inode->i_mode); - posix_acl_release(acl); + struct posix_acl *old_acl = acl; + err = posix_acl_update_mode(inode, &inode->i_mode, &acl); + posix_acl_release(old_acl); if (err < 0) return err; mark_inode_dirty(inode); 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..8c9b6a06dcbb 100644 --- a/fs/jfs/xattr.c +++ b/fs/jfs/xattr.c @@ -693,8 +693,9 @@ 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); - posix_acl_release(acl); + struct posix_acl *old_acl = acl; + rc = posix_acl_update_mode(inode, &inode->i_mode, &acl); + posix_acl_release(old_acl); if (rc < 0) { printk(KERN_ERR "posix_acl_equiv_mode returned %d\n", diff --git a/fs/ocfs2/acl.c b/fs/ocfs2/acl.c index b4f788e0ca31..23095b017752 100644 --- a/fs/ocfs2/acl.c +++ b/fs/ocfs2/acl.c @@ -270,20 +270,14 @@ 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; + ret = ocfs2_acl_set_mode(inode, di_bh, + handle, mode); + if (ret) return ret; - else { - if (ret == 0) - acl = NULL; - - ret = ocfs2_acl_set_mode(inode, di_bh, - handle, mode); - if (ret) - return ret; - - } } break; case ACL_TYPE_DEFAULT: diff --git a/fs/posix_acl.c b/fs/posix_acl.c index 3542f1f814e2..1da000aabb08 100644 --- a/fs/posix_acl.c +++ b/fs/posix_acl.c @@ -407,6 +407,37 @@ 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_wrt_inode_uidgid(inode, 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 0e2f37efedd0..9c7b5ce06f4f 100644 --- a/fs/xfs/xfs_acl.c +++ b/fs/xfs/xfs_acl.c @@ -402,16 +402,15 @@ 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); + umode_t mode; + struct posix_acl *old_acl = acl; - if (error <= 0) { - posix_acl_release(acl); - acl = NULL; + error = posix_acl_update_mode(inode, &mode, &acl); - if (error < 0) - return error; - } + if (error) + goto out_release; + if (!acl) + posix_acl_release(old_acl); error = xfs_set_mode(inode, mode); if (error) diff --git a/include/linux/posix_acl.h b/include/linux/posix_acl.h index 7931efe71175..43cb8d59d0a7 100644 --- a/include/linux/posix_acl.h +++ b/include/linux/posix_acl.h @@ -89,6 +89,7 @@ extern int posix_acl_permission(struct inode *, const struct posix_acl *, int); 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_update_mode(struct inode *, umode_t *, struct posix_acl **); extern int posix_acl_chmod(struct posix_acl **, gfp_t, umode_t); extern struct posix_acl *get_posix_acl(struct inode *, int); -- 2.11.0