From patchwork Wed May 3 17:35:55 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Amit Pundir X-Patchwork-Id: 98502 Delivered-To: patch@linaro.org Received: by 10.140.89.200 with SMTP id v66csp166234qgd; Wed, 3 May 2017 10:36:15 -0700 (PDT) X-Received: by 10.84.238.198 with SMTP id l6mr51117469pln.95.1493832975189; Wed, 03 May 2017 10:36:15 -0700 (PDT) Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id p20si12867632pli.35.2017.05.03.10.36.14; Wed, 03 May 2017 10:36:15 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of stable-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@linaro.org; spf=pass (google.com: best guess record for domain of stable-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=stable-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753976AbdECRgO (ORCPT + 6 others); Wed, 3 May 2017 13:36:14 -0400 Received: from mail-pf0-f171.google.com ([209.85.192.171]:35214 "EHLO mail-pf0-f171.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751880AbdECRgN (ORCPT ); Wed, 3 May 2017 13:36:13 -0400 Received: by mail-pf0-f171.google.com with SMTP id v14so15824619pfd.2 for ; Wed, 03 May 2017 10:36:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=/ZEOLzxXRkipJLMI6H1qemwAKPD8EErteUcemL6N+w0=; b=R+W6KAY4cuGRMvO/W3EwFo7shY1h0ncuXhg3vgDfnmp5Ra+mrbJQ+Eo5Wj2Wb0ewAO hC4U683tg8Y08fc4D5SaCsvX8NRgIskD3kx2jR4TZVfQF1Y3TtpqSkNyiqPKzzj2B9jZ B21zZMe6zjanHagFX0uGeeqKzM4s4GqpFYre0= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=/ZEOLzxXRkipJLMI6H1qemwAKPD8EErteUcemL6N+w0=; b=flLu92a8pE5BhS/Xxf2PTXxYtTQ5Xun/jg0xcvH5U5zZLbaxxbs/Cm98b+OtAUp29E 8Pmt8PX5CGZ87FFZpCHjXWxNBD5yDlhgZy2uCwoCnhi9MD+zWDQmo1H8202Mk2j1TySs YWVY1iBMPUlQlvurpSPne8LxXgOM+6jzefCaUO6pUV6mS/8kqhZGwyegVGL2k/j6pQM0 nvcUy572wRRUWb5XRDu7XIDu6g9Y+Bw4ONct+vltV/pleC64BSj6XkhCj9ijGuw0LP5Z h6EzwAF8j+OBdRW3KNa+Mimr1dznB3Y6UP+3x0C3b88ZwfK8L6rBNgPEsipMSGCAJ38w BVdw== X-Gm-Message-State: AN3rC/5Qfoa196xcNdUkPQhvxIcN7kehXmAZPKso2gJ5HDPO1kLaJKWH JGBqzgvKvdaJhJbgrA6vLw== X-Received: by 10.99.144.65 with SMTP id a62mr21180315pge.222.1493832972625; Wed, 03 May 2017 10:36:12 -0700 (PDT) Received: from localhost.localdomain ([106.51.135.126]) by smtp.gmail.com with ESMTPSA id c3sm5895206pfg.46.2017.05.03.10.36.10 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Wed, 03 May 2017 10:36:12 -0700 (PDT) From: Amit Pundir To: Greg KH Cc: stable@vger.kernel.org, Jan Kara , Andreas Gruenbacher Subject: [PATCH for-3.18 4/7] posix_acl: Clear SGID bit when setting file permissions Date: Wed, 3 May 2017 23:05:55 +0530 Message-Id: <1493832958-12489-5-git-send-email-amit.pundir@linaro.org> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1493832958-12489-1-git-send-email-amit.pundir@linaro.org> References: <1493832958-12489-1-git-send-email-amit.pundir@linaro.org> Sender: stable-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: stable@vger.kernel.org From: Jan Kara 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 Reviewed-by: Jeff Layton Signed-off-by: Jan Kara Signed-off-by: Andreas Gruenbacher (cherry picked from commit 073931017b49d9458aa351605b43a7e34598caef) Signed-off-by: Amit Pundir --- fs/9p/acl.c | 40 +++++++++++++++++----------------------- fs/btrfs/acl.c | 6 ++---- fs/ceph/acl.c | 6 ++---- fs/ext2/acl.c | 12 ++++-------- fs/ext4/acl.c | 12 ++++-------- fs/f2fs/acl.c | 6 ++---- fs/gfs2/acl.c | 12 +++--------- fs/hfsplus/posix_acl.c | 4 ++-- fs/jffs2/acl.c | 9 ++++----- fs/jfs/acl.c | 6 ++---- fs/ocfs2/acl.c | 20 ++++++++------------ fs/posix_acl.c | 31 +++++++++++++++++++++++++++++++ fs/reiserfs/xattr_acl.c | 8 ++------ fs/xfs/xfs_acl.c | 13 ++++--------- include/linux/posix_acl.h | 1 + 15 files changed, 88 insertions(+), 98 deletions(-) -- 2.7.4 diff --git a/fs/9p/acl.c b/fs/9p/acl.c index 8482f2d11606..d3f5d487ae46 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 9a0124a95851..fb3e64d37cb4 100644 --- a/fs/btrfs/acl.c +++ b/fs/btrfs/acl.c @@ -83,11 +83,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/ceph/acl.c b/fs/ceph/acl.c index 5bd853ba44ff..6a4a3e2a46cf 100644 --- a/fs/ceph/acl.c +++ b/fs/ceph/acl.c @@ -108,11 +108,9 @@ int ceph_set_acl(struct inode *inode, struct posix_acl *acl, int type) case ACL_TYPE_ACCESS: name = POSIX_ACL_XATTR_ACCESS; if (acl) { - ret = posix_acl_equiv_mode(acl, &new_mode); - if (ret < 0) + ret = posix_acl_update_mode(inode, &new_mode, &acl); + if (ret) goto out; - if (ret == 0) - acl = NULL; } break; case ACL_TYPE_DEFAULT: diff --git a/fs/ext2/acl.c b/fs/ext2/acl.c index 27695e6f4e46..d6aeb84e90b6 100644 --- a/fs/ext2/acl.c +++ b/fs/ext2/acl.c @@ -193,15 +193,11 @@ ext2_set_acl(struct inode *inode, struct posix_acl *acl, int type) 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/ext4/acl.c b/fs/ext4/acl.c index d40c8dbbb0d6..87d9bbf6a53f 100644 --- a/fs/ext4/acl.c +++ b/fs/ext4/acl.c @@ -201,15 +201,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 83b9b5a8d112..f12d5c5ecc31 100644 --- a/fs/f2fs/acl.c +++ b/fs/f2fs/acl.c @@ -207,12 +207,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/gfs2/acl.c b/fs/gfs2/acl.c index 7b3143064af1..88e66aa516c4 100644 --- a/fs/gfs2/acl.c +++ b/fs/gfs2/acl.c @@ -79,17 +79,11 @@ int gfs2_set_acl(struct inode *inode, struct posix_acl *acl, int type) if (type == ACL_TYPE_ACCESS) { umode_t mode = inode->i_mode; - error = posix_acl_equiv_mode(acl, &mode); - if (error < 0) + error = posix_acl_update_mode(inode, &inode->i_mode, &acl); + if (error) return error; - - if (error == 0) - acl = NULL; - - if (mode != inode->i_mode) { - inode->i_mode = mode; + if (mode != inode->i_mode) mark_inode_dirty(inode); - } } if (acl) { diff --git a/fs/hfsplus/posix_acl.c b/fs/hfsplus/posix_acl.c index df0c9af68d05..71b3087b7e32 100644 --- a/fs/hfsplus/posix_acl.c +++ b/fs/hfsplus/posix_acl.c @@ -68,8 +68,8 @@ int hfsplus_set_posix_acl(struct inode *inode, struct posix_acl *acl, 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 2f7a3c090489..f9f86f87d32b 100644 --- a/fs/jffs2/acl.c +++ b/fs/jffs2/acl.c @@ -235,9 +235,10 @@ int jffs2_set_acl(struct inode *inode, struct posix_acl *acl, int type) 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; @@ -249,8 +250,6 @@ int jffs2_set_acl(struct inode *inode, struct posix_acl *acl, int type) if (rc < 0) return rc; } - if (rc == 0) - acl = NULL; } break; case ACL_TYPE_DEFAULT: diff --git a/fs/jfs/acl.c b/fs/jfs/acl.c index 0c8ca830b113..9fad9f4fe883 100644 --- a/fs/jfs/acl.c +++ b/fs/jfs/acl.c @@ -84,13 +84,11 @@ static int __jfs_set_acl(tid_t tid, struct inode *inode, int type, case ACL_TYPE_ACCESS: ea_name = POSIX_ACL_XATTR_ACCESS; if (acl) { - rc = posix_acl_equiv_mode(acl, &inode->i_mode); - if (rc < 0) + rc = posix_acl_update_mode(inode, &inode->i_mode, &acl); + if (rc) return rc; inode->i_ctime = CURRENT_TIME; mark_inode_dirty(inode); - if (rc == 0) - acl = NULL; } break; case ACL_TYPE_DEFAULT: diff --git a/fs/ocfs2/acl.c b/fs/ocfs2/acl.c index 8a7d2f812b5b..c7641f656494 100644 --- a/fs/ocfs2/acl.c +++ b/fs/ocfs2/acl.c @@ -241,20 +241,16 @@ 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) - return ret; - else { - if (ret == 0) - acl = NULL; + umode_t mode; - ret = ocfs2_acl_set_mode(inode, di_bh, - handle, mode); - if (ret) - return ret; + 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; } break; case ACL_TYPE_DEFAULT: diff --git a/fs/posix_acl.c b/fs/posix_acl.c index 3de7c223c963..38c91932eb52 100644 --- a/fs/posix_acl.c +++ b/fs/posix_acl.c @@ -594,6 +594,37 @@ no_acl: } EXPORT_SYMBOL_GPL(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); + /* * Fix up the uids and gids in posix acl extended attributes in place. */ diff --git a/fs/reiserfs/xattr_acl.c b/fs/reiserfs/xattr_acl.c index 4b34b9dc03dd..9b1824f35501 100644 --- a/fs/reiserfs/xattr_acl.c +++ b/fs/reiserfs/xattr_acl.c @@ -246,13 +246,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 a65fa5dde6e9..e0406717edbc 100644 --- a/fs/xfs/xfs_acl.c +++ b/fs/xfs/xfs_acl.c @@ -286,16 +286,11 @@ xfs_set_acl(struct inode *inode, struct posix_acl *acl, int type) return error; if (type == ACL_TYPE_ACCESS) { - umode_t mode = inode->i_mode; - error = posix_acl_equiv_mode(acl, &mode); - - if (error <= 0) { - acl = NULL; - - if (error < 0) - return error; - } + umode_t mode; + error = posix_acl_update_mode(inode, &mode, &acl); + if (error) + return error; error = xfs_set_mode(inode, mode); if (error) return error; diff --git a/include/linux/posix_acl.h b/include/linux/posix_acl.h index 3e96a6a76103..d1a8ad7e5ae4 100644 --- a/include/linux/posix_acl.h +++ b/include/linux/posix_acl.h @@ -95,6 +95,7 @@ extern int set_posix_acl(struct inode *, int, struct posix_acl *); extern int posix_acl_chmod(struct inode *, umode_t); extern int posix_acl_create(struct inode *, umode_t *, struct posix_acl **, struct posix_acl **); +extern int posix_acl_update_mode(struct inode *, umode_t *, struct posix_acl **); extern int simple_set_acl(struct inode *, struct posix_acl *, int); extern int simple_acl_create(struct inode *, struct inode *);