* [LTP] [PATCH v2 1/2] syscalls/creat09: Add umask test condition @ 2022-08-03 3:24 Yang Xu 2022-08-03 3:24 ` [LTP] [PATCH v2 2/2] syscalls/openat: Add new regression test when using open(O_TMPFILE) under umask Yang Xu ` (3 more replies) 0 siblings, 4 replies; 17+ messages in thread From: Yang Xu @ 2022-08-03 3:24 UTC (permalink / raw) To: ltp; +Cc: brauner, martin.doucha A kernel patch set that fix setgid strip logic under umask(S_IXGRP) found by this case has been merged into linux-next branch[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/next/linux-next.git/commit/?h=next-20220802&id=1639a49ccdce [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> --- v1-v2: 1.update linux-next and xfstests url 2.use Ternary Operator instead of switch or if testcases/kernel/syscalls/creat/creat09.c | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/testcases/kernel/syscalls/creat/creat09.c b/testcases/kernel/syscalls/creat/creat09.c index bed7bddb0..7077cbcff 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,11 @@ 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) { + umask(n ? S_IXGRP : 0); + tst_res(TINFO, "under umask(%s) situation", n ? "S_IXGRP" : "0"); + fd = SAFE_CREAT(CREAT_FILE, MODE_SGID); SAFE_CLOSE(fd); file_test(CREAT_FILE); @@ -115,13 +128,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 +146,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 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [LTP] [PATCH v2 2/2] syscalls/openat: Add new regression test when using open(O_TMPFILE) under umask 2022-08-03 3:24 [LTP] [PATCH v2 1/2] syscalls/creat09: Add umask test condition Yang Xu @ 2022-08-03 3:24 ` Yang Xu 2022-08-04 16:08 ` Martin Doucha 2022-08-03 7:49 ` [LTP] [PATCH v2 1/2] syscalls/creat09: Add umask test condition Christian Brauner ` (2 subsequent siblings) 3 siblings, 1 reply; 17+ messages in thread From: Yang Xu @ 2022-08-03 3:24 UTC (permalink / raw) To: ltp; +Cc: brauner, martin.doucha Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com> --- v1-v2: no change 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] 17+ messages in thread
* Re: [LTP] [PATCH v2 2/2] syscalls/openat: Add new regression test when using open(O_TMPFILE) under umask 2022-08-03 3:24 ` [LTP] [PATCH v2 2/2] syscalls/openat: Add new regression test when using open(O_TMPFILE) under umask Yang Xu @ 2022-08-04 16:08 ` Martin Doucha 2022-08-04 20:32 ` Petr Vorel ` (2 more replies) 0 siblings, 3 replies; 17+ messages in thread From: Martin Doucha @ 2022-08-04 16:08 UTC (permalink / raw) To: Yang Xu, ltp; +Cc: brauner Hi, On 03. 08. 22 5:24, Yang Xu wrote: > Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com> > --- > v1-v2: > no change > 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); > + } > + The tst_brk() calls above and below are identical. You can either remove the one above, or change the error message to "Invalid return value" below. > + 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); > + } What if openat() returns some other negative value? > + > + 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(); You don't need to fork() here. Just change EUID/GID at the end of setup() like in creat09 and then change EUID back at the beginning of cleanup(). > + 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"}, > + {} > + }, > +}; -- Martin Doucha mdoucha@suse.cz QA Engineer for Software Maintenance SUSE LINUX, s.r.o. CORSO IIa Krizikova 148/34 186 00 Prague 8 Czech Republic -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [LTP] [PATCH v2 2/2] syscalls/openat: Add new regression test when using open(O_TMPFILE) under umask 2022-08-04 16:08 ` Martin Doucha @ 2022-08-04 20:32 ` Petr Vorel 2022-08-15 7:58 ` xuyang2018.jy 2022-08-15 7:41 ` xuyang2018.jy 2022-08-15 9:27 ` [LTP] [PATCH v3 1/2] syscalls/creat09: Add umask test condition Yang Xu 2 siblings, 1 reply; 17+ messages in thread From: Petr Vorel @ 2022-08-04 20:32 UTC (permalink / raw) To: Martin Doucha; +Cc: brauner, ltp Hi all, ... > > +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); > > + } > What if openat() returns some other negative value? How about add ENOTSUP to safe_openat() (lib/tst_safe_file_at.c) and use SAFE_OPENAT() here? Kind regards, Petr > > + > > + SAFE_CLOSE(TST_RET); > > +} ... -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [LTP] [PATCH v2 2/2] syscalls/openat: Add new regression test when using open(O_TMPFILE) under umask 2022-08-04 20:32 ` Petr Vorel @ 2022-08-15 7:58 ` xuyang2018.jy 0 siblings, 0 replies; 17+ messages in thread From: xuyang2018.jy @ 2022-08-15 7:58 UTC (permalink / raw) To: Petr Vorel, Martin Doucha; +Cc: brauner, ltp Hi Petr > Hi all, > > ... >>> +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); >>> + } > >> What if openat() returns some other negative value? > How about add ENOTSUP to safe_openat() (lib/tst_safe_file_at.c) and use SAFE_OPENAT() here? I am fine with this. Best Regards Yang Xu > > Kind regards, > Petr > >>> + >>> + SAFE_CLOSE(TST_RET); >>> +} > ... -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [LTP] [PATCH v2 2/2] syscalls/openat: Add new regression test when using open(O_TMPFILE) under umask 2022-08-04 16:08 ` Martin Doucha 2022-08-04 20:32 ` Petr Vorel @ 2022-08-15 7:41 ` xuyang2018.jy 2022-08-15 9:27 ` [LTP] [PATCH v3 1/2] syscalls/creat09: Add umask test condition Yang Xu 2 siblings, 0 replies; 17+ messages in thread From: xuyang2018.jy @ 2022-08-15 7:41 UTC (permalink / raw) To: Martin Doucha, ltp; +Cc: brauner Hi Martin > Hi, > > On 03. 08. 22 5:24, Yang Xu wrote: >> Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com> >> --- >> v1-v2: >> no change >> 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); >> + } >> + > > The tst_brk() calls above and below are identical. You can either remove > the one above, or change the error message to "Invalid return value" below. Oh, yes, will change the error message to "Invalid return value". > >> + 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); >> + } > > What if openat() returns some other negative value? Will add it for invalid return value. > >> + >> + 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(); > > You don't need to fork() here. Just change EUID/GID at the end of > setup() like in creat09 and then change EUID back at the beginning of > cleanup(). Yes. Best Regards Yang Xu > >> + 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"}, >> + {} >> + }, >> +}; > > -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 17+ messages in thread
* [LTP] [PATCH v3 1/2] syscalls/creat09: Add umask test condition 2022-08-04 16:08 ` Martin Doucha 2022-08-04 20:32 ` Petr Vorel 2022-08-15 7:41 ` xuyang2018.jy @ 2022-08-15 9:27 ` Yang Xu 2022-08-15 9:21 ` Christian Brauner 2022-08-15 9:27 ` [LTP] [PATCH v3 2/2] syscalls/openat: Add new regression test when using open(O_TMPFILE) under umask Yang Xu 2 siblings, 2 replies; 17+ messages in thread From: Yang Xu @ 2022-08-15 9:27 UTC (permalink / raw) To: ltp; +Cc: brauner 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 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [LTP] [PATCH v3 1/2] syscalls/creat09: Add umask test condition 2022-08-15 9:27 ` [LTP] [PATCH v3 1/2] syscalls/creat09: Add umask test condition Yang Xu @ 2022-08-15 9:21 ` Christian Brauner 2022-08-15 9:27 ` [LTP] [PATCH v3 2/2] syscalls/openat: Add new regression test when using open(O_TMPFILE) under umask Yang Xu 1 sibling, 0 replies; 17+ messages in thread From: Christian Brauner @ 2022-08-15 9:21 UTC (permalink / raw) To: Yang Xu; +Cc: ltp On Mon, Aug 15, 2022 at 05:27:06PM +0800, Yang Xu wrote: > 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. I just realized that before I added the idmapped mounts testsuite into xfstests which tests setgid inheritance in ~April 2021 tests like this didn't exist in LTP apparently as well. Interesting. I was wondering why setgid inheritance bugs hadn't been caught by it. :) Great to expand them! > > 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> > --- Reviewed-by: Christian Brauner (Microsoft) <brauner@kernel.org> > 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* [LTP] [PATCH v3 2/2] syscalls/openat: Add new regression test when using open(O_TMPFILE) under umask 2022-08-15 9:27 ` [LTP] [PATCH v3 1/2] syscalls/creat09: Add umask test condition Yang Xu 2022-08-15 9:21 ` Christian Brauner @ 2022-08-15 9:27 ` Yang Xu 2022-08-31 6:09 ` xuyang2018.jy 2022-09-13 11:42 ` Cyril Hrubis 1 sibling, 2 replies; 17+ messages in thread From: Yang Xu @ 2022-08-15 9: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 | 188 ++++++++++++++++++++ 3 files changed, 190 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..78deaa6f0 --- /dev/null +++ b/testcases/kernel/syscalls/openat/openat04.c @@ -0,0 +1,188 @@ +// 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); + } else if (TST_RET) { + tst_brk(TBROK, "Invalid 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, "open(%d, O_TMPFILE) failed", dirfd); + } else if (TST_RET < 0) { + tst_brk(TBROK, "Invalid 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; + + 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) +{ + 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 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [LTP] [PATCH v3 2/2] syscalls/openat: Add new regression test when using open(O_TMPFILE) under umask 2022-08-15 9:27 ` [LTP] [PATCH v3 2/2] syscalls/openat: Add new regression test when using open(O_TMPFILE) under umask Yang Xu @ 2022-08-31 6:09 ` xuyang2018.jy 2022-09-13 11:42 ` Cyril Hrubis 1 sibling, 0 replies; 17+ messages in thread From: xuyang2018.jy @ 2022-08-31 6:09 UTC (permalink / raw) To: ltp; +Cc: brauner Hi All I want to merge this patch set together, but this patch did't get any comment or Reviewed-by tag. So ping! Best Regards 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 | 188 ++++++++++++++++++++ > 3 files changed, 190 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..78deaa6f0 > --- /dev/null > +++ b/testcases/kernel/syscalls/openat/openat04.c > @@ -0,0 +1,188 @@ > +// 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); > + } else if (TST_RET) { > + tst_brk(TBROK, "Invalid 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, "open(%d, O_TMPFILE) failed", dirfd); > + } else if (TST_RET < 0) { > + tst_brk(TBROK, "Invalid 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; > + > + 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) > +{ > + 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"}, > + {} > + }, > +}; -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [LTP] [PATCH v3 2/2] syscalls/openat: Add new regression test when using open(O_TMPFILE) under umask 2022-08-15 9:27 ` [LTP] [PATCH v3 2/2] syscalls/openat: Add new regression test when using open(O_TMPFILE) under umask Yang Xu 2022-08-31 6:09 ` xuyang2018.jy @ 2022-09-13 11:42 ` Cyril Hrubis 2022-09-14 5:49 ` xuyang2018.jy 1 sibling, 1 reply; 17+ messages in thread From: Cyril Hrubis @ 2022-09-13 11:42 UTC (permalink / raw) To: Yang Xu; +Cc: brauner, ltp Hi! > +// 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 ^ POSIX ACL Both of these are acronyms and should be spelled with uppercase. > + * 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); > + } else if (TST_RET) { There is no need for else if we do tst_brk() in the previous if () > + tst_brk(TBROK, "Invalid 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, "open(%d, O_TMPFILE) failed", dirfd); ^ openat > + } else if (TST_RET < 0) { Here as well. > + tst_brk(TBROK, "Invalid return value %ld", TST_RET); ^ openat() > + } > + > + 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; > + > + TST_EXP_PASS_SILENT(fstatat(dfd, path, &buf, flags)); > + if (!TST_PASS) { > + tst_res(TFAIL, "fstat failed"); > + return; > + } If nothing else this part is really ugly, it's a misuse of the TST_EXP_PASS_SILENT() macro and you even print the TFAIL message manually for the second time. This should really be replaced with SAFE_FSTATAT() after a patch that adds SAFE_FSTATAT() to the test library. > + 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); > + } 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 -- Cyril Hrubis chrubis@suse.cz -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [LTP] [PATCH v3 2/2] syscalls/openat: Add new regression test when using open(O_TMPFILE) under umask 2022-09-13 11:42 ` Cyril Hrubis @ 2022-09-14 5:49 ` xuyang2018.jy 0 siblings, 0 replies; 17+ messages in thread From: xuyang2018.jy @ 2022-09-14 5:49 UTC (permalink / raw) To: Cyril Hrubis; +Cc: brauner, ltp Hi Cyril > Hi! >> +// 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 > ^ > POSIX ACL > > Both of these are acronyms and should be spelled with uppercase. Yes. > >> + * 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); >> + } else if (TST_RET) { > > There is no need for else if we do tst_brk() in the previous if () Yes. > >> + tst_brk(TBROK, "Invalid 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, "open(%d, O_TMPFILE) failed", dirfd); > ^ > openat >> + } else if (TST_RET < 0) { > > Here as well. > >> + tst_brk(TBROK, "Invalid return value %ld", TST_RET); > ^ > openat() Yes, Will add. >> + } >> + >> + 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; >> + >> + TST_EXP_PASS_SILENT(fstatat(dfd, path, &buf, flags)); >> + if (!TST_PASS) { >> + tst_res(TFAIL, "fstat failed"); >> + return; >> + } > > If nothing else this part is really ugly, it's a misuse of the > TST_EXP_PASS_SILENT() macro and you even print the TFAIL message > manually for the second time. > > This should really be replaced with SAFE_FSTATAT() after a patch that > adds SAFE_FSTATAT() to the test library. Will add this SAFE macro. > >> + 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); >> + } > > TST_EXP_EQ_LI(buf.st_gid, free_gid); Yes. Best Regards Yang Xu > >> + 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 > -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [LTP] [PATCH v2 1/2] syscalls/creat09: Add umask test condition 2022-08-03 3:24 [LTP] [PATCH v2 1/2] syscalls/creat09: Add umask test condition Yang Xu 2022-08-03 3:24 ` [LTP] [PATCH v2 2/2] syscalls/openat: Add new regression test when using open(O_TMPFILE) under umask Yang Xu @ 2022-08-03 7:49 ` Christian Brauner 2022-08-03 8:06 ` xuyang2018.jy 2022-08-04 15:47 ` Martin Doucha 2022-08-15 9:32 ` Christian Brauner 3 siblings, 1 reply; 17+ messages in thread From: Christian Brauner @ 2022-08-03 7:49 UTC (permalink / raw) To: Yang Xu; +Cc: martin.doucha, ltp, sforshee On Wed, Aug 03, 2022 at 11:24:22AM +0800, Yang Xu wrote: > A kernel patch set that fix setgid strip logic under umask(S_IXGRP) found by > this case has been merged into linux-next branch[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/next/linux-next.git/commit/?h=next-20220802&id=1639a49ccdce > [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> Fyi, I have this Thursday and Friday off which is why I haven't sent the pull request for setgid changes to Linus yet. I only sent the ones that I thought were less likely to cause regressions. I don't want to send a PR and then not be around to respond to issues. So I will send the setgid PR on Tuesday or Wednesday next week. Just a heads up! Christian -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [LTP] [PATCH v2 1/2] syscalls/creat09: Add umask test condition 2022-08-03 7:49 ` [LTP] [PATCH v2 1/2] syscalls/creat09: Add umask test condition Christian Brauner @ 2022-08-03 8:06 ` xuyang2018.jy 0 siblings, 0 replies; 17+ messages in thread From: xuyang2018.jy @ 2022-08-03 8:06 UTC (permalink / raw) To: Christian Brauner; +Cc: martin.doucha, ltp, sforshee on 2022/08/03 15:49, Christian Brauner wrote: > On Wed, Aug 03, 2022 at 11:24:22AM +0800, Yang Xu wrote: >> A kernel patch set that fix setgid strip logic under umask(S_IXGRP) found by >> this case has been merged into linux-next branch[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/next/linux-next.git/commit/?h=next-20220802&id=1639a49ccdce >> [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> > > Fyi, I have this Thursday and Friday off which is why I haven't sent the > pull request for setgid changes to Linus yet. I only sent the ones that > I thought were less likely to cause regressions. I don't want to send a > PR and then not be around to respond to issues. So I will send the > setgid PR on Tuesday or Wednesday next week. Just a heads up! Glad to know this. Thanks! Best Regards Yang Xu > > Christian -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [LTP] [PATCH v2 1/2] syscalls/creat09: Add umask test condition 2022-08-03 3:24 [LTP] [PATCH v2 1/2] syscalls/creat09: Add umask test condition Yang Xu 2022-08-03 3:24 ` [LTP] [PATCH v2 2/2] syscalls/openat: Add new regression test when using open(O_TMPFILE) under umask Yang Xu 2022-08-03 7:49 ` [LTP] [PATCH v2 1/2] syscalls/creat09: Add umask test condition Christian Brauner @ 2022-08-04 15:47 ` Martin Doucha 2022-08-05 11:13 ` xuyang2018.jy 2022-08-15 9:32 ` Christian Brauner 3 siblings, 1 reply; 17+ messages in thread From: Martin Doucha @ 2022-08-04 15:47 UTC (permalink / raw) To: Yang Xu, ltp; +Cc: brauner, martin.doucha Hi, On 03. 08. 22 5:24, Yang Xu wrote: > A kernel patch set that fix setgid strip logic under umask(S_IXGRP) found by > this case has been merged into linux-next branch[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/next/linux-next.git/commit/?h=next-20220802&id=1639a49ccdce > [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> > --- > v1-v2: > 1.update linux-next and xfstests url > 2.use Ternary Operator instead of switch or if > testcases/kernel/syscalls/creat/creat09.c | 19 +++++++++++++++++-- > 1 file changed, 17 insertions(+), 2 deletions(-) > > diff --git a/testcases/kernel/syscalls/creat/creat09.c b/testcases/kernel/syscalls/creat/creat09.c > index bed7bddb0..7077cbcff 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,11 @@ 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) > { > + umask(n ? S_IXGRP : 0); > + tst_res(TINFO, "under umask(%s) situation", n ? "S_IXGRP" : "0"); It'd be much cleaner to use an array of testcase structures and then call: tst_res(TINFO, testcase_list[n].name); umask(testcase_list[n].mask); ... .tcnt = ARRAY_SIZE(testcase_list), See also creat04 for example. > + > fd = SAFE_CREAT(CREAT_FILE, MODE_SGID); > SAFE_CLOSE(fd); > file_test(CREAT_FILE); > @@ -115,13 +128,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 +146,7 @@ static struct tst_test test = { > {"linux-git", "0fa3ecd87848"}, > {"CVE", "2018-13405"}, > {"linux-git", "01ea173e103e"}, > + {"linux-git", "1639a49ccdce"}, > {} > }, > }; -- Martin Doucha mdoucha@suse.cz QA Engineer for Software Maintenance SUSE LINUX, s.r.o. CORSO IIa Krizikova 148/34 186 00 Prague 8 Czech Republic -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [LTP] [PATCH v2 1/2] syscalls/creat09: Add umask test condition 2022-08-04 15:47 ` Martin Doucha @ 2022-08-05 11:13 ` xuyang2018.jy 0 siblings, 0 replies; 17+ messages in thread From: xuyang2018.jy @ 2022-08-05 11:13 UTC (permalink / raw) To: Martin Doucha, ltp; +Cc: brauner, martin.doucha Hi Martin Thanks for your review. I have a holiday next week, so will send v2 when I come back. Best Regards Yang Xu > Hi, > > On 03. 08. 22 5:24, Yang Xu wrote: >> A kernel patch set that fix setgid strip logic under umask(S_IXGRP) found by >> this case has been merged into linux-next branch[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/next/linux-next.git/commit/?h=next-20220802&id=1639a49ccdce >> [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> >> --- >> v1-v2: >> 1.update linux-next and xfstests url >> 2.use Ternary Operator instead of switch or if >> testcases/kernel/syscalls/creat/creat09.c | 19 +++++++++++++++++-- >> 1 file changed, 17 insertions(+), 2 deletions(-) >> >> diff --git a/testcases/kernel/syscalls/creat/creat09.c b/testcases/kernel/syscalls/creat/creat09.c >> index bed7bddb0..7077cbcff 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,11 @@ 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) >> { >> + umask(n ? S_IXGRP : 0); >> + tst_res(TINFO, "under umask(%s) situation", n ? "S_IXGRP" : "0"); > > It'd be much cleaner to use an array of testcase structures and then call: > tst_res(TINFO, testcase_list[n].name); > umask(testcase_list[n].mask); > > ... > > .tcnt = ARRAY_SIZE(testcase_list), > > See also creat04 for example. > >> + >> fd = SAFE_CREAT(CREAT_FILE, MODE_SGID); >> SAFE_CLOSE(fd); >> file_test(CREAT_FILE); >> @@ -115,13 +128,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 +146,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] 17+ messages in thread
* Re: [LTP] [PATCH v2 1/2] syscalls/creat09: Add umask test condition 2022-08-03 3:24 [LTP] [PATCH v2 1/2] syscalls/creat09: Add umask test condition Yang Xu ` (2 preceding siblings ...) 2022-08-04 15:47 ` Martin Doucha @ 2022-08-15 9:32 ` Christian Brauner 3 siblings, 0 replies; 17+ messages in thread From: Christian Brauner @ 2022-08-15 9:32 UTC (permalink / raw) To: Yang Xu; +Cc: martin.doucha, ltp On Wed, Aug 03, 2022 at 11:24:22AM +0800, Yang Xu wrote: > A kernel patch set that fix setgid strip logic under umask(S_IXGRP) found by > this case has been merged into linux-next branch[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/next/linux-next.git/commit/?h=next-20220802&id=1639a49ccdce > [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> > --- Looks good to me, Acked-by: Christian Brauner (Microsoft) <brauner@kernel.org> > v1-v2: > 1.update linux-next and xfstests url > 2.use Ternary Operator instead of switch or if > testcases/kernel/syscalls/creat/creat09.c | 19 +++++++++++++++++-- > 1 file changed, 17 insertions(+), 2 deletions(-) > > diff --git a/testcases/kernel/syscalls/creat/creat09.c b/testcases/kernel/syscalls/creat/creat09.c > index bed7bddb0..7077cbcff 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,11 @@ 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) > { > + umask(n ? S_IXGRP : 0); > + tst_res(TINFO, "under umask(%s) situation", n ? "S_IXGRP" : "0"); > + > fd = SAFE_CREAT(CREAT_FILE, MODE_SGID); > SAFE_CLOSE(fd); > file_test(CREAT_FILE); > @@ -115,13 +128,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 +146,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 ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2022-09-14 5:49 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-08-03 3:24 [LTP] [PATCH v2 1/2] syscalls/creat09: Add umask test condition Yang Xu 2022-08-03 3:24 ` [LTP] [PATCH v2 2/2] syscalls/openat: Add new regression test when using open(O_TMPFILE) under umask Yang Xu 2022-08-04 16:08 ` Martin Doucha 2022-08-04 20:32 ` Petr Vorel 2022-08-15 7:58 ` xuyang2018.jy 2022-08-15 7:41 ` xuyang2018.jy 2022-08-15 9:27 ` [LTP] [PATCH v3 1/2] syscalls/creat09: Add umask test condition Yang Xu 2022-08-15 9:21 ` Christian Brauner 2022-08-15 9:27 ` [LTP] [PATCH v3 2/2] syscalls/openat: Add new regression test when using open(O_TMPFILE) under umask Yang Xu 2022-08-31 6:09 ` xuyang2018.jy 2022-09-13 11:42 ` Cyril Hrubis 2022-09-14 5:49 ` xuyang2018.jy 2022-08-03 7:49 ` [LTP] [PATCH v2 1/2] syscalls/creat09: Add umask test condition Christian Brauner 2022-08-03 8:06 ` xuyang2018.jy 2022-08-04 15:47 ` Martin Doucha 2022-08-05 11:13 ` xuyang2018.jy 2022-08-15 9:32 ` Christian Brauner
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.