Re: [Jfs-discussion] [PATCH 06/11] jfs: Don't clear SGID when inheriting ACLs

2017-07-18 Thread Dave Kleikamp
On 07/18/2017 11:19 AM, Jan Kara wrote:
> On Thu 22-06-17 15:31:10, 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 moving posix_acl_update_mode() out of
>> __jfs_set_acl() into jfs_set_acl(). That way the function will not be
>> called when inheriting ACLs which is what we want as it prevents SGID
>> bit clearing and the mode has been properly set by posix_acl_create()
>> anyway.
>>
>> Fixes: 073931017b49d9458aa351605b43a7e34598caef
>> CC: sta...@vger.kernel.org
>> CC: Dave Kleikamp 
>> CC: jfs-discussion@lists.sourceforge.net
>> Signed-off-by: Jan Kara 
> 
> Dave, can you please pick up this fix? Thanks!

Yeah. I'll take care if it.

Thanks,
Shaggy

> 
>   Honza
> 
>> ---
>>  fs/jfs/acl.c | 15 ---
>>  1 file changed, 8 insertions(+), 7 deletions(-)
>>
>> diff --git a/fs/jfs/acl.c b/fs/jfs/acl.c
>> index 7bc186f4ed4d..1be45c8d460d 100644
>> --- a/fs/jfs/acl.c
>> +++ b/fs/jfs/acl.c
>> @@ -77,13 +77,6 @@ static int __jfs_set_acl(tid_t tid, struct inode *inode, 
>> int type,
>>  switch (type) {
>>  case ACL_TYPE_ACCESS:
>>  ea_name = XATTR_NAME_POSIX_ACL_ACCESS;
>> -if (acl) {
>> -rc = posix_acl_update_mode(inode, >i_mode, );
>> -if (rc)
>> -return rc;
>> -inode->i_ctime = current_time(inode);
>> -mark_inode_dirty(inode);
>> -}
>>  break;
>>  case ACL_TYPE_DEFAULT:
>>  ea_name = XATTR_NAME_POSIX_ACL_DEFAULT;
>> @@ -118,9 +111,17 @@ int jfs_set_acl(struct inode *inode, struct posix_acl 
>> *acl, int type)
>>  
>>  tid = txBegin(inode->i_sb, 0);
>>  mutex_lock(_IP(inode)->commit_mutex);
>> +if (type == ACL_TYPE_ACCESS && acl) {
>> +rc = posix_acl_update_mode(inode, >i_mode, );
>> +if (rc)
>> +goto end_tx;
>> +inode->i_ctime = current_time(inode);
>> +mark_inode_dirty(inode);
>> +}
>>  rc = __jfs_set_acl(tid, inode, type, acl);
>>  if (!rc)
>>  rc = txCommit(tid, 1, , 0);
>> +end_tx:
>>  txEnd(tid);
>>  mutex_unlock(_IP(inode)->commit_mutex);
>>  return rc;
>> -- 
>> 2.12.3
>>

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Jfs-discussion mailing list
Jfs-discussion@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/jfs-discussion


Re: [Jfs-discussion] [PATCH 06/11] jfs: Don't clear SGID when inheriting ACLs

2017-07-18 Thread Jan Kara
On Thu 22-06-17 15:31:10, 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 moving posix_acl_update_mode() out of
> __jfs_set_acl() into jfs_set_acl(). That way the function will not be
> called when inheriting ACLs which is what we want as it prevents SGID
> bit clearing and the mode has been properly set by posix_acl_create()
> anyway.
> 
> Fixes: 073931017b49d9458aa351605b43a7e34598caef
> CC: sta...@vger.kernel.org
> CC: Dave Kleikamp 
> CC: jfs-discussion@lists.sourceforge.net
> Signed-off-by: Jan Kara 

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

Honza

> ---
>  fs/jfs/acl.c | 15 ---
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/jfs/acl.c b/fs/jfs/acl.c
> index 7bc186f4ed4d..1be45c8d460d 100644
> --- a/fs/jfs/acl.c
> +++ b/fs/jfs/acl.c
> @@ -77,13 +77,6 @@ static int __jfs_set_acl(tid_t tid, struct inode *inode, 
> int type,
>   switch (type) {
>   case ACL_TYPE_ACCESS:
>   ea_name = XATTR_NAME_POSIX_ACL_ACCESS;
> - if (acl) {
> - rc = posix_acl_update_mode(inode, >i_mode, );
> - if (rc)
> - return rc;
> - inode->i_ctime = current_time(inode);
> - mark_inode_dirty(inode);
> - }
>   break;
>   case ACL_TYPE_DEFAULT:
>   ea_name = XATTR_NAME_POSIX_ACL_DEFAULT;
> @@ -118,9 +111,17 @@ int jfs_set_acl(struct inode *inode, struct posix_acl 
> *acl, int type)
>  
>   tid = txBegin(inode->i_sb, 0);
>   mutex_lock(_IP(inode)->commit_mutex);
> + if (type == ACL_TYPE_ACCESS && acl) {
> + rc = posix_acl_update_mode(inode, >i_mode, );
> + if (rc)
> + goto end_tx;
> + inode->i_ctime = current_time(inode);
> + mark_inode_dirty(inode);
> + }
>   rc = __jfs_set_acl(tid, inode, type, acl);
>   if (!rc)
>   rc = txCommit(tid, 1, , 0);
> +end_tx:
>   txEnd(tid);
>   mutex_unlock(_IP(inode)->commit_mutex);
>   return rc;
> -- 
> 2.12.3
> 
-- 
Jan Kara 
SUSE Labs, CR

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Jfs-discussion mailing list
Jfs-discussion@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/jfs-discussion