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@xxxxxxxxxxxxxxx
> CC: Mike Marshall <hubcap@xxxxxxxxxxxx>
> CC: pvfs2-developers@xxxxxxxxxxxxxxxxxxxxxxx
> Signed-off-by: Jan Kara <jack@xxxxxxx>
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@xxxxxxxx>
SUSE Labs, CR