* [LTP] [PATCH v1 2/2] syscalls/openat: Add new regression test when using open(O_TMPFILE) under umask
2022-07-22 10:27 [LTP] [PATCH v1 1/2] syscalls/creat09: Add umask condition Yang Xu
@ 2022-07-22 10:27 ` Yang Xu
2022-08-02 17:26 ` [LTP] [PATCH v1 1/2] syscalls/creat09: Add umask condition Petr Vorel
1 sibling, 0 replies; 4+ messages in thread
From: Yang Xu @ 2022-07-22 10:27 UTC (permalink / raw)
To: ltp; +Cc: brauner
Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
runtest/syscalls | 2 +-
testcases/kernel/syscalls/openat/.gitignore | 1 +
testcases/kernel/syscalls/openat/openat04.c | 194 ++++++++++++++++++++
3 files changed, 196 insertions(+), 1 deletion(-)
create mode 100644 testcases/kernel/syscalls/openat/openat04.c
diff --git a/runtest/syscalls b/runtest/syscalls
index 3847e8af2..448b5613c 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -920,10 +920,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..323d9a971
--- /dev/null
+++ b/testcases/kernel/syscalls/openat/openat04.c
@@ -0,0 +1,194 @@
+// 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 | TTERRNO, "mount(%s, %s, %s, %lu, %p) failed",
+ source, target, filesystemtype, mountflags, data);
+ }
+
+ 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, "open(%d, O_TMPFILE) failed", dirfd);
+ }
+
+ 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);
+ }
+}
+
+static void file_test(int dfd, const char *path, int flags)
+{
+ struct stat buf;
+
+ TST_EXP_PASS_SILENT(fstatat(dfd, path, &buf, flags));
+ if (!TST_PASS) {
+ tst_res(TFAIL, "fstat failed");
+ return;
+ }
+
+ if (buf.st_gid != free_gid) {
+ tst_res(TFAIL, "%s: Incorrect group, %u != %u", path,
+ buf.st_gid, free_gid);
+ } else {
+ tst_res(TPASS, "%s: Owned by correct group", path);
+ }
+
+ 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)
+{
+ int pid;
+ char path[PATH_MAX];
+
+ pid = SAFE_FORK();
+ if (pid == 0) {
+ /* Switch user */
+ SAFE_SETGID(ltpuser->pw_gid);
+ SAFE_SETREUID(-1, ltpuser->pw_uid);
+
+ 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);
+ }
+
+ tst_reap_children();
+}
+
+static void cleanup(void)
+{
+ 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,
+ .forks_child = 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
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [LTP] [PATCH v1 1/2] syscalls/creat09: Add umask condition
2022-07-22 10:27 [LTP] [PATCH v1 1/2] syscalls/creat09: Add umask condition Yang Xu
2022-07-22 10:27 ` [LTP] [PATCH v1 2/2] syscalls/openat: Add new regression test when using open(O_TMPFILE) under umask Yang Xu
@ 2022-08-02 17:26 ` Petr Vorel
2022-08-03 1:36 ` xuyang2018.jy
1 sibling, 1 reply; 4+ messages in thread
From: Petr Vorel @ 2022-08-02 17:26 UTC (permalink / raw)
To: Yang Xu; +Cc: brauner, Martin Doucha, ltp
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
^ permalink raw reply [flat|nested] 4+ messages in thread