Re: [PATCH v1 1/2] vfs: Add new setgid_create_umask test

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



on 2022/07/17 0:12, Zorro Lang wrote:
> On Sat, May 21, 2022 at 12:04:23AM +0800, Yang Xu wrote:
>> The current_umask() is stripped from the mode directly in the vfs if the
>> filesystem either doesn't support acls or the filesystem has been
>> mounted without posic acl support.
>>
>> If the filesystem does support acls then current_umask() stripping is
>> deferred to posix_acl_create(). So when the filesystem calls
>> posix_acl_create() and there are no acls set or not supported then
>> current_umask() will be stripped.
>>
>> This patch is also designed to test kernel patchset behaviour
>> "move S_ISGID stripping into the vfs"
>> https://patchwork.kernel.org/project/linux-fsdevel/list/?series=635692
>>
>> Here we only use umask(S_IXGRP) to check whether inode strip
>> S_ISGID works correctly and check whether S_IXGRP mode exists.
>>
>> Signed-off-by: Yang Xu <xuyang2018.jy@xxxxxxxxxxx>
>> ---
> 
> This patchset has been stuck at here more than one month. As VFS guys gave it
> many review points last time [1]. So cc vfs list to make sure if they have
> more concern.
> 

Yes.

I don't plan to update these two tests until kernel patch set[1] is 
merged at next merge window.

ps: I may also refer to Christian 31c01ce18 ("generic: add test for 
tmpfs POSIX ACLs") format and only add  regression test in normal 
situation instead of in additional idmapped mount and userns situation.

[1]https://patchwork.kernel.org/project/linux-fsdevel/list/?series=659534

Best Regards
Yang Xu

> Thanks,
> Zorro
> 
> [1]
> https://lore.kernel.org/fstests/1649763226-2329-4-git-send-email-xuyang2018.jy@xxxxxxxxxxx/
> 
>>   src/vfs/idmapped-mounts.c | 429 ++++++++++++++++++++++++++++++++++++++
>>   src/vfs/idmapped-mounts.h |   1 +
>>   src/vfs/utils.c           |  14 ++
>>   src/vfs/utils.h           |   1 +
>>   src/vfs/vfstest.c         | 205 +++++++++++++++++-
>>   tests/generic/691         |  40 ++++
>>   tests/generic/691.out     |   2 +
>>   7 files changed, 691 insertions(+), 1 deletion(-)
>>   create mode 100755 tests/generic/691
>>   create mode 100644 tests/generic/691.out
>>
>> diff --git a/src/vfs/idmapped-mounts.c b/src/vfs/idmapped-mounts.c
>> index 63297d5f..72de52cc 100644
>> --- a/src/vfs/idmapped-mounts.c
>> +++ b/src/vfs/idmapped-mounts.c
>> @@ -7664,6 +7664,425 @@ out:
>>   	return fret;
>>   }
>>   
>> +static int setgid_create_umask_idmapped(const struct vfstest_info *info)
>> +{
>> +	int fret = -1;
>> +	int file1_fd = -EBADF, open_tree_fd = -EBADF;
>> +	struct mount_attr attr = {
>> +		.attr_set = MOUNT_ATTR_IDMAP,
>> +	};
>> +	pid_t pid;
>> +	int tmpfile_fd = -EBADF;
>> +	bool supported = false;
>> +	char path[PATH_MAX];
>> +	mode_t mode;
>> +
>> +	if (!caps_supported())
>> +		return 0;
>> +
>> +	if (fchmod(info->t_dir1_fd, S_IRUSR |
>> +			      S_IWUSR |
>> +			      S_IRGRP |
>> +			      S_IWGRP |
>> +			      S_IROTH |
>> +			      S_IWOTH |
>> +			      S_IXUSR |
>> +			      S_IXGRP |
>> +			      S_IXOTH |
>> +			      S_ISGID), 0) {
>> +		log_stderr("failure: fchmod");
>> +		goto out;
>> +	}
>> +
>> +	/* Verify that the sid bits got raised. */
>> +	if (!is_setgid(info->t_dir1_fd, "", AT_EMPTY_PATH)) {
>> +		log_stderr("failure: is_setgid");
>> +		goto out;
>> +	}
>> +
>> +	/* Changing mount properties on a detached mount. */
>> +	attr.userns_fd	= get_userns_fd(0, 10000, 10000);
>> +	if (attr.userns_fd < 0) {
>> +		log_stderr("failure: get_userns_fd");
>> +		goto out;
>> +	}
>> +
>> +	open_tree_fd = sys_open_tree(info->t_dir1_fd, "",
>> +				     AT_EMPTY_PATH |
>> +				     AT_NO_AUTOMOUNT |
>> +				     AT_SYMLINK_NOFOLLOW |
>> +				     OPEN_TREE_CLOEXEC |
>> +				     OPEN_TREE_CLONE);
>> +	if (open_tree_fd < 0) {
>> +		log_stderr("failure: sys_open_tree");
>> +		goto out;
>> +	}
>> +
>> +	if (sys_mount_setattr(open_tree_fd, "", AT_EMPTY_PATH, &attr, sizeof(attr))) {
>> +		log_stderr("failure: sys_mount_setattr");
>> +		goto out;
>> +	}
>> +
>> +	supported = openat_tmpfile_supported(open_tree_fd);
>> +
>> +	/* Only umask with S_IXGRP because inode strip S_ISGID will check mode
>> +	 * whether has group execute or search permission.
>> +	 */
>> +	umask(S_IXGRP);
>> +	mode = umask(S_IXGRP);
>> +	if (!(mode & S_IXGRP))
>> +		die("failure: umask");
>> +
>> +		pid = fork();
>> +	if (pid < 0) {
>> +		log_stderr("failure: fork");
>> +		goto out;
>> +	}
>> +	if (pid == 0) {
>> +		if (!switch_ids(10000, 11000))
>> +			die("failure: switch fsids");
>> +
>> +		/* create regular file via open() */
>> +		file1_fd = openat(open_tree_fd, FILE1, O_CREAT | O_EXCL | O_CLOEXEC, S_IXGRP | S_ISGID);
>> +		if (file1_fd < 0)
>> +			die("failure: create");
>> +
>> +		/* Neither in_group_p() nor capable_wrt_inode_uidgid() so setgid
>> +		 * bit needs to be stripped.
>> +		 */
>> +		if (is_setgid(open_tree_fd, FILE1, 0))
>> +			die("failure: is_setgid");
>> +
>> +		if (is_ixgrp(open_tree_fd, FILE1, 0))
>> +			die("failure: is_ixgrp");
>> +
>> +		/* create directory */
>> +		if (mkdirat(open_tree_fd, DIR1, 0000))
>> +			die("failure: create");
>> +
>> +		if (xfs_irix_sgid_inherit_enabled(info->t_fstype)) {
>> +			/* We're not in_group_p(). */
>> +			if (is_setgid(open_tree_fd, DIR1, 0))
>> +				die("failure: is_setgid");
>> +		} else {
>> +			/* Directories always inherit the setgid bit. */
>> +			if (!is_setgid(open_tree_fd, DIR1, 0))
>> +				die("failure: is_setgid");
>> +		}
>> +
>> +		if (is_ixgrp(open_tree_fd, DIR1, 0))
>> +			die("failure: is_ixgrp");
>> +
>> +		/* create a special file via mknodat() vfs_create */
>> +		if (mknodat(open_tree_fd, FILE2, S_IFREG | S_ISGID | S_IXGRP, 0))
>> +			die("failure: mknodat");
>> +
>> +		if (is_setgid(open_tree_fd, FILE2, 0))
>> +			die("failure: is_setgid");
>> +
>> +		if (is_ixgrp(open_tree_fd, FILE2, 0))
>> +			die("failure: is_ixgrp");
>> +
>> +		/* create a whiteout device via mknodat() vfs_mknod */
>> +		if (mknodat(open_tree_fd, CHRDEV1, S_IFCHR | S_ISGID | S_IXGRP, 0))
>> +			die("failure: mknodat");
>> +
>> +		if (is_setgid(open_tree_fd, CHRDEV1, 0))
>> +			die("failure: is_setgid");
>> +
>> +		if (is_ixgrp(open_tree_fd, CHRDEV1, 0))
>> +			die("failure: is_ixgrp");
>> +
>> +		/*
>> +		 * In setgid directories newly created files always inherit the
>> +		 * gid from the parent directory. Verify that the file is owned
>> +		 * by gid 10000, not by gid 11000.
>> +		 */
>> +		if (!expected_uid_gid(open_tree_fd, FILE1, 0, 10000, 10000))
>> +			die("failure: check ownership");
>> +
>> +		/*
>> +		 * In setgid directories newly created directories always
>> +		 * inherit the gid from the parent directory. Verify that the
>> +		 * directory is owned by gid 10000, not by gid 11000.
>> +		 */
>> +		if (!expected_uid_gid(open_tree_fd, DIR1, 0, 10000, 10000))
>> +			die("failure: check ownership");
>> +
>> +		if (!expected_uid_gid(open_tree_fd, FILE2, 0, 10000, 10000))
>> +			die("failure: check ownership");
>> +
>> +		if (!expected_uid_gid(open_tree_fd, CHRDEV1, 0, 10000, 10000))
>> +			die("failure: check ownership");
>> +
>> +		if (unlinkat(open_tree_fd, FILE1, 0))
>> +			die("failure: delete");
>> +
>> +		if (unlinkat(open_tree_fd, DIR1, AT_REMOVEDIR))
>> +			die("failure: delete");
>> +
>> +		if (unlinkat(open_tree_fd, FILE2, 0))
>> +			die("failure: delete");
>> +
>> +		if (unlinkat(open_tree_fd, CHRDEV1, 0))
>> +			die("failure: delete");
>> +
>> +		/* create tmpfile via filesystem tmpfile api */
>> +		if (supported) {
>> +			tmpfile_fd = openat(open_tree_fd, ".", O_TMPFILE | O_RDWR, S_IXGRP | S_ISGID);
>> +			if (tmpfile_fd < 0)
>> +				die("failure: create");
>> +			/* link the temporary file into the filesystem, making it permanent */
>> +			snprintf(path, PATH_MAX,  "/proc/self/fd/%d", tmpfile_fd);
>> +			if (linkat(AT_FDCWD, path, open_tree_fd, FILE3, AT_SYMLINK_FOLLOW))
>> +				die("failure: linkat");
>> +			if (close(tmpfile_fd))
>> +				die("failure: close");
>> +			if (is_setgid(open_tree_fd, FILE3, 0))
>> +				die("failure: is_setgid");
>> +			if (is_ixgrp(open_tree_fd, FILE3, 0))
>> +				die("failure: is_ixgrp");
>> +			if (!expected_uid_gid(open_tree_fd, FILE3, 0, 10000, 10000))
>> +				die("failure: check ownership");
>> +			if (unlinkat(open_tree_fd, FILE3, 0))
>> +				die("failure: delete");
>> +		}
>> +
>> +		exit(EXIT_SUCCESS);
>> +	}
>> +	if (wait_for_pid(pid))
>> +		goto out;
>> +
>> +	fret = 0;
>> +	log_debug("Ran test");
>> +out:
>> +	safe_close(attr.userns_fd);
>> +	safe_close(file1_fd);
>> +	safe_close(open_tree_fd);
>> +
>> +	return fret;
>> +}
>> +
>> +static int setgid_create_umask_idmapped_in_userns(const struct vfstest_info *info)
>> +{
>> +	int fret = -1;
>> +	int file1_fd = -EBADF, open_tree_fd = -EBADF;
>> +	struct mount_attr attr = {
>> +		.attr_set = MOUNT_ATTR_IDMAP,
>> +	};
>> +	pid_t pid;
>> +	int tmpfile_fd = -EBADF;
>> +	bool supported = false;
>> +	char path[PATH_MAX];
>> +	mode_t mode;
>> +
>> +	if (!caps_supported())
>> +		return 0;
>> +
>> +	if (fchmod(info->t_dir1_fd, S_IRUSR |
>> +			      S_IWUSR |
>> +			      S_IRGRP |
>> +			      S_IWGRP |
>> +			      S_IROTH |
>> +			      S_IWOTH |
>> +			      S_IXUSR |
>> +			      S_IXGRP |
>> +			      S_IXOTH |
>> +			      S_ISGID), 0) {
>> +		log_stderr("failure: fchmod");
>> +		goto out;
>> +	}
>> +
>> +	/* Verify that the sid bits got raised. */
>> +	if (!is_setgid(info->t_dir1_fd, "", AT_EMPTY_PATH)) {
>> +		log_stderr("failure: is_setgid");
>> +		goto out;
>> +	}
>> +
>> +	/* Changing mount properties on a detached mount. */
>> +	attr.userns_fd	= get_userns_fd(0, 10000, 10000);
>> +	if (attr.userns_fd < 0) {
>> +		log_stderr("failure: get_userns_fd");
>> +		goto out;
>> +	}
>> +
>> +	open_tree_fd = sys_open_tree(info->t_dir1_fd, "",
>> +				     AT_EMPTY_PATH |
>> +				     AT_NO_AUTOMOUNT |
>> +				     AT_SYMLINK_NOFOLLOW |
>> +				     OPEN_TREE_CLOEXEC |
>> +				     OPEN_TREE_CLONE);
>> +	if (open_tree_fd < 0) {
>> +		log_stderr("failure: sys_open_tree");
>> +		goto out;
>> +	}
>> +
>> +	if (sys_mount_setattr(open_tree_fd, "", AT_EMPTY_PATH, &attr, sizeof(attr))) {
>> +		log_stderr("failure: sys_mount_setattr");
>> +		goto out;
>> +	}
>> +
>> +	supported = openat_tmpfile_supported(open_tree_fd);
>> +
>> +	/* Only umask with S_IXGRP because inode strip S_ISGID will check mode
>> +	 * whether has group execute or search permission.
>> +	 */
>> +	umask(S_IXGRP);
>> +	mode = umask(S_IXGRP);
>> +	if (!(mode & S_IXGRP))
>> +		die("failure: umask");
>> +
>> +	/*
>> +	 * Below we verify that setgid inheritance for a newly created file or
>> +	 * directory works correctly. As part of this we need to verify that
>> +	 * newly created files or directories inherit their gid from their
>> +	 * parent directory. So we change the parent directorie's gid to 1000
>> +	 * and create a file with fs{g,u}id 0 and verify that the newly created
>> +	 * file and directory inherit gid 1000, not 0.
>> +	 */
>> +	if (fchownat(info->t_dir1_fd, "", -1, 1000, AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH)) {
>> +		log_stderr("failure: fchownat");
>> +		goto out;
>> +	}
>> +
>> +	pid = fork();
>> +	if (pid < 0) {
>> +		log_stderr("failure: fork");
>> +		goto out;
>> +	}
>> +	if (pid == 0) {
>> +		if (!caps_supported()) {
>> +			log_debug("skip: capability library not installed");
>> +			exit(EXIT_SUCCESS);
>> +		}
>> +
>> +		if (!switch_userns(attr.userns_fd, 0, 0, false))
>> +			die("failure: switch_userns");
>> +
>> +		if (!caps_down_fsetid())
>> +			die("failure: caps_down_fsetid");
>> +
>> +		/* create regular file via open() */
>> +		file1_fd = openat(open_tree_fd, FILE1, O_CREAT | O_EXCL | O_CLOEXEC, S_IXGRP | S_ISGID);
>> +		if (file1_fd < 0)
>> +			die("failure: create");
>> +
>> +		/* Neither in_group_p() nor capable_wrt_inode_uidgid() so setgid
>> +		 * bit needs to be stripped.
>> +		 */
>> +		if (is_setgid(open_tree_fd, FILE1, 0))
>> +			die("failure: is_setgid");
>> +
>> +		if (is_ixgrp(open_tree_fd, FILE1, 0))
>> +			die("failure: is_ixgrp");
>> +
>> +		/* create directory */
>> +		if (mkdirat(open_tree_fd, DIR1, 0000))
>> +			die("failure: create");
>> +
>> +		if (xfs_irix_sgid_inherit_enabled(info->t_fstype)) {
>> +			/* We're not in_group_p(). */
>> +			if (is_setgid(open_tree_fd, DIR1, 0))
>> +				die("failure: is_setgid");
>> +		} else {
>> +			/* Directories always inherit the setgid bit. */
>> +			if (!is_setgid(open_tree_fd, DIR1, 0))
>> +				die("failure: is_setgid");
>> +		}
>> +
>> +		if (is_ixgrp(open_tree_fd, DIR1, 0))
>> +			die("failure: is_ixgrp");
>> +
>> +		/* create a special file via mknodat() vfs_create */
>> +		if (mknodat(open_tree_fd, FILE2, S_IFREG | S_ISGID | S_IXGRP, 0))
>> +			die("failure: mknodat");
>> +
>> +		if (is_setgid(open_tree_fd, FILE2, 0))
>> +			die("failure: is_setgid");
>> +
>> +		if (is_ixgrp(open_tree_fd, FILE2, 0))
>> +			die("failure: is_ixgrp");
>> +
>> +		/* create a whiteout device via mknodat() vfs_mknod */
>> +		if (mknodat(open_tree_fd, CHRDEV1, S_IFCHR | S_ISGID | S_IXGRP, 0))
>> +			die("failure: mknodat");
>> +
>> +		if (is_setgid(open_tree_fd, CHRDEV1, 0))
>> +			die("failure: is_setgid");
>> +
>> +		if (is_ixgrp(open_tree_fd, CHRDEV1, 0))
>> +			die("failure: is_ixgrp");
>> +
>> +		/*
>> +		 * In setgid directories newly created files always inherit the
>> +		 * gid from the parent directory. Verify that the file is owned
>> +		 * by gid 1000, not by gid 0.
>> +		 */
>> +		if (!expected_uid_gid(open_tree_fd, FILE1, 0, 0, 1000))
>> +			die("failure: check ownership");
>> +
>> +		/*
>> +		 * In setgid directories newly created directories always
>> +		 * inherit the gid from the parent directory. Verify that the
>> +		 * directory is owned by gid 1000, not by gid 0.
>> +		 */
>> +		if (!expected_uid_gid(open_tree_fd, DIR1, 0, 0, 1000))
>> +			die("failure: check ownership");
>> +
>> +		if (!expected_uid_gid(open_tree_fd, FILE2, 0, 0, 1000))
>> +			die("failure: check ownership");
>> +
>> +		if (!expected_uid_gid(open_tree_fd, CHRDEV1, 0, 0, 1000))
>> +			die("failure: check ownership");
>> +
>> +		if (unlinkat(open_tree_fd, FILE1, 0))
>> +			die("failure: delete");
>> +
>> +		if (unlinkat(open_tree_fd, DIR1, AT_REMOVEDIR))
>> +			die("failure: delete");
>> +
>> +		if (unlinkat(open_tree_fd, FILE2, 0))
>> +			die("failure: delete");
>> +
>> +		if (unlinkat(open_tree_fd, CHRDEV1, 0))
>> +			die("failure: delete");
>> +
>> +		/* create tmpfile via filesystem tmpfile api */
>> +		if (supported) {
>> +			tmpfile_fd = openat(open_tree_fd, ".", O_TMPFILE | O_RDWR, S_IXGRP | S_ISGID);
>> +			if (tmpfile_fd < 0)
>> +				die("failure: create");
>> +			/* link the temporary file into the filesystem, making it permanent */
>> +			snprintf(path, PATH_MAX,  "/proc/self/fd/%d", tmpfile_fd);
>> +			if (linkat(AT_FDCWD, path, open_tree_fd, FILE3, AT_SYMLINK_FOLLOW))
>> +				die("failure: linkat");
>> +			if (close(tmpfile_fd))
>> +				die("failure: close");
>> +			if (is_setgid(open_tree_fd, FILE3, 0))
>> +				die("failure: is_setgid");
>> +			if (is_ixgrp(open_tree_fd, FILE3, 0))
>> +				die("failure: is_ixgrp");
>> +			if (!expected_uid_gid(open_tree_fd, FILE3, 0, 0, 1000))
>> +				die("failure: check ownership");
>> +			if (unlinkat(open_tree_fd, FILE3, 0))
>> +				die("failure: delete");
>> +		}
>> +
>> +		exit(EXIT_SUCCESS);
>> +	}
>> +	if (wait_for_pid(pid))
>> +		goto out;
>> +
>> +	fret = 0;
>> +	log_debug("Ran test");
>> +out:
>> +	safe_close(attr.userns_fd);
>> +	safe_close(file1_fd);
>> +	safe_close(open_tree_fd);
>> +
>> +	return fret;
>> +}
>> +
>>   static const struct test_struct t_idmapped_mounts[] = {
>>   	{ acls,                                                         true,   "posix acls on regular mounts",                                                                 },
>>   	{ create_in_userns,                                             true,   "create operations in user namespace",                                                          },
>> @@ -7745,3 +8164,13 @@ const struct test_suite s_setxattr_fix_705191b03d50 = {
>>   	.tests = t_setxattr_fix_705191b03d50,
>>   	.nr_tests = ARRAY_SIZE(t_setxattr_fix_705191b03d50),
>>   };
>> +
>> +static const struct test_struct t_setgid_create_umask_idmapped[] = {
>> +	{ setgid_create_umask_idmapped,					T_REQUIRE_IDMAPPED_MOUNTS,	"create operations by using umask in directories with setgid bit set on idmapped mount",		},
>> +	{ setgid_create_umask_idmapped_in_userns,			T_REQUIRE_IDMAPPED_MOUNTS,	"create operations by using umask in directories with setgid bit set on idmapped mount inside userns",	},
>> +};
>> +
>> +const struct test_suite s_setgid_create_umask_idmapped = {
>> +	.tests = t_setgid_create_umask_idmapped,
>> +	.nr_tests = ARRAY_SIZE(t_setgid_create_umask_idmapped),
>> +};
>> diff --git a/src/vfs/idmapped-mounts.h b/src/vfs/idmapped-mounts.h
>> index ff21ea2c..a332439f 100644
>> --- a/src/vfs/idmapped-mounts.h
>> +++ b/src/vfs/idmapped-mounts.h
>> @@ -14,5 +14,6 @@ extern const struct test_suite s_fscaps_in_ancestor_userns;
>>   extern const struct test_suite s_nested_userns;
>>   extern const struct test_suite s_setattr_fix_968219708108;
>>   extern const struct test_suite s_setxattr_fix_705191b03d50;
>> +extern const struct test_suite s_setgid_create_umask_idmapped;
>>   
>>   #endif /* __IDMAPPED_MOUNTS_H */
>> diff --git a/src/vfs/utils.c b/src/vfs/utils.c
>> index 1388edda..6db7a11d 100644
>> --- a/src/vfs/utils.c
>> +++ b/src/vfs/utils.c
>> @@ -809,6 +809,20 @@ bool is_sticky(int dfd, const char *path, int flags)
>>   	return (st.st_mode & S_ISVTX) > 0;
>>   }
>>   
>> +/*is_ixgrp - check whether file or directory is S_IXGRP */
>> +bool is_ixgrp(int dfd, const char *path, int flags)
>> +{
>> +	int ret;
>> +	struct stat st;
>> +
>> +	ret = fstatat(dfd, path, &st, flags);
>> +	if (ret < 0)
>> +		return false;
>> +
>> +	errno = 0; /* Don't report misleading errno. */
>> +	return (st.st_mode & S_IXGRP);
>> +}
>> +
>>   bool switch_resids(uid_t uid, gid_t gid)
>>   {
>>   	if (setresgid(gid, gid, gid))
>> diff --git a/src/vfs/utils.h b/src/vfs/utils.h
>> index 7fb702fd..c0dbe370 100644
>> --- a/src/vfs/utils.h
>> +++ b/src/vfs/utils.h
>> @@ -368,6 +368,7 @@ extern bool expected_file_size(int dfd, const char *path, int flags,
>>   extern bool is_setid(int dfd, const char *path, int flags);
>>   extern bool is_setgid(int dfd, const char *path, int flags);
>>   extern bool is_sticky(int dfd, const char *path, int flags);
>> +extern bool is_ixgrp(int dfd, const char *path, int flags);
>>   extern bool openat_tmpfile_supported(int dirfd);
>>   
>>   #endif /* __IDMAP_UTILS_H */
>> diff --git a/src/vfs/vfstest.c b/src/vfs/vfstest.c
>> index 29ac0bec..8448362e 100644
>> --- a/src/vfs/vfstest.c
>> +++ b/src/vfs/vfstest.c
>> @@ -1733,6 +1733,186 @@ out:
>>   	return fret;
>>   }
>>   
>> +/* The current_umask() is stripped from the mode directly in the vfs if the
>> + * filesystem either doesn't support acls or the filesystem has been
>> + * mounted without posic acl support.
>> + *
>> + * If the filesystem does support acls then current_umask() stripping is
>> + * deferred to posix_acl_create(). So when the filesystem calls
>> + * posix_acl_create() and there are no acls set or not supported then
>> + * current_umask() will be stripped.
>> + *
>> + * Use umask(S_IXGRP) to check whether inode strip S_ISGID works correctly.
>> + */
>> +
>> +static int setgid_create_umask(const struct vfstest_info *info)
>> +{
>> +	int fret = -1;
>> +	int file1_fd = -EBADF;
>> +	int tmpfile_fd = -EBADF;
>> +	pid_t pid;
>> +	bool supported = false;
>> +	mode_t mode;
>> +
>> +	if (!caps_supported())
>> +		return 0;
>> +
>> +	if (fchmod(info->t_dir1_fd, S_IRUSR |
>> +			      S_IWUSR |
>> +			      S_IRGRP |
>> +			      S_IWGRP |
>> +			      S_IROTH |
>> +			      S_IWOTH |
>> +			      S_IXUSR |
>> +			      S_IXGRP |
>> +			      S_IXOTH |
>> +			      S_ISGID), 0) {
>> +		log_stderr("failure: fchmod");
>> +		goto out;
>> +	}
>> +
>> +	/* Verify that the setgid bit got raised. */
>> +	if (!is_setgid(info->t_dir1_fd, "", AT_EMPTY_PATH)) {
>> +		log_stderr("failure: is_setgid");
>> +		goto out;
>> +	}
>> +
>> +	supported = openat_tmpfile_supported(info->t_dir1_fd);
>> +
>> +	/* Only umask with S_IXGRP because inode strip S_ISGID will check mode
>> +	 * whether has group execute or search permission.
>> +	 */
>> +	umask(S_IXGRP);
>> +	mode = umask(S_IXGRP);
>> +	if (!(mode & S_IXGRP))
>> +		die("failure: umask");
>> +
>> +	pid = fork();
>> +	if (pid < 0) {
>> +		log_stderr("failure: fork");
>> +		goto out;
>> +	}
>> +	if (pid == 0) {
>> +		if (!switch_ids(0, 10000))
>> +			die("failure: switch_ids");
>> +
>> +		if (!caps_down_fsetid())
>> +			die("failure: caps_down_fsetid");
>> +
>> +		/* create regular file via open() */
>> +		file1_fd = openat(info->t_dir1_fd, FILE1, O_CREAT | O_EXCL | O_CLOEXEC, S_IXGRP | S_ISGID);
>> +		if (file1_fd < 0)
>> +			die("failure: create");
>> +
>> +		/* Neither in_group_p() nor capable_wrt_inode_uidgid() so setgid
>> +		 * bit needs to be stripped.
>> +		 */
>> +		if (is_setgid(info->t_dir1_fd, FILE1, 0))
>> +			die("failure: is_setgid");
>> +
>> +		if (is_ixgrp(info->t_dir1_fd, FILE1, 0))
>> +			die("failure: is_ixgrp");
>> +
>> +		if (mkdirat(info->t_dir1_fd, DIR1, 0000))
>> +			die("failure: create");
>> +
>> +		if (xfs_irix_sgid_inherit_enabled(info->t_fstype)) {
>> +			/* We're not in_group_p(). */
>> +			if (is_setgid(info->t_dir1_fd, DIR1, 0))
>> +				die("failure: is_setgid");
>> +		} else {
>> +			/* Directories always inherit the setgid bit. */
>> +			if (!is_setgid(info->t_dir1_fd, DIR1, 0))
>> +				die("failure: is_setgid");
>> +		}
>> +
>> +		if (is_ixgrp(info->t_dir1_fd, DIR1, 0))
>> +			die("failure: is_ixgrp");
>> +
>> +		/* create a special file via mknodat() vfs_create */
>> +		if (mknodat(info->t_dir1_fd, FILE2, S_IFREG | S_ISGID | S_IXGRP, 0))
>> +			die("failure: mknodat");
>> +
>> +		if (is_setgid(info->t_dir1_fd, FILE2, 0))
>> +			die("failure: is_setgid");
>> +
>> +		if (is_ixgrp(info->t_dir1_fd, FILE2, 0))
>> +			die("failure: is_ixgrp");
>> +
>> +		/* create a character device via mknodat() vfs_mknod */
>> +		if (mknodat(info->t_dir1_fd, CHRDEV1, S_IFCHR | S_ISGID | S_IXGRP, makedev(5, 1)))
>> +			die("failure: mknodat");
>> +
>> +		if (is_setgid(info->t_dir1_fd, CHRDEV1, 0))
>> +			die("failure: is_setgid");
>> +
>> +		if (is_ixgrp(info->t_dir1_fd, CHRDEV1, 0))
>> +			die("failure: is_ixgrp");
>> +
>> +		/*
>> +		 * In setgid directories newly created files always inherit the
>> +		 * gid from the parent directory. Verify that the file is owned
>> +		 * by gid 0, not by gid 10000.
>> +		 */
>> +		if (!expected_uid_gid(info->t_dir1_fd, FILE1, 0, 0, 0))
>> +			die("failure: check ownership");
>> +
>> +		/*
>> +		 * In setgid directories newly created directories always
>> +		 * inherit the gid from the parent directory. Verify that the
>> +		 * directory is owned by gid 0, not by gid 10000.
>> +		 */
>> +		if (!expected_uid_gid(info->t_dir1_fd, DIR1, 0, 0, 0))
>> +			die("failure: check ownership");
>> +
>> +		if (!expected_uid_gid(info->t_dir1_fd, FILE2, 0, 0, 0))
>> +			die("failure: check ownership");
>> +
>> +		if (!expected_uid_gid(info->t_dir1_fd, CHRDEV1, 0, 0, 0))
>> +			die("failure: check ownership");
>> +
>> +		if (unlinkat(info->t_dir1_fd, FILE1, 0))
>> +			die("failure: delete");
>> +
>> +		if (unlinkat(info->t_dir1_fd, DIR1, AT_REMOVEDIR))
>> +			die("failure: delete");
>> +
>> +		if (unlinkat(info->t_dir1_fd, FILE2, 0))
>> +			die("failure: delete");
>> +
>> +		if (unlinkat(info->t_dir1_fd, CHRDEV1, 0))
>> +			die("failure: delete");
>> +
>> +		/* create tmpfile via filesystem tmpfile api */
>> +		if (supported) {
>> +			tmpfile_fd = openat(info->t_dir1_fd, ".", O_TMPFILE | O_RDWR, S_IXGRP | S_ISGID);
>> +			if (tmpfile_fd < 0)
>> +				die("failure: create");
>> +			/* link the temporary file into the filesystem, making it permanent */
>> +			if (linkat(tmpfile_fd, "", info->t_dir1_fd, FILE3, AT_EMPTY_PATH))
>> +				die("failure: linkat");
>> +			if (close(tmpfile_fd))
>> +				die("failure: close");
>> +			if (is_setgid(info->t_dir1_fd, FILE3, 0))
>> +				die("failure: is_setgid");
>> +			if (is_ixgrp(info->t_dir1_fd, FILE3, 0))
>> +				die("failure: is_ixgrp");
>> +			if (unlinkat(info->t_dir1_fd, FILE3, 0))
>> +				die("failure: delete");
>> +		}
>> +
>> +		exit(EXIT_SUCCESS);
>> +	}
>> +	if (wait_for_pid(pid))
>> +		goto out;
>> +
>> +	fret = 0;
>> +	log_debug("Ran test");
>> +out:
>> +	safe_close(file1_fd);
>> +	return fret;
>> +}
>> +
>>   static int setattr_truncate(const struct vfstest_info *info)
>>   {
>>   	int fret = -1;
>> @@ -1807,6 +1987,7 @@ static void usage(void)
>>   	fprintf(stderr, "--test-btrfs                        Run btrfs specific idmapped mount testsuite\n");
>>   	fprintf(stderr, "--test-setattr-fix-968219708108     Run setattr regression tests\n");
>>   	fprintf(stderr, "--test-setxattr-fix-705191b03d50    Run setxattr regression tests\n");
>> +	fprintf(stderr, "--test-setgid-create-umask          Run setgid with umask tests\n");
>>   
>>   	_exit(EXIT_SUCCESS);
>>   }
>> @@ -1825,6 +2006,7 @@ static const struct option longopts[] = {
>>   	{"test-btrfs",				no_argument,		0,	'b'},
>>   	{"test-setattr-fix-968219708108",	no_argument,		0,	'i'},
>>   	{"test-setxattr-fix-705191b03d50",	no_argument,		0,	'j'},
>> +	{"test-setgid-create-umask",		no_argument,		0,	'u'},
>>   	{NULL,					0,			0,	  0},
>>   };
>>   
>> @@ -1850,6 +2032,15 @@ static const struct test_suite s_basic = {
>>   	.nr_tests = ARRAY_SIZE(t_basic),
>>   };
>>   
>> +static const struct test_struct t_setgid_create_umask[] = {
>> +	{ setgid_create_umask,						0,			"create operations in directories with setgid bit set under umask",				},
>> +};
>> +
>> +static const struct test_suite s_setgid_create_umask = {
>> +	.tests = t_setgid_create_umask,
>> +	.nr_tests = ARRAY_SIZE(t_setgid_create_umask),
>> +};
>> +
>>   static bool run_test(struct vfstest_info *info, const struct test_struct suite[], size_t suite_size)
>>   {
>>   	int i;
>> @@ -1947,7 +2138,8 @@ int main(int argc, char *argv[])
>>   	bool idmapped_mounts_supported = false, test_btrfs = false,
>>   	     test_core = false, test_fscaps_regression = false,
>>   	     test_nested_userns = false, test_setattr_fix_968219708108 = false,
>> -	     test_setxattr_fix_705191b03d50 = false;
>> +	     test_setxattr_fix_705191b03d50 = false,
>> +	     test_setgid_create_umask = false;
>>   
>>   	init_vfstest_info(&info);
>>   
>> @@ -1989,6 +2181,9 @@ int main(int argc, char *argv[])
>>   		case 'j':
>>   			test_setxattr_fix_705191b03d50 = true;
>>   			break;
>> +		case 'u':
>> +			test_setgid_create_umask = true;
>> +			break;
>>   		case 'h':
>>   			/* fallthrough */
>>   		default:
>> @@ -2066,6 +2261,14 @@ int main(int argc, char *argv[])
>>   	    !run_suite(&info, &s_setxattr_fix_705191b03d50))
>>   		goto out;
>>   
>> +	if (test_setgid_create_umask) {
>> +		if (!run_suite(&info, &s_setgid_create_umask))
>> +			goto out;
>> +
>> +		if (!run_suite(&info, &s_setgid_create_umask_idmapped))
>> +			goto out;
>> +	}
>> +
>>   	fret = EXIT_SUCCESS;
>>   
>>   out:
>> diff --git a/tests/generic/691 b/tests/generic/691
>> new file mode 100755
>> index 00000000..d4875854
>> --- /dev/null
>> +++ b/tests/generic/691
>> @@ -0,0 +1,40 @@
>> +#! /bin/bash
>> +# SPDX-License-Identifier: GPL-2.0
>> +# Copyright (c) 2022 Fujitsu Limited. All Rights Reserved.
>> +#
>> +# FS QA Test No. 691
>> +#
>> +# Test that idmapped mounts setgid's behave correctly when using
>> +# umask(S_IXGRP)
>> +#
>> +. ./common/preamble
>> +_begin_fstest auto quick cap idmapped mount perms rw unlink
>> +
>> +# Import common functions.
>> +. ./common/filter
>> +
>> +# real QA test starts here
>> +
>> +_supported_fs generic
>> +_require_test
>> +_require_scratch
>> +
>> +echo "Silence is golden"
>> +
>> +$here/src/vfs/vfstest --test-setgid-create-umask \
>> +        --device "$TEST_DEV" --mount "$TEST_DIR" --fstype "$FSTYP"
>> +
>> +_scratch_mkfs > "$seqres.full" 2>&1
>> +export MOUNT_OPTIONS="-o noacl"
>> +
>> +# If filesystem supports noacl mount option, also test setgid bit whether
>> +# was stripped correctly.
>> +# noacl will earse acl flag in superblock, so kernel will use current_umask
>> +# in vfs directly instead of calling posix_acl_create on underflying
>> +# filesystem.
>> +_try_scratch_mount >>$seqres.full 2>&1 && \
>> +	$here/src/vfs/vfstest --test-setgid-create-umask \
>> +        --device "$SCRATCH_DEV" --mount "$SCRATCH_MNT" --fstype "$FSTYP"
>> +
>> +status=0
>> +exit
>> diff --git a/tests/generic/691.out b/tests/generic/691.out
>> new file mode 100644
>> index 00000000..006aef43
>> --- /dev/null
>> +++ b/tests/generic/691.out
>> @@ -0,0 +1,2 @@
>> +QA output created by 691
>> +Silence is golden
>> -- 
>> 2.27.0
>>
> 




[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux