Commit 07393101 authored by Jan Kara's avatar Jan Kara
Browse files

posix_acl: Clear SGID bit when setting file permissions 



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: default avatarChristoph Hellwig <hch@lst.de>
Reviewed-by: default avatarJeff Layton <jlayton@redhat.com>
Signed-off-by: default avatarJan Kara <jack@suse.cz>
Signed-off-by: default avatarAndreas Gruenbacher <agruenba@redhat.com>
parent 5d3ddd84

fs/9p/acl.c

+17 −23
Original line number Diff line number Diff line
@@ -276,25 +276,20 @@ static int v9fs_xattr_set_acl(const struct xattr_handler *handler, 
	switch (handler->flags) {

	case ACL_TYPE_ACCESS:

		if (acl) {

			umode_t mode = inode->i_mode;

			retval = posix_acl_equiv_mode(acl, &mode);

			if (retval < 0)

				goto err_out;

			else {

			struct iattr iattr;

				if (retval == 0) {



			retval = posix_acl_update_mode(inode, &iattr.ia_mode, &acl);

			if (retval)

				goto err_out;

			if (!acl) {

				/*

				 * 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
@@ -302,7 +297,6 @@ static int v9fs_xattr_set_acl(const struct xattr_handler *handler, 
			 */

			v9fs_vfs_setattr_dotl(dentry, &iattr);

		}

		}

		break;

	case ACL_TYPE_DEFAULT:

		if (!S_ISDIR(inode->i_mode)) {

fs/btrfs/acl.c

+2 −4
Original line number Diff line number Diff line
@@ -79,11 +79,9 @@ static int __btrfs_set_acl(struct btrfs_trans_handle *trans, 
	case ACL_TYPE_ACCESS:

		name = XATTR_NAME_POSIX_ACL_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;

fs/ceph/acl.c

+2 −4
Original line number Diff line number Diff line
@@ -95,11 +95,9 @@ int ceph_set_acl(struct inode *inode, struct posix_acl *acl, int type) 
	case ACL_TYPE_ACCESS:

		name = XATTR_NAME_POSIX_ACL_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:

fs/ext2/acl.c

+4 −8
Original line number Diff line number Diff line
@@ -190,15 +190,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;

				}

			}

			break;

fs/ext4/acl.c

+4 −8
Original line number Diff line number Diff line
@@ -193,15 +193,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;

			}

		}

		break;

CRA Git | Maintained and supported by SUSTech CRA and CCSE