Re: [PATCH 09/11] orangefs: Don't clear SGID when inheriting ACLs

[Jan Kara <jack@suse.cz>]

On Thu 22-06-17 15:31:13, Jan Kara wrote:
> When new directory 'DIR1' is created in a directory 'DIR0' with SGID bit
> set, DIR1 is expected to have SGID bit set (and owning group equal to
> the owning group of 'DIR0'). However when 'DIR0' also has some default
> ACLs that 'DIR1' inherits, setting these ACLs will result in SGID bit on
> 'DIR1' to get cleared if user is not member of the owning group.
> 
> Fix the problem by creating __orangefs_set_acl() function that does not
> call posix_acl_update_mode() and use it when inheriting ACLs. That
> prevents SGID bit clearing and the mode has been properly set by
> posix_acl_create() anyway.
> 
> Fixes: 073931017b49d9458aa351605b43a7e34598caef
> CC: stable@vger.kernel.org
> CC: Mike Marshall <hubcap@omnibond.com>
> CC: pvfs2-developers@beowulf-underground.org
> Signed-off-by: Jan Kara <jack@suse.cz>

Mike, can you please pick up this fix? Thanks!

								Honza

> ---
>  fs/orangefs/acl.c | 48 ++++++++++++++++++++++++++++--------------------
>  1 file changed, 28 insertions(+), 20 deletions(-)
> 
> diff --git a/fs/orangefs/acl.c b/fs/orangefs/acl.c
> index 7a3754488312..9409aac232f7 100644
> --- a/fs/orangefs/acl.c
> +++ b/fs/orangefs/acl.c
> @@ -61,9 +61,9 @@ struct posix_acl *orangefs_get_acl(struct inode *inode, int type)
>  	return acl;
>  }
>  
> -int orangefs_set_acl(struct inode *inode, struct posix_acl *acl, int type)
> +static int __orangefs_set_acl(struct inode *inode, struct posix_acl *acl,
> +			      int type)
>  {
> -	struct orangefs_inode_s *orangefs_inode = ORANGEFS_I(inode);
>  	int error = 0;
>  	void *value = NULL;
>  	size_t size = 0;
> @@ -72,22 +72,6 @@ int orangefs_set_acl(struct inode *inode, struct posix_acl *acl, int type)
>  	switch (type) {
>  	case ACL_TYPE_ACCESS:
>  		name = XATTR_NAME_POSIX_ACL_ACCESS;
> -		if (acl) {
> -			umode_t mode;
> -
> -			error = posix_acl_update_mode(inode, &mode, &acl);
> -			if (error) {
> -				gossip_err("%s: posix_acl_update_mode err: %d\n",
> -					   __func__,
> -					   error);
> -				return error;
> -			}
> -
> -			if (inode->i_mode != mode)
> -				SetModeFlag(orangefs_inode);
> -			inode->i_mode = mode;
> -			mark_inode_dirty_sync(inode);
> -		}
>  		break;
>  	case ACL_TYPE_DEFAULT:
>  		name = XATTR_NAME_POSIX_ACL_DEFAULT;
> @@ -132,6 +116,29 @@ int orangefs_set_acl(struct inode *inode, struct posix_acl *acl, int type)
>  	return error;
>  }
>  
> +int orangefs_set_acl(struct inode *inode, struct posix_acl *acl, int type)
> +{
> +	int error;
> +
> +	if (type == ACL_TYPE_ACCESS && acl) {
> +		umode_t mode;
> +
> +		error = posix_acl_update_mode(inode, &mode, &acl);
> +		if (error) {
> +			gossip_err("%s: posix_acl_update_mode err: %d\n",
> +				   __func__,
> +				   error);
> +			return error;
> +		}
> +
> +		if (inode->i_mode != mode)
> +			SetModeFlag(ORANGEFS_I(inode));
> +		inode->i_mode = mode;
> +		mark_inode_dirty_sync(inode);
> +	}
> +	return __orangefs_set_acl(inode, acl, type);
> +}
> +
>  int orangefs_init_acl(struct inode *inode, struct inode *dir)
>  {
>  	struct orangefs_inode_s *orangefs_inode = ORANGEFS_I(inode);
> @@ -146,13 +153,14 @@ int orangefs_init_acl(struct inode *inode, struct inode *dir)
>  		return error;
>  
>  	if (default_acl) {
> -		error = orangefs_set_acl(inode, default_acl, ACL_TYPE_DEFAULT);
> +		error = __orangefs_set_acl(inode, default_acl,
> +					   ACL_TYPE_DEFAULT);
>  		posix_acl_release(default_acl);
>  	}
>  
>  	if (acl) {
>  		if (!error)
> -			error = orangefs_set_acl(inode, acl, ACL_TYPE_ACCESS);
> +			error = __orangefs_set_acl(inode, acl, ACL_TYPE_ACCESS);
>  		posix_acl_release(acl);
>  	}
>  
> -- 
> 2.12.3
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR