[LTP] [RFC PATCH 1/1] creat09: Fix on more restrictive umask

[xuyang2018.jy@fujitsu.com]

on 2022/3/19 6:12, Darrick J. Wong wrote:
> You really ought to cc the xfs list for questions about longstanding
> behaviors of XFS...
>
> [cc linux-xfs]
Oh, yes, sorry for this. I have sent a RFC patch to linux-xfs and we can 
move discussion on that mail.

Best Regards
Yang Xu
>
> --D
>
> On Fri, Mar 18, 2022 at 10:37:03AM +0000, xuyang2018.jy@fujitsu.com wrote:
>> Hi Darrick, Jack
>>
>> Petr meet a problem when running creat09 on xfs, ext4 doesn't have problem.
>>
>> It seems xfs will still use umask when enable default acl, but ext4 will
>> not.
>>
>> As umask2 manpage , it said
>> "Alternatively, if the parent directory has a default ACL (see acl(5)),
>> the umask is ignored, the default ACL is inherited, the permission bits
>> are set based on the inherited ACL, and permission bits absent
>>          in the mode argument are turned off.
>> "
>>
>> It seem xfs doesn't obey this rule.
>>
>> the xfs calltrace as below:
>>
>>     will use  inode_init_owner(struct user_namespace *mnt_userns,
>> structinode *inode)
>>
>>    296.760675]  xfs_init_new_inode+0x10e/0x6c0
>> [  296.760678]  xfs_create+0x401/0x610
>>     will use posix_acl_create(dir,&mode,&default_acl,&acl);
>> [  296.760681]  xfs_generic_create+0x123/0x2e0
>> [  296.760684]  ? _raw_spin_unlock+0x16/0x30
>> [  296.760687]  path_openat+0xfb8/0x1210
>> [  296.760689]  do_filp_open+0xb4/0x120
>> [  296.760691]  ? file_tty_write.isra.31+0x203/0x340
>> [  296.760697]  ? __check_object_size+0x150/0x170
>> [  296.760699]  do_sys_openat2+0x242/0x310
>> [  296.760702]  do_sys_open+0x4b/0x80
>> [  296.760704]  do_syscall_64+0x3a/0x80
>>
>>
>> the ext4 calltrace as below:
>> [  296.460999]  __ext4_new_inode+0xe07/0x1780 [ext4]
>> posix_acl_create(dir,&inode->i_mode,&default_acl,&acl);
>> [  296.461035]  ext4_create+0x106/0x1c0 [ext4]
>> [  296.461059]  path_openat+0xfb8/0x1210
>> [  296.461062]  do_filp_open+0xb4/0x120
>> [  296.461065]  ? __check_object_size+0x150/0x170
>> [  296.461068]  do_sys_openat2+0x242/0x310
>> [  296.461070]  do_sys_open+0x4b/0x80
>> [  296.461073]  do_syscall_64+0x3a/0x80
>> [  296.461077]  entry_SYSCALL_64_after_hwframe+0x44/0xae
>>
>> I guess xfs modify its mode value instead of inode->i_mode in
>> posix_acl_create by using current->umask value, so inode_init_owner
>> doesn't clear no-sgid bits on created file because of missing S_IXGRP.
>>
>> Is it a kernel bug?
>>
>> Best Regards
>> Yang Xu
>>
>>> Hi Petr
>>>
>>> It fails because the create file without S_IXGRP mode, then we miss
>>> remove S_ISGID[1] bit.
>>>
>>> But I don't known why other filesystem doesn't have this problem.
>>>
>>> [1]
>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/inode.c#n2249
>>>
>>> Best Regards
>>> Yang Xu
>>>> XFS fails on umask 0077:
>>>>
>>>> tst_test.c:1528: TINFO: Testing on xfs
>>>> tst_test.c:997: TINFO: Formatting /dev/loop0 with xfs opts='' extra opts=''
>>>> tst_test.c:1458: TINFO: Timeout per run is 0h 05m 00s
>>>> creat09.c:61: TINFO: User nobody: uid = 65534, gid = 65534
>>>> creat09.c:62: TINFO: Found unused GID 3: SUCCESS (0)
>>>> creat09.c:93: TPASS: mntpoint/testdir/creat.tmp: Owned by correct group
>>>> creat09.c:97: TFAIL: mntpoint/testdir/creat.tmp: Setgid bit is set
>>>> creat09.c:93: TPASS: mntpoint/testdir/open.tmp: Owned by correct group
>>>> creat09.c:97: TFAIL: mntpoint/testdir/open.tmp: Setgid bit is set
>>>>
>>>> Thus clear the default umask.
>>>>
>>>> Signed-off-by: Petr Vorel<pvorel@suse.cz>
>>>> ---
>>>>     testcases/kernel/syscalls/creat/creat09.c | 2 ++
>>>>     1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/testcases/kernel/syscalls/creat/creat09.c b/testcases/kernel/syscalls/creat/creat09.c
>>>> index bed7bddb0e..70da7d2fc7 100644
>>>> --- a/testcases/kernel/syscalls/creat/creat09.c
>>>> +++ b/testcases/kernel/syscalls/creat/creat09.c
>>>> @@ -56,6 +56,8 @@ static void setup(void)
>>>>     		(int)ltpuser->pw_gid);
>>>>     	free_gid = tst_get_free_gid(ltpuser->pw_gid);
>>>>
>>>> +	umask(0);
>>>> +
>>>>     	/* Create directories and set permissions */
>>>>     	SAFE_MKDIR(WORKDIR, MODE_RWX);
>>>>     	SAFE_CHOWN(WORKDIR, ltpuser->pw_uid, free_gid);
>>>