BACKPORT: posix_acl: Clear SGID bit when setting file permissions

(cherry pick from commit 073931017b49d9458aa351605b43a7e34598caef)

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.

NB: conflicts resolution included extending the change to all visible
    users of the near deprecated function posix_acl_equiv_mode
    replaced with posix_acl_update_mode. We did not resolve the ACL
    leak in this CL, require additional upstream fixes.

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>
Bug: 32458736
Change-Id: I19591ad452cc825ac282b3cfd2daaa72aa9a1ac1

diff --git a/fs/9p/acl.c b/fs/9p/acl.c
index 7af425f53beef91a6d21dc86b76e161ff7ed1696..9686c1f17653f4469156ea19d83a1ceaeef763da 100644 (file)
--- 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 0890c83643e944f69e43a22f4151e381e7503f04..d6d53e5e7945db193e1fec7cdfbb6b230b916542 100644 (file)
--- 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 110b6b371a4edc353fc1904bcc79299580087184..48c3c2d7d2619e4c27121de46d9d8dc8c9dbb391 100644 (file)
--- 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 dbb5ad59a7fc3c22380ce30b9034f8942bec967b..bb2f60a62d82ad42f8659e858a360a9231631957 100644 (file)
--- 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 39a54a0e9fe4a9a6290f75b961cde9ccea06f669..c844f1bfb451920f896269f05d511eb46b696cba 100644 (file)
--- 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/generic_acl.c b/fs/generic_acl.c
index b3f3676796d31a6870a771ed6952977cfc16d448..21408084c3b34c9fb51d706fc69480a9fd2915a2 100644 (file)
--- a/fs/generic_acl.c
+++ b/fs/generic_acl.c
@@ -87,14 +87,10 @@ generic_acl_set(struct dentry *dentry, const char *name, const void *value,
                        goto failed;
                switch (type) {
                case ACL_TYPE_ACCESS:
-                       error = posix_acl_equiv_mode(acl, &inode->i_mode);
-                       if (error < 0)
+                       error = posix_acl_update_mode(inode, &inode->i_mode, &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)) {

diff --git a/fs/gfs2/acl.c b/fs/gfs2/acl.c
index f69ac0af5496cd456bf6df4d0c932747b6767779..e7b330ef4dfdaaaf636254f304051f3f442cf6e5 100644 (file)
--- a/fs/gfs2/acl.c
+++ b/fs/gfs2/acl.c
@@ -268,15 +268,10 @@ 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);
+               error = posix_acl_update_mode(inode, &inode->i_mode, &acl);
 
-               if (error <= 0) {
-                       posix_acl_release(acl);
-                       acl = NULL;
-
-                       if (error < 0)
-                               return error;
-               }
+               if (error)
+                       goto out_release;
 
                error = gfs2_set_mode(inode, mode);
                if (error)

diff --git a/fs/jffs2/acl.c b/fs/jffs2/acl.c
index 223283c301116f3d8d88cb70cf30aa030a54c456..9335b8d3cf5212cede3093af48bf37567400623d 100644 (file)
--- 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 42d67f9757bf641d316b07e78a3fb1a698f76409..c79b1d7a53e24be8c918313318b325b949997954 100644 (file)
--- a/fs/jfs/xattr.c
+++ b/fs/jfs/xattr.c
@@ -693,11 +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);
+                       rc = posix_acl_update_mode(inode, &inode->i_mode, &acl);
                        posix_acl_release(acl);
-                       if (rc < 0) {
+                       if (rc) {
                                printk(KERN_ERR
-                                      "posix_acl_equiv_mode returned %d\n",
+                                      "posix_acl_update_mode returned %d\n",
                                       rc);
                                return rc;
                        }

diff --git a/fs/ocfs2/acl.c b/fs/ocfs2/acl.c
index 8a404576fb26557eb8578e21f66d0277a94a9e76..2fe643160cf735d4d157cd1c1af6d43e30dccb89 100644 (file)
--- a/fs/ocfs2/acl.c
+++ b/fs/ocfs2/acl.c
@@ -275,19 +275,13 @@ static int ocfs2_set_acl(handle_t *handle,
                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)
+                       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 3542f1f814e2a966d33429f61e9f78ab5404a18c..35cc1f40b82df52a0aa14ccb1830b7726c36463a 100644 (file)
--- a/fs/posix_acl.c
+++ b/fs/posix_acl.c
@@ -341,6 +341,37 @@ static int posix_acl_create_masq(struct posix_acl *acl, umode_t *mode_p)
         return not_equiv;
 }
 
+/**
+ * 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);
+
 /*
  * Modify the ACL for the chmod syscall.
  */

diff --git a/fs/reiserfs/xattr_acl.c b/fs/reiserfs/xattr_acl.c
index 6c8767fdfc6a287b0a9d201b4048d89b06fecaad..2d73589f37d643fe2a38ab76188cc92e2c4c91d6 100644 (file)
--- a/fs/reiserfs/xattr_acl.c
+++ b/fs/reiserfs/xattr_acl.c
@@ -286,13 +286,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 306d883d89bc7d6420ca4b5b8c5f848e573249bf..4f8e4770229cf78e0659e241aa11902a11b3609b 100644 (file)
--- a/fs/xfs/xfs_acl.c
+++ b/fs/xfs/xfs_acl.c
@@ -389,16 +389,9 @@ xfs_xattr_acl_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);
-
-               if (error <= 0) {
-                       posix_acl_release(acl);
-                       acl = NULL;
-
-                       if (error < 0)
-                               return error;
-               }
-
+               error = posix_acl_update_mode(inode, &mode, &acl);
+               if (error)
+                       goto out_release;
                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 7931efe7117553d00a920cfd4aef757f72578658..2ae0bba45f12478feab0808b5cf2442e6a6e4165 100644 (file)
--- 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 *);