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

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: [email protected]
>> CC: Dave Kleikamp <[email protected]>
>> CC: [email protected]
>> Signed-off-by: Jan Kara <[email protected]>
> 
> 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, &inode->i_mode, &acl);
>> -			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(&JFS_IP(inode)->commit_mutex);
>> +	if (type == ACL_TYPE_ACCESS && acl) {
>> +		rc = posix_acl_update_mode(inode, &inode->i_mode, &acl);
>> +		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, &inode, 0);
>> +end_tx:
>>  	txEnd(tid);
>>  	mutex_unlock(&JFS_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