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

[Petr Vorel <pvorel@suse.cz>]

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>

> [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.
I also thought that .test_variants could be used for this kind of setup.

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"},
>  		{}
>  	},
>  };

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