Re: [LTP] [PATCH v4 1/3] syscalls/creat09: Add umask test condition

Re: [LTP] [PATCH v4 1/3] syscalls/creat09: Add umask test condition

[xuyang2018.jy]

Hi All

> A kernel patch set that fix setgid strip logic under umask(S_IXGRP) found by
> this case has been merged into 6.0-rc1 kernel[1].
> 
> I will add acl and umask test[2] in xfstests because there is more suitable
> to do this.
> 
> Here I just only add umask test condition simply.
> 
> [1]https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=1639a49c
> [2]https://patchwork.kernel.org/project/fstests/list/?series=662984

I Should update the [2] url because the corresponding patch is in 
xfstest for-next branch.

[2]https://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git/commit/?h=for-next&id=c875434286737e792d200cda2eb679e2a8441837

Also miss Acked-by: Christian Brauner (Microsoft) <brauner@kernel.org> tag.

Best Regards
Yang Xu
> 
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
> Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
> ---
>   testcases/kernel/syscalls/creat/creat09.c | 30 +++++++++++++++++++++--
>   1 file changed, 28 insertions(+), 2 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/creat/creat09.c b/testcases/kernel/syscalls/creat/creat09.c
> index bed7bddb0..d583cceca 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>
> @@ -47,6 +57,14 @@
>   static gid_t free_gid;
>   static int fd = -1;
>   
> +static struct tcase {
> +	const char *msg;
> +	int mask;
> +} tcases[] = {
> +	{"under umask(0) situation", 0},
> +	{"under umask(S_IXGRP) situation", S_IXGRP}
> +};
> +
>   static void setup(void)
>   {
>   	struct stat buf;
> @@ -94,8 +112,14 @@ 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)
>   {
> +	struct tcase *tc = &tcases[n];
> +
> +	umask(tc->mask);
> +	tst_res(TINFO, "Testing setgid behaviour when creating file %s",
> +			tc->msg);
> +
>   	fd = SAFE_CREAT(CREAT_FILE, MODE_SGID);
>   	SAFE_CLOSE(fd);
>   	file_test(CREAT_FILE);
> @@ -115,13 +139,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 = ARRAY_SIZE(tcases),
>   	.skip_filesystems = (const char*[]) {
>   		"exfat",
>   		"ntfs",
> @@ -132,6 +157,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

[LTP] [PATCH v4 1/3] syscalls/creat09: Add umask test condition

[Yang Xu]

A kernel patch set that fix setgid strip logic under umask(S_IXGRP) found by
this case has been merged into 6.0-rc1 kernel[1].

I will add acl and umask test[2] in xfstests because there is more suitable
to do this.

Here I just only add umask test condition simply.

[1]https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=1639a49c
[2]https://patchwork.kernel.org/project/fstests/list/?series=662984

Reviewed-by: Petr Vorel <pvorel@suse.cz>
Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
 testcases/kernel/syscalls/creat/creat09.c | 30 +++++++++++++++++++++--
 1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/testcases/kernel/syscalls/creat/creat09.c b/testcases/kernel/syscalls/creat/creat09.c
index bed7bddb0..d583cceca 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>
@@ -47,6 +57,14 @@
 static gid_t free_gid;
 static int fd = -1;
 
+static struct tcase {
+	const char *msg;
+	int mask;
+} tcases[] = {
+	{"under umask(0) situation", 0},
+	{"under umask(S_IXGRP) situation", S_IXGRP}
+};
+
 static void setup(void)
 {
 	struct stat buf;
@@ -94,8 +112,14 @@ 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)
 {
+	struct tcase *tc = &tcases[n];
+
+	umask(tc->mask);
+	tst_res(TINFO, "Testing setgid behaviour when creating file %s",
+			tc->msg);
+
 	fd = SAFE_CREAT(CREAT_FILE, MODE_SGID);
 	SAFE_CLOSE(fd);
 	file_test(CREAT_FILE);
@@ -115,13 +139,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 = ARRAY_SIZE(tcases),
 	.skip_filesystems = (const char*[]) {
 		"exfat",
 		"ntfs",
@@ -132,6 +157,7 @@ static struct tst_test test = {
 		{"linux-git", "0fa3ecd87848"},
 		{"CVE", "2018-13405"},
 		{"linux-git", "01ea173e103e"},
+		{"linux-git", "1639a49ccdce"},
 		{}
 	},
 };
-- 
2.23.0


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

[LTP] [PATCH v4 2/3] tst_safe_file_at: Add SAFE_FSTATAT macro

[Yang Xu]

Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
 include/tst_safe_file_at.h |  7 +++++++
 lib/tst_safe_file_at.c     | 20 ++++++++++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/include/tst_safe_file_at.h b/include/tst_safe_file_at.h
index e253198e6..082a5acd9 100644
--- a/include/tst_safe_file_at.h
+++ b/include/tst_safe_file_at.h
@@ -29,6 +29,9 @@
 	safe_fchownat(__FILE__, __LINE__,				\
 			(dirfd), (path), (owner), (group), (flags))
 
+#define SAFE_FSTATAT(dirfd, path, statbuf, flags)			\
+	safe_fstatat(__FILE__, __LINE__, (dirfd), (path), (statbuf), (flags))
+
 const char *tst_decode_fd(const int fd)
 			  __attribute__((warn_unused_result));
 
@@ -67,4 +70,8 @@ int safe_fchownat(const char *const file, const int lineno,
 		  gid_t group, int flags)
 		  __attribute__ ((nonnull));
 
+int safe_fstatat(const char *const file, const int lineno,
+		 const int dirfd, const char *const path, struct stat *statbuf,
+		 int flags)
+		 __attribute__ ((nonnull));
 #endif
diff --git a/lib/tst_safe_file_at.c b/lib/tst_safe_file_at.c
index 6370a68e5..f530dc349 100644
--- a/lib/tst_safe_file_at.c
+++ b/lib/tst_safe_file_at.c
@@ -215,3 +215,23 @@ int safe_fchownat(const char *const file, const int lineno,
 
 	return rval;
 }
+
+int safe_fstatat(const char *const file, const int lineno,
+		 const int dirfd, const char *const path, struct stat *statbuf, int flags)
+{
+	int rval;
+
+	rval = fstatat(dirfd, path, statbuf, flags);
+
+	if (rval == -1) {
+		tst_brk_(file, lineno, TBROK | TERRNO,
+			 "fstatat(%d<%s>, '%s', %p, %d) failed", dirfd,
+			 tst_decode_fd(dirfd), path, statbuf, flags);
+	} else if (rval) {
+		tst_brk_(file, lineno, TBROK | TERRNO,
+			 "Invalid fstatat(%d<%s>, '%s', %p, %d) return value %d",
+			 dirfd, tst_decode_fd(dirfd), path, statbuf, flags, rval);
+	}
+
+	return rval;
+}
-- 
2.23.0


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

[LTP] [PATCH v4 3/3] syscalls/openat: Add new regression test when using open(O_TMPFILE) under umask

[Yang Xu]

Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
 runtest/syscalls                            |   2 +-
 testcases/kernel/syscalls/openat/.gitignore |   1 +
 testcases/kernel/syscalls/openat/openat04.c | 181 ++++++++++++++++++++
 3 files changed, 183 insertions(+), 1 deletion(-)
 create mode 100644 testcases/kernel/syscalls/openat/openat04.c

diff --git a/runtest/syscalls b/runtest/syscalls
index 9d58e0aa1..cd38a4ddf 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -919,10 +919,10 @@ open12 open12
 open13 open13
 open14 open14
 
-#openat test cases
 openat01 openat01
 openat02 openat02
 openat03 openat03
+openat04 openat04
 
 openat201 openat201
 openat202 openat202
diff --git a/testcases/kernel/syscalls/openat/.gitignore b/testcases/kernel/syscalls/openat/.gitignore
index 2928dae22..2d15872ab 100644
--- a/testcases/kernel/syscalls/openat/.gitignore
+++ b/testcases/kernel/syscalls/openat/.gitignore
@@ -2,3 +2,4 @@
 /openat02
 /openat02_child
 /openat03
+/openat04
diff --git a/testcases/kernel/syscalls/openat/openat04.c b/testcases/kernel/syscalls/openat/openat04.c
new file mode 100644
index 000000000..1fbe51c60
--- /dev/null
+++ b/testcases/kernel/syscalls/openat/openat04.c
@@ -0,0 +1,181 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2022 FUJITSU LIMITED. All rights reserved.
+ * Author: Yang Xu <xuyang2018.jy@fujitsu.com>
+ */
+
+/*\
+ * [Description]
+ *
+ * Check setgid strip logic whether works correctly when creating tmpfile under
+ * filesystem without POSIX ACL supported(by using noacl mount option). Test it
+ * with umask S_IXGRP and also check file mode whether has filtered S_IXGRP.
+ *
+ * Fixed in:
+ *
+ *  commit ac6800e279a22b28f4fc21439843025a0d5bf03e
+ *  Author: Yang Xu <xuyang2018.jy@fujitsu.com>
+ *  Date:   Thu July 14 14:11:26 2022 +0800
+ *
+ *  fs: Add missing umask strip in vfs_tmpfile
+ *
+ * The most code is pasted form creat09.c.
+ */
+
+#define _GNU_SOURCE
+#include <stdlib.h>
+#include <sys/types.h>
+#include <pwd.h>
+#include <sys/mount.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <stdio.h>
+#include "tst_test.h"
+#include "tst_uid.h"
+#include "tst_safe_file_at.h"
+
+#define MODE_RWX        0777
+#define MODE_SGID       (S_ISGID|0777)
+#define MNTPOINT	"mntpoint"
+#define WORKDIR		MNTPOINT "/testdir"
+#define OPEN_FILE	"open.tmp"
+
+static gid_t free_gid;
+static int tmpfile_fd = -1, dir_fd = -1, mount_flag;
+static struct passwd *ltpuser;
+
+static void do_mount(const char *source, const char *target,
+	const char *filesystemtype, unsigned long mountflags,
+	const void *data)
+{
+	TEST(mount(source, target, filesystemtype, mountflags, data));
+
+	if (TST_RET == -1 && TST_ERR == EINVAL)
+		tst_brk(TCONF, "Kernel does not support noacl feature");
+
+	if (TST_RET == -1) {
+		tst_brk(TBROK | TTERRNO, "mount(%s, %s, %s, %lu, %p) failed",
+			source, target, filesystemtype, mountflags, data);
+	}
+
+	if (TST_RET)
+		tst_brk(TBROK, "Invalid mount return value %ld", TST_RET);
+
+	mount_flag = 1;
+}
+
+static void open_tmpfile_supported(int dirfd)
+{
+	TEST(openat(dirfd, ".", O_TMPFILE | O_RDWR, S_IXGRP | S_ISGID));
+
+	if (TST_RET == -1) {
+		if (errno == ENOTSUP)
+			tst_brk(TCONF, "fs doesn't support O_TMPFILE");
+		else
+			tst_brk(TBROK | TTERRNO, "openat(%d, O_TMPFILE) failed", dirfd);
+	}
+
+	if (TST_RET < 0)
+		tst_brk(TBROK, "Invalid openat return value %ld", TST_RET);
+
+	SAFE_CLOSE(TST_RET);
+}
+
+static void setup(void)
+{
+	struct stat buf;
+
+	ltpuser = SAFE_GETPWNAM("nobody");
+
+	do_mount(tst_device->dev, MNTPOINT, tst_device->fs_type, 0, "noacl");
+
+	tst_res(TINFO, "User nobody: uid = %d, gid = %d", (int)ltpuser->pw_uid,
+		(int)ltpuser->pw_gid);
+	free_gid = tst_get_free_gid(ltpuser->pw_gid);
+
+	/* Create directories and set permissions */
+	SAFE_MKDIR(WORKDIR, MODE_RWX);
+	dir_fd = SAFE_OPEN(WORKDIR, O_RDONLY, O_DIRECTORY);
+	open_tmpfile_supported(dir_fd);
+
+	SAFE_CHOWN(WORKDIR, ltpuser->pw_uid, free_gid);
+	SAFE_CHMOD(WORKDIR, MODE_SGID);
+	SAFE_STAT(WORKDIR, &buf);
+
+	if (!(buf.st_mode & S_ISGID))
+		tst_brk(TBROK, "%s: Setgid bit not set", WORKDIR);
+
+	if (buf.st_gid != free_gid) {
+		tst_brk(TBROK, "%s: Incorrect group, %u != %u", WORKDIR,
+			buf.st_gid, free_gid);
+	}
+
+	/* Switch user */
+	SAFE_SETGID(ltpuser->pw_gid);
+	SAFE_SETREUID(-1, ltpuser->pw_uid);
+}
+
+static void file_test(int dfd, const char *path, int flags)
+{
+	struct stat buf;
+
+	SAFE_FSTATAT(dfd, path, &buf, flags);
+
+	TST_EXP_EQ_LI(buf.st_gid, free_gid);
+
+	if (buf.st_mode & S_ISGID)
+		tst_res(TFAIL, "%s: Setgid bit is set", path);
+	else
+		tst_res(TPASS, "%s: Setgid bit not set", path);
+
+	if (buf.st_mode & S_IXGRP)
+		tst_res(TFAIL, "%s: S_IXGRP bit is set", path);
+	else
+		tst_res(TPASS, "%s: S_IXGRP bit is not set", path);
+}
+
+static void run(void)
+{
+	char path[PATH_MAX];
+
+	umask(S_IXGRP);
+	tmpfile_fd = SAFE_OPENAT(dir_fd, ".", O_TMPFILE | O_RDWR, MODE_SGID);
+	snprintf(path, PATH_MAX, "/proc/self/fd/%d", tmpfile_fd);
+	SAFE_LINKAT(AT_FDCWD, path, dir_fd, OPEN_FILE, AT_SYMLINK_FOLLOW);
+	file_test(dir_fd, OPEN_FILE, 0);
+	SAFE_CLOSE(tmpfile_fd);
+	/* Cleanup between loops */
+	tst_purge_dir(WORKDIR);
+}
+
+static void cleanup(void)
+{
+	SAFE_SETREUID(-1, 0);
+
+	if (tmpfile_fd >= 0)
+		SAFE_CLOSE(tmpfile_fd);
+	if (dir_fd >= 0)
+		SAFE_CLOSE(dir_fd);
+	if (mount_flag && tst_umount(MNTPOINT))
+		tst_res(TWARN | TERRNO, "umount(%s)", MNTPOINT);
+}
+
+static struct tst_test test = {
+	.test_all = run,
+	.setup = setup,
+	.cleanup = cleanup,
+	.needs_root = 1,
+	.all_filesystems = 1,
+	.format_device = 1,
+	.mntpoint = MNTPOINT,
+	.skip_filesystems = (const char*[]) {
+		"exfat",
+		"ntfs",
+		"vfat",
+		NULL
+	},
+	.tags = (const struct tst_tag[]) {
+		{"linux-git", "ac6800e279a2"},
+		{}
+	},
+};
-- 
2.23.0


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

Re: [LTP] [PATCH v4 1/3] syscalls/creat09: Add umask test condition

[Cyril Hrubis]

Hi!
> diff --git a/testcases/kernel/syscalls/creat/creat09.c b/testcases/kernel/syscalls/creat/creat09.c
> index bed7bddb0..d583cceca 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>
> @@ -47,6 +57,14 @@
>  static gid_t free_gid;
>  static int fd = -1;
>  
> +static struct tcase {
> +	const char *msg;
> +	int mask;
> +} tcases[] = {
> +	{"under umask(0) situation", 0},
> +	{"under umask(S_IXGRP) situation", S_IXGRP}
> +};
> +
>  static void setup(void)
>  {
>  	struct stat buf;
> @@ -94,8 +112,14 @@ 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)
>  {
> +	struct tcase *tc = &tcases[n];
> +
> +	umask(tc->mask);
> +	tst_res(TINFO, "Testing setgid behaviour when creating file %s",
> +			tc->msg);

This can be shorter and more to the point, something as:

	tst_res(TINFO, "File created with %s", tc->msg);

And the msg could be just "umask(0)" and "umask(S_IXGRP)".


Otherwise it's fine and I can fix the messages before applying if you
want.

>  	fd = SAFE_CREAT(CREAT_FILE, MODE_SGID);
>  	SAFE_CLOSE(fd);
>  	file_test(CREAT_FILE);
> @@ -115,13 +139,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 = ARRAY_SIZE(tcases),
>  	.skip_filesystems = (const char*[]) {
>  		"exfat",
>  		"ntfs",
> @@ -132,6 +157,7 @@ static struct tst_test test = {
>  		{"linux-git", "0fa3ecd87848"},
>  		{"CVE", "2018-13405"},
>  		{"linux-git", "01ea173e103e"},
> +		{"linux-git", "1639a49ccdce"},
>  		{}
>  	},
>  };
> -- 
> 2.23.0
> 

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH v4 2/3] tst_safe_file_at: Add SAFE_FSTATAT macro

[Cyril Hrubis]

Hi!
Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH v4 1/3] syscalls/creat09: Add umask test condition

[xuyang2018.jy]

Hi Cyril

> Hi!
>> diff --git a/testcases/kernel/syscalls/creat/creat09.c b/testcases/kernel/syscalls/creat/creat09.c
>> index bed7bddb0..d583cceca 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>
>> @@ -47,6 +57,14 @@
>>   static gid_t free_gid;
>>   static int fd = -1;
>>   
>> +static struct tcase {
>> +	const char *msg;
>> +	int mask;
>> +} tcases[] = {
>> +	{"under umask(0) situation", 0},
>> +	{"under umask(S_IXGRP) situation", S_IXGRP}
>> +};
>> +
>>   static void setup(void)
>>   {
>>   	struct stat buf;
>> @@ -94,8 +112,14 @@ 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)
>>   {
>> +	struct tcase *tc = &tcases[n];
>> +
>> +	umask(tc->mask);
>> +	tst_res(TINFO, "Testing setgid behaviour when creating file %s",
>> +			tc->msg);
> 
> This can be shorter and more to the point, something as:
> 
> 	tst_res(TINFO, "File created with %s", tc->msg);
> 
> And the msg could be just "umask(0)" and "umask(S_IXGRP)".
> 
> 
> Otherwise it's fine and I can fix the messages before applying if you
> want.

Yes.  Please apply it with fix the messages. Thanks.


Best Regards
Yang Xu
> 
>>   	fd = SAFE_CREAT(CREAT_FILE, MODE_SGID);
>>   	SAFE_CLOSE(fd);
>>   	file_test(CREAT_FILE);
>> @@ -115,13 +139,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 = ARRAY_SIZE(tcases),
>>   	.skip_filesystems = (const char*[]) {
>>   		"exfat",
>>   		"ntfs",
>> @@ -132,6 +157,7 @@ static struct tst_test test = {
>>   		{"linux-git", "0fa3ecd87848"},
>>   		{"CVE", "2018-13405"},
>>   		{"linux-git", "01ea173e103e"},
>> +		{"linux-git", "1639a49ccdce"},
>>   		{}
>>   	},
>>   };
>> -- 
>> 2.23.0
>>
> 

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

Re: [LTP] [PATCH v4 1/3] syscalls/creat09: Add umask test condition

[xuyang2018.jy]

Hi Cyril

> Hi Cyril
> 
>> Hi!
>>> diff --git a/testcases/kernel/syscalls/creat/creat09.c b/testcases/kernel/syscalls/creat/creat09.c
>>> index bed7bddb0..d583cceca 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>
>>> @@ -47,6 +57,14 @@
>>>    static gid_t free_gid;
>>>    static int fd = -1;
>>>    
>>> +static struct tcase {
>>> +	const char *msg;
>>> +	int mask;
>>> +} tcases[] = {
>>> +	{"under umask(0) situation", 0},
>>> +	{"under umask(S_IXGRP) situation", S_IXGRP}
>>> +};
>>> +
>>>    static void setup(void)
>>>    {
>>>    	struct stat buf;
>>> @@ -94,8 +112,14 @@ 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)
>>>    {
>>> +	struct tcase *tc = &tcases[n];
>>> +
>>> +	umask(tc->mask);
>>> +	tst_res(TINFO, "Testing setgid behaviour when creating file %s",
>>> +			tc->msg);
>>
>> This can be shorter and more to the point, something as:
>>
>> 	tst_res(TINFO, "File created with %s", tc->msg);
>>
>> And the msg could be just "umask(0)" and "umask(S_IXGRP)".
>>
>>
>> Otherwise it's fine and I can fix the messages before applying if you
>> want.
> 
> Yes.  Please apply it with fix the messages. Thanks.

I guess you miss this patchset. so ping again.


Best Regards
Yang Xu
> 
> 
> Best Regards
> Yang Xu
>>
>>>    	fd = SAFE_CREAT(CREAT_FILE, MODE_SGID);
>>>    	SAFE_CLOSE(fd);
>>>    	file_test(CREAT_FILE);
>>> @@ -115,13 +139,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 = ARRAY_SIZE(tcases),
>>>    	.skip_filesystems = (const char*[]) {
>>>    		"exfat",
>>>    		"ntfs",
>>> @@ -132,6 +157,7 @@ static struct tst_test test = {
>>>    		{"linux-git", "0fa3ecd87848"},
>>>    		{"CVE", "2018-13405"},
>>>    		{"linux-git", "01ea173e103e"},
>>> +		{"linux-git", "1639a49ccdce"},
>>>    		{}
>>>    	},
>>>    };
>>> -- 
>>> 2.23.0
>>>
>>
> 

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

Re: [LTP] [PATCH v4 1/3] syscalls/creat09: Add umask test condition

[Cyril Hrubis]

Hi!
> > Yes.  Please apply it with fix the messages. Thanks.
> 
> I guess you miss this patchset. so ping again.

Sorry for the delay.

I just did quick test and I got failures on XFS on kernel 5.19, wasn't
this supposed to be fixed in 5.19?

...
creat09.c:106: TPASS: mntpoint/testdir/creat.tmp: Owned by correct group
creat09.c:112: TPASS: mntpoint/testdir/creat.tmp: Setgid bit not set
creat09.c:106: TPASS: mntpoint/testdir/open.tmp: Owned by correct group
creat09.c:112: TPASS: mntpoint/testdir/open.tmp: Setgid bit not set
creat09.c:120: TINFO: File created with umask(S_IXGRP)
creat09.c:106: TPASS: mntpoint/testdir/creat.tmp: Owned by correct group
creat09.c:110: TFAIL: mntpoint/testdir/creat.tmp: Setgid bit is set
creat09.c:106: TPASS: mntpoint/testdir/open.tmp: Owned by correct group
creat09.c:110: TFAIL: mntpoint/testdir/open.tmp: Setgid bit is set
...

$ uname -r
5.19.0

$ git describe 1639a49ccdce
v5.19-rc7-3-g1639a49ccdce


Is there any in-flight patch for v6.0 that fixes this for XFS?

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH v4 1/3] syscalls/creat09: Add umask test condition

[Christian Brauner]

On Wed, Sep 21, 2022 at 03:55:56PM +0200, Cyril Hrubis wrote:
> Hi!
> > > Yes.  Please apply it with fix the messages. Thanks.
> > 
> > I guess you miss this patchset. so ping again.
> 
> Sorry for the delay.
> 
> I just did quick test and I got failures on XFS on kernel 5.19, wasn't
> this supposed to be fixed in 5.19?

No, this is fixed starting with v6.0-rc1 :)

> 
> ...
> creat09.c:106: TPASS: mntpoint/testdir/creat.tmp: Owned by correct group
> creat09.c:112: TPASS: mntpoint/testdir/creat.tmp: Setgid bit not set
> creat09.c:106: TPASS: mntpoint/testdir/open.tmp: Owned by correct group
> creat09.c:112: TPASS: mntpoint/testdir/open.tmp: Setgid bit not set
> creat09.c:120: TINFO: File created with umask(S_IXGRP)
> creat09.c:106: TPASS: mntpoint/testdir/creat.tmp: Owned by correct group
> creat09.c:110: TFAIL: mntpoint/testdir/creat.tmp: Setgid bit is set
> creat09.c:106: TPASS: mntpoint/testdir/open.tmp: Owned by correct group
> creat09.c:110: TFAIL: mntpoint/testdir/open.tmp: Setgid bit is set
> ...
> 
> $ uname -r
> 5.19.0
> 
> $ git describe 1639a49ccdce
> v5.19-rc7-3-g1639a49ccdce
> 
> 
> Is there any in-flight patch for v6.0 that fixes this for XFS?

You're looking for
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=426b4ca2d6a5ab51f6b6175d06e4f8ddea434cdf

Hope this helps!
Christian

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

Re: [LTP] [PATCH v4 1/3] syscalls/creat09: Add umask test condition

[Cyril Hrubis]

Hi!
> > Is there any in-flight patch for v6.0 that fixes this for XFS?
> 
> You're looking for
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=426b4ca2d6a5ab51f6b6175d06e4f8ddea434cdf

That's the missing piece!

I've added this patch to the test tags and pushed, thanks everyone.

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH v4 3/3] syscalls/openat: Add new regression test when using open(O_TMPFILE) under umask

[Cyril Hrubis]

Hi!
And this one is probably missing some kernel commit tags too, since the
only that that is attached is supposedly in 5.19 but the test still
fails.

I supposed that we should add the 426b4ca2d6a5 to the test tags here as
well, right?

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH v4 3/3] syscalls/openat: Add new regression test when using open(O_TMPFILE) under umask

[xuyang2018.jy]

Hi Cyril

>Hi!
> And this one is probably missing some kernel commit tags too, since the only that that is attached is supposedly in 5.19 but the test still fails.

> I supposed that we should add the 426b4ca2d6a5 to the test tags here as well, right?

Yes, BTW,  I usually use git tag --contains command, so I know this kernel fix is belong to 6.0.

git tag --contains ac6800e27
v6.0-rc1
v6.0-rc2

Best  Regards
Yang Xu

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

Re: [LTP] [PATCH v4 3/3] syscalls/openat: Add new regression test when using open(O_TMPFILE) under umask

[Cyril Hrubis]

Hi!
> > And this one is probably missing some kernel commit tags too, since the only that that is attached is supposedly in 5.19 but the test still fails.
> 
> > I supposed that we should add the 426b4ca2d6a5 to the test tags here as well, right?
> 
> Yes, BTW,  I usually use git tag --contains command, so I know this kernel fix is belong to 6.0.
> 
> git tag --contains ac6800e27
> v6.0-rc1
> v6.0-rc2

Now I'm confused, if I do git describe ac6800e27 it says that it's a
second commit on the top of 5.19-rc7. So shouldn't the the git tag
--contains report 5.19-rc8 and newer? What do I miss?

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH v4 3/3] syscalls/openat: Add new regression test when using open(O_TMPFILE) under umask

[xuyang2018.jy]

Hi  Cyril

>> > And this one is probably missing some kernel commit tags too, since the only that that is attached is supposedly in 5.19 but the test still fails.
>>> 
>> > I supposed that we should add the 426b4ca2d6a5 to the test tags here as well, right?
>>>
>> Yes, BTW,  I usually use git tag --contains command, so I know this kernel fix is belong to 6.0.
>> 
>> git tag --contains ac6800e27
>> v6.0-rc1
>> v6.0-rc2

>Now I'm confused, if I do git describe ac6800e27 it says that it's a second commit on the top of 5.19-rc7. So shouldn't the the git tag --contains report 5.19-rc8 and newer? What do I miss?

I don't know about this. It is a detail in kernel community collaborative work .
But kernel subsystem maintainer should be aware of this. @ Christian Brauner  Can you help Cyril settle this confuse ?

Best Regards
Yang Xu

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

Re: [LTP] [PATCH v4 3/3] syscalls/openat: Add new regression test when using open(O_TMPFILE) under umask

[Christian Brauner]

On Thu, Sep 22, 2022 at 11:27:43AM +0200, Cyril Hrubis wrote:
> Hi!
> > > And this one is probably missing some kernel commit tags too, since the only that that is attached is supposedly in 5.19 but the test still fails.
> > 
> > > I supposed that we should add the 426b4ca2d6a5 to the test tags here as well, right?
> > 
> > Yes, BTW,  I usually use git tag --contains command, so I know this kernel fix is belong to 6.0.
> > 
> > git tag --contains ac6800e27
> > v6.0-rc1
> > v6.0-rc2
> 
> Now I'm confused, if I do git describe ac6800e27 it says that it's a
> second commit on the top of 5.19-rc7. So shouldn't the the git tag
> --contains report 5.19-rc8 and newer? What do I miss?

So, ac6800e279a2 ("fs: Add missing umask strip in vfs_tmpfile") has been
backported to kernels since before the dawn of time. But the the all the
pieces that move setgid handling out of individual filesystems and into
the vfs proper are only >= v6.0-rc1.

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

Re: [LTP] [PATCH v4 3/3] syscalls/openat: Add new regression test when using open(O_TMPFILE) under umask

[xuyang2018.jy]

Hi Christian

> On Thu, Sep 22, 2022 at 11:27:43AM +0200, Cyril Hrubis wrote:
>> Hi!
>>>> And this one is probably missing some kernel commit tags too, since the only that that is attached is supposedly in 5.19 but the test still fails.
>>>
>>>> I supposed that we should add the 426b4ca2d6a5 to the test tags here as well, right?
>>>
>>> Yes, BTW,  I usually use git tag --contains command, so I know this kernel fix is belong to 6.0.
>>>
>>> git tag --contains ac6800e27
>>> v6.0-rc1
>>> v6.0-rc2
>>
>> Now I'm confused, if I do git describe ac6800e27 it says that it's a
>> second commit on the top of 5.19-rc7. So shouldn't the the git tag
>> --contains report 5.19-rc8 and newer? What do I miss?
> 
> So, ac6800e279a2 ("fs: Add missing umask strip in vfs_tmpfile") has been
> backported to kernels since before the dawn of time. But the the all the
> pieces that move setgid handling out of individual filesystems and into
> the vfs proper are only >= v6.0-rc1.

I search the commit mesage in the following url,but not find c6800e279a2 
commit

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/log/?h=v5.19

@Cyril
I guess git describe ac6800e2 will serach the commit id because it is 
merge request"Merge tag 'fs.setgid.v6.0' of 
git://git.kernel.org/pub/scm/linux/kerne/git/brauner/linux"
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=426b4ca2d6a5

then it will serach brauner linux fs.setgid. branch
see the following url
https://git.kernel.org/pub/scm/linux/kernel/git/brauner/linux.git/log/?h=fs.setgid

so git describe ac6800e27 it says that it's a second commit on the top 
of 5.19-rc7.


Best Regards
Yang Xu

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

Re: [LTP] [PATCH v4 3/3] syscalls/openat: Add new regression test when using open(O_TMPFILE) under umask

[xuyang2018.jy]

Hi Cyril

> Hi Christian
> 
>> On Thu, Sep 22, 2022 at 11:27:43AM +0200, Cyril Hrubis wrote:
>>> Hi!
>>>>> And this one is probably missing some kernel commit tags too, since the only that that is attached is supposedly in 5.19 but the test still fails.
>>>>
>>>>> I supposed that we should add the 426b4ca2d6a5 to the test tags here as well, right?
>>>>
>>>> Yes, BTW,  I usually use git tag --contains command, so I know this kernel fix is belong to 6.0.
>>>>
>>>> git tag --contains ac6800e27
>>>> v6.0-rc1
>>>> v6.0-rc2
>>>
>>> Now I'm confused, if I do git describe ac6800e27 it says that it's a
>>> second commit on the top of 5.19-rc7. So shouldn't the the git tag
>>> --contains report 5.19-rc8 and newer? What do I miss?
>>
>> So, ac6800e279a2 ("fs: Add missing umask strip in vfs_tmpfile") has been
>> backported to kernels since before the dawn of time. But the the all the
>> pieces that move setgid handling out of individual filesystems and into
>> the vfs proper are only >= v6.0-rc1.
> 
> I search the commit mesage in the following url,but not find c6800e279a2
> commit
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/log/?h=v5.19
> 
> @Cyril
> I guess git describe ac6800e2 will serach the commit id because it is
> merge request"Merge tag 'fs.setgid.v6.0' of
> git://git.kernel.org/pub/scm/linux/kerne/git/brauner/linux"
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=426b4ca2d6a5
> 
> then it will serach brauner linux fs.setgid. branch
> see the following url
> https://git.kernel.org/pub/scm/linux/kernel/git/brauner/linux.git/log/?h=fs.setgid
> 
> so git describe ac6800e27 it says that it's a second commit on the top
> of 5.19-rc7.

So do you still have doubt on this?  I don't think this should block 
this two patches.

Best Regards
Yang Xu
> 
> 
> Best Regards
> Yang Xu
> 

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