Re: [PATCH v1 1/2] vfs: Add new setgid_create_umask test

["xuyang2018.jy@fujitsu.com" <xuyang2018.jy@fujitsu.com>]

on 2022/07/25 22:20, Christian Brauner wrote:
> On Mon, Jul 25, 2022 at 07:55:23AM +0000, xuyang2018.jy@fujitsu.com wrote:
>>
>> on  2022/05/21 0:04, Yang Xu wrote:
>>> The current_umask() is stripped from the mode directly in the vfs if the
>>> filesystem either doesn't support acls or the filesystem has been
>>> mounted without posic acl support.
>>>
>>> If the filesystem does support acls then current_umask() stripping is
>>> deferred to posix_acl_create(). So when the filesystem calls
>>> posix_acl_create() and there are no acls set or not supported then
>>> current_umask() will be stripped.
>>>
>>> This patch is also designed to test kernel patchset behaviour
>>> "move S_ISGID stripping into the vfs"
>>> https://patchwork.kernel.org/project/linux-fsdevel/list/?series=635692
>>
>> The kernel patch has been merged into linux-next branch[1].
>>
>> Does anyone review this fstests patch or give some comment?
>>
>> I plan to remove setgid_create_umask_idmapped_in_userns and
>> setgid_create_umask_idmapped cases because they doesn't trigger bug.
>>
>> Just treat it  as a kernel regression test ie 31c01ce18 ("generic: add
>> test for tmpfs POSIX ACLs") format.
>>
>> CC Christian to confirm that whether need to test idmapped and
>> idmapped_in_userbs situation .
> 
> You've also sent a patch to LTP I saw which is good. I'm not completely
> sure why you want to remove setgid_create_umask_idmapped{_in_userns}.
> But as long as we have tests for setgid stripping on idmapped mounts I'm
> fine with this.

I think about it again today, maybe still have  some meaningful. I will 
keep it on v2. v2 only updates a small, include add kernel commit id and 
remove duplicated comment.

Best Regards
Yang Xu
> 
> Christian