All of lore.kernel.org
 help / color / mirror / Atom feed
* [LTP] [RFC PATCH 1/1] creat09: Fix on more restrictive umask
@ 2022-03-14 19:12 Petr Vorel
  2022-03-18  7:59 ` xuyang2018.jy
  2022-07-06  9:37 ` xuyang2018.jy
  0 siblings, 2 replies; 12+ messages in thread
From: Petr Vorel @ 2022-03-14 19:12 UTC (permalink / raw)
  To: ltp; +Cc: Martin Doucha

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);
-- 
2.35.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [LTP] [RFC PATCH 1/1] creat09: Fix on more restrictive umask
  2022-03-14 19:12 [LTP] [RFC PATCH 1/1] creat09: Fix on more restrictive umask Petr Vorel
@ 2022-03-18  7:59 ` xuyang2018.jy
  2022-03-18 10:37   ` xuyang2018.jy
  2022-07-06  9:37 ` xuyang2018.jy
  1 sibling, 1 reply; 12+ messages in thread
From: xuyang2018.jy @ 2022-03-18  7:59 UTC (permalink / raw)
  To: Petr Vorel; +Cc: Martin Doucha, ltp

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);

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [LTP] [RFC PATCH 1/1] creat09: Fix on more restrictive umask
  2022-03-18  7:59 ` xuyang2018.jy
@ 2022-03-18 10:37   ` xuyang2018.jy
  2022-03-18 22:12       ` Darrick J. Wong
  0 siblings, 1 reply; 12+ messages in thread
From: xuyang2018.jy @ 2022-03-18 10:37 UTC (permalink / raw)
  To: ltp, Jan Kara, Darrick J. Wong; +Cc: Martin Doucha

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);
>

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [LTP] [RFC PATCH 1/1] creat09: Fix on more restrictive umask
  2022-03-18 10:37   ` xuyang2018.jy
@ 2022-03-18 22:12       ` Darrick J. Wong
  0 siblings, 0 replies; 12+ 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] 12+ messages in thread

* Re: [LTP] [RFC PATCH 1/1] creat09: Fix on more restrictive umask
@ 2022-03-18 22:12       ` Darrick J. Wong
  0 siblings, 0 replies; 12+ messages in thread
From: Darrick J. Wong @ 2022-03-18 22:12 UTC (permalink / raw)
  To: xuyang2018.jy; +Cc: xfs, Jan Kara, Martin Doucha, ltp

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);
> >

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [LTP] [RFC PATCH 1/1] creat09: Fix on more restrictive umask
  2022-03-18 22:12       ` Darrick J. Wong
@ 2022-03-22  6:28         ` xuyang2018.jy
  -1 siblings, 0 replies; 12+ messages in thread
From: xuyang2018.jy @ 2022-03-22  6:28 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: xfs, Jan Kara, Martin Doucha, ltp

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);
>>>

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [LTP] [RFC PATCH 1/1] creat09: Fix on more restrictive umask
@ 2022-03-22  6:28         ` xuyang2018.jy
  0 siblings, 0 replies; 12+ 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] 12+ messages in thread

* Re: [LTP] [RFC PATCH 1/1] creat09: Fix on more restrictive umask
  2022-03-14 19:12 [LTP] [RFC PATCH 1/1] creat09: Fix on more restrictive umask Petr Vorel
  2022-03-18  7:59 ` xuyang2018.jy
@ 2022-07-06  9:37 ` xuyang2018.jy
  2022-07-07 18:04   ` Petr Vorel
  1 sibling, 1 reply; 12+ messages in thread
From: xuyang2018.jy @ 2022-07-06  9:37 UTC (permalink / raw)
  To: Petr Vorel; +Cc: Martin Doucha, ltp

Hi Petr

Share some process with you...

I have rejected this patch and a kernel patchset[1] is plan to fix this 
but doesn't merged to upstream kernel due to none continue to review 
this(It is about one month since v9...)

TBH, this case only tests simple creat file with open but miss
open with O_TMPFILE[2]
mknodat[3]

setgid strip logic is not only affected by umask but also include acl 
operations[4].


[1]https://patchwork.kernel.org/project/linux-fsdevel/list/?series=643645
[2]https://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git/commit/?id=87cf32ad3fa234e3d5ec501e0f86516bef91d805
[3]https://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git/commit/?id=5328ab947ff4d2d492a6ac90b22d31ab02637bd8
[4]https://patchwork.kernel.org/project/fstests/list/?series=643643&state=%2A&archive=both

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);

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [LTP] [RFC PATCH 1/1] creat09: Fix on more restrictive umask
  2022-07-06  9:37 ` xuyang2018.jy
@ 2022-07-07 18:04   ` Petr Vorel
  2022-07-08 14:37     ` Christian Brauner
  0 siblings, 1 reply; 12+ messages in thread
From: Petr Vorel @ 2022-07-07 18:04 UTC (permalink / raw)
  To: xuyang2018.jy; +Cc: Christian Brauner, Martin Doucha, ltp, Al Viro

Hi Xu,

> Hi Petr

> Share some process with you...

> I have rejected this patch and a kernel patchset[1] is plan to fix this 
> but doesn't merged to upstream kernel due to none continue to review 
> this(It is about one month since v9...)

> TBH, this case only tests simple creat file with open but miss
> open with O_TMPFILE[2]
> mknodat[3]

> setgid strip logic is not only affected by umask but also include acl 
> operations[4].

Thanks a lot for an update.
Al, Christian, Amir, could you please have look on v9?

Kind regards,
Petr


> [1]https://patchwork.kernel.org/project/linux-fsdevel/list/?series=643645
> [2]https://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git/commit/?id=87cf32ad3fa234e3d5ec501e0f86516bef91d805
> [3]https://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git/commit/?id=5328ab947ff4d2d492a6ac90b22d31ab02637bd8
> [4]https://patchwork.kernel.org/project/fstests/list/?series=643643&state=%2A&archive=both

> 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);

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [LTP] [RFC PATCH 1/1] creat09: Fix on more restrictive umask
  2022-07-07 18:04   ` Petr Vorel
@ 2022-07-08 14:37     ` Christian Brauner
  2022-07-11  3:56       ` xuyang2018.jy
  2022-07-11  5:25       ` Petr Vorel
  0 siblings, 2 replies; 12+ messages in thread
From: Christian Brauner @ 2022-07-08 14:37 UTC (permalink / raw)
  To: Petr Vorel; +Cc: Martin Doucha, ltp, Al Viro

On Thu, Jul 07, 2022 at 08:04:18PM +0200, Petr Vorel wrote:
> Hi Xu,
> 
> > Hi Petr
> 
> > Share some process with you...
> 
> > I have rejected this patch and a kernel patchset[1] is plan to fix this 
> > but doesn't merged to upstream kernel due to none continue to review 
> > this(It is about one month since v9...)
> 
> > TBH, this case only tests simple creat file with open but miss
> > open with O_TMPFILE[2]
> > mknodat[3]
> 
> > setgid strip logic is not only affected by umask but also include acl 
> > operations[4].
> 
> Thanks a lot for an update.
> Al, Christian, Amir, could you please have look on v9?

Afaict, I reviewed v9 and left comments and had pointed out things that
needed to be changed. So I'd wait for v10 (I'm on vacation next week
though.).

Christian

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [LTP] [RFC PATCH 1/1] creat09: Fix on more restrictive umask
  2022-07-08 14:37     ` Christian Brauner
@ 2022-07-11  3:56       ` xuyang2018.jy
  2022-07-11  5:25       ` Petr Vorel
  1 sibling, 0 replies; 12+ messages in thread
From: xuyang2018.jy @ 2022-07-11  3:56 UTC (permalink / raw)
  To: Christian Brauner, Petr Vorel; +Cc: Martin Doucha, ltp, Al Viro

on 2022/07/08 22:37, Christian Brauner wrote:
> On Thu, Jul 07, 2022 at 08:04:18PM +0200, Petr Vorel wrote:
>> Hi Xu,
>>
>>> Hi Petr
>>
>>> Share some process with you...
>>
>>> I have rejected this patch and a kernel patchset[1] is plan to fix this
>>> but doesn't merged to upstream kernel due to none continue to review
>>> this(It is about one month since v9...)
>>
>>> TBH, this case only tests simple creat file with open but miss
>>> open with O_TMPFILE[2]
>>> mknodat[3]
>>
>>> setgid strip logic is not only affected by umask but also include acl
>>> operations[4].
>>
>> Thanks a lot for an update.
>> Al, Christian, Amir, could you please have look on v9?
> 
> Afaict, I reviewed v9 and left comments and had pointed out things that
> needed to be changed. So I'd wait for v10 (I'm on vacation next week
> though.).

Oh, I missed that email and will think how to move logic into vfs_* as 
you suggested.

Best Regards
Yang Xu

> 
> Christian

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [LTP] [RFC PATCH 1/1] creat09: Fix on more restrictive umask
  2022-07-08 14:37     ` Christian Brauner
  2022-07-11  3:56       ` xuyang2018.jy
@ 2022-07-11  5:25       ` Petr Vorel
  1 sibling, 0 replies; 12+ messages in thread
From: Petr Vorel @ 2022-07-11  5:25 UTC (permalink / raw)
  To: Christian Brauner; +Cc: Martin Doucha, ltp, Al Viro

> On Thu, Jul 07, 2022 at 08:04:18PM +0200, Petr Vorel wrote:
> > Hi Xu,

> > > Hi Petr

> > > Share some process with you...

> > > I have rejected this patch and a kernel patchset[1] is plan to fix this 
> > > but doesn't merged to upstream kernel due to none continue to review 
> > > this(It is about one month since v9...)

> > > TBH, this case only tests simple creat file with open but miss
> > > open with O_TMPFILE[2]
> > > mknodat[3]

> > > setgid strip logic is not only affected by umask but also include acl 
> > > operations[4].

> > Thanks a lot for an update.
> > Al, Christian, Amir, could you please have look on v9?

> Afaict, I reviewed v9 and left comments and had pointed out things that
> needed to be changed. So I'd wait for v10 (I'm on vacation next week
> though.).

Thanks a lot, sorry for overlooking your change requests!

Kind regards,
Petr

> Christian

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2022-07-13  9:39 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-14 19:12 [LTP] [RFC PATCH 1/1] creat09: Fix on more restrictive umask Petr Vorel
2022-03-18  7:59 ` xuyang2018.jy
2022-03-18 10:37   ` xuyang2018.jy
2022-03-18 22:12     ` Darrick J. Wong
2022-03-18 22:12       ` Darrick J. Wong
2022-03-22  6:28       ` xuyang2018.jy
2022-03-22  6:28         ` xuyang2018.jy
2022-07-06  9:37 ` xuyang2018.jy
2022-07-07 18:04   ` Petr Vorel
2022-07-08 14:37     ` Christian Brauner
2022-07-11  3:56       ` xuyang2018.jy
2022-07-11  5:25       ` Petr Vorel

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.