* Re: [LTP] [RFC PATCH 1/1] creat09: Fix on more restrictive umask
[not found] ` <623460FD.8070500@fujitsu.com>
@ 2022-03-18 22:12 ` Darrick J. Wong
2022-03-22 6:28 ` xuyang2018.jy
0 siblings, 1 reply; 2+ messages in thread
From: Darrick J. Wong @ 2022-03-18 22:12 UTC (permalink / raw)
To: xuyang2018.jy; +Cc: ltp, Jan Kara, Petr Vorel, Martin Doucha, xfs
You really ought to cc the xfs list for questions about longstanding
behaviors of XFS...
[cc linux-xfs]
--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);
> >
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [LTP] [RFC PATCH 1/1] creat09: Fix on more restrictive umask
2022-03-18 22:12 ` [LTP] [RFC PATCH 1/1] creat09: Fix on more restrictive umask Darrick J. Wong
@ 2022-03-22 6:28 ` xuyang2018.jy
0 siblings, 0 replies; 2+ messages in thread
From: xuyang2018.jy @ 2022-03-22 6:28 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: ltp, Jan Kara, Petr Vorel, Martin Doucha, xfs
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);
>>>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-03-22 6:30 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20220314191234.12382-1-pvorel@suse.cz>
[not found] ` <62343BF2.1020006@fujitsu.com>
[not found] ` <623460FD.8070500@fujitsu.com>
2022-03-18 22:12 ` [LTP] [RFC PATCH 1/1] creat09: Fix on more restrictive umask Darrick J. Wong
2022-03-22 6:28 ` xuyang2018.jy
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).