linux-kernel - Re: [PATCH] tmpfs: clear S_ISGID when setting posix ACLs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Mon, 9 Jan 2017 09:26:58 +0800
From:   Gu Zheng <guzheng1@...wei.com>
To:     Jan Kara <jack@...e.cz>
CC:     <linux-fsdevel@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <viro@...iv.linux.org.uk>
Subject: Re: [PATCH] tmpfs: clear S_ISGID when setting posix ACLs

thanks, I will update it.

在 2017/1/6 18:10, Jan Kara 写道:
> On Fri 06-01-17 16:12:55, Gu Zheng wrote:
>> This change was missed the tmpfs modification in In CVE-2016-7097
>> commit 073931017b49d9458aa351605b43a7e34598caef
>> posix_acl: Clear SGID bit when setting file permissions.
>> It can test by xfstest generic/375, which failed to clear
>> setgid bit in the following test case on tmpfs:
>>
>>    touch $testfile
>>    chown 100:100 $testfile
>>    chmod 2755 $testfile
>>    _runas -u 100 -g 101 -- setfacl -m u::rwx,g::rwx,o::rwx $testfile
>>
>> Signed-off-by: Gu Zheng <guzheng1@...wei.com>
>
> Ah, good catch. One comment below:
>
>> diff --git a/fs/posix_acl.c b/fs/posix_acl.c
>> index 5955220..d014dff 100644
>> --- a/fs/posix_acl.c
>> +++ b/fs/posix_acl.c
>> @@ -922,11 +922,10 @@ int simple_set_acl(struct inode *inode, struct posix_acl *acl, int type)
>>   	int error;
>>
>>   	if (type == ACL_TYPE_ACCESS) {
>> -		error = posix_acl_equiv_mode(acl, &inode->i_mode);
>> -		if (error < 0)
>> -			return 0;
>> -		if (error == 0)
>> -			acl = NULL;
>> +		error = posix_acl_update_mode(inode,
>> +				&inode->i_mode, &acl);
>> +		if (error > 0)
>> +			return error;
>
> Uh, why this error > 0 check? AFAIU it should be:
>
> 	if (error < 0)
> 		return 0;
>
> As it used to be before...
>
> 								Honza
>