[LTP] [PATCH v1 1/2] syscalls/creat09: Add umask condition

[xuyang2018.jy@fujitsu.com]

Hi Petr

> Hi Xu,
> 
> [ Cc Martin ]
> 
>> A kernel patch set that fix setgid strip logic under umask(S_IXGRP) found by
>> this case has been merged into Christian Brauner for-next branch[1].
> 
>> I guess it will be merged into linux-next branch.
> 
>> I will add acl and umask test[2][3] in xfstests because there is more suitable
>> to do this.
> 
>> Here I just only add umask condition simply.
> 
> Reviewed-by: Petr Vorel <pvorel@suse.cz>

Thanks.

> 
>> [1]https://git.kernel.org/pub/scm/linux/kernel/git/brauner/linux.git/log/?h=for-next
>> [2]https://www.spinics.net/lists/fstests/msg19554.html
>> [3]https://www.spinics.net/lists/fstests/msg19555.html
> 
>> Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
>> ---
>>   testcases/kernel/syscalls/creat/creat09.c | 27 +++++++++++++++++++++--
>>   1 file changed, 25 insertions(+), 2 deletions(-)
> 
>> diff --git a/testcases/kernel/syscalls/creat/creat09.c b/testcases/kernel/syscalls/creat/creat09.c
>> index bed7bddb0..04bc98d11 100644
>> --- a/testcases/kernel/syscalls/creat/creat09.c
>> +++ b/testcases/kernel/syscalls/creat/creat09.c
>> @@ -28,6 +28,16 @@
>>    *  Date:   Fri Jan 22 16:48:18 2021 -0800
>>    *
>>    *  xfs: fix up non-directory creation in SGID directories
>> + *
>> + * When use acl or umask, it still has bug.
>> + *
>> + * Fixed in:
>> + *
>> + *  commit 1639a49ccdce58ea248841ed9b23babcce6dbb0b
>> + *  Author: Yang Xu <xuyang2018.jy@fujitsu.com>
>> + *  Date:   Thu July 14 14:11:27 2022 +0800
>> + *
>> + *  fs: move S_ISGID stripping into the vfs_*() helpers
>>    */
> 
>>   #include <stdlib.h>
>> @@ -94,8 +104,19 @@ static void file_test(const char *name)
>>   		tst_res(TPASS, "%s: Setgid bit not set", name);
>>   }
> 
>> -static void run(void)
>> +static void run(unsigned int n)
>>   {
>> +	switch (n) {
>> +	case 0:
>> +		umask(0);
>> +		tst_res(TINFO, "under umask(0) situation");
>> +		break;
>> +	case 1:
>> +		umask(S_IXGRP);
>> +		tst_res(TINFO, "under umask(S_IXGRP) situation");
>> +		break;
>> +	}
> nit: Maybe just use if for to cases.

Of course, will use if in v2.

> I also thought that .test_variants could be used for this kind of setup.

In here, I don't agree because we usually use it for similar syscalls or 
glibc wrappesr but not the test condition.

Best Regards
Yang xu
> 
> Kind regards,
> Petr
> 
>> +
>>   	fd = SAFE_CREAT(CREAT_FILE, MODE_SGID);
>>   	SAFE_CLOSE(fd);
>>   	file_test(CREAT_FILE);
>> @@ -115,13 +136,14 @@ static void cleanup(void)
>>   }
> 
>>   static struct tst_test test = {
>> -	.test_all = run,
>> +	.test = run,
>>   	.setup = setup,
>>   	.cleanup = cleanup,
>>   	.needs_root = 1,
>>   	.all_filesystems = 1,
>>   	.mount_device = 1,
>>   	.mntpoint = MNTPOINT,
>> +	.tcnt = 2,
>>   	.skip_filesystems = (const char*[]) {
>>   		"exfat",
>>   		"ntfs",
>> @@ -132,6 +154,7 @@ static struct tst_test test = {
>>   		{"linux-git", "0fa3ecd87848"},
>>   		{"CVE", "2018-13405"},
>>   		{"linux-git", "01ea173e103e"},
>> +		{"linux-git", "1639a49ccdce"},
>>   		{}
>>   	},
>>   };