[LTP] [PATCH v2 2/2] syscalls/openat: Add new regression test when using open(O_TMPFILE) under umask
xuyang2018.jy@fujitsu.com
xuyang2018.jy@fujitsu.com
Mon Aug 15 09:41:26 CEST 2022
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"},
>> + {}
>> + },
>> +};
>
>
More information about the ltp
mailing list