[LTP] [PATCH v13 3/3] open16: allow restricted O_CREAT of FIFOs and regular files
Petr Vorel
pvorel@suse.cz
Wed Sep 16 15:01:39 CEST 2026
Hi Wei,
> Add LTP coverage for kernel commit 30aba6656f61 (Linux 4.19), which
> introduced protection against spoofing attacks via O_CREAT of FIFOs
> and regular files in world-writable or group-writable sticky
> directories.
> This commit adds test cases to verify these security restrictions
> (Level 1 and Level 2 protections) for opening FIFOs and regular
> files in world-writable or group-writable sticky directories when the
> file is not owned by the opener.
I was thinking what level you're talking about.
...
> diff --git a/testcases/kernel/syscalls/open/open16.c b/testcases/kernel/syscalls/open/open16.c
> new file mode 100644
> index 000000000..3d29a56ca
> --- /dev/null
> +++ b/testcases/kernel/syscalls/open/open16.c
> @@ -0,0 +1,133 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2026 Wei Gao <wegao@suse.com>
> + */
> +
> +/*\
> + * Verify restricted opening (:manpage:`open(2)` and :manpage:`openat(2)`) of
> + * FIFOs and regular files in sticky directories. This test covers the positive
> + * case where access is allowed when protection is disabled (level 0), and the
> + * negative cases where access is disallowed (EACCES) in world-writable (level
> + * 1) or group-writable (level 2) sticky directories when the file is not owned
> + * by the opener.
Even here it's not clear. Could you just mention that these are
/proc/sys/fs/protected_fifos values? man proc_sys_fs(5) which document it does
not talk about levels at all.
> + *
> + * This test requires root to modify /proc/sys/fs/protected_* sysctls and
> + * to manage file ownership and permissions in sticky directories.
> + */
> +
...
> +#define PROTECTED_REGULAR "/proc/sys/fs/protected_regular"
> +#define PROTECTED_FIFOS "/proc/sys/fs/protected_fifos"
Could you please add these 2 into include/tst_path_defs.h?
> +#define TEST_FIFO_PATH DIR "/" TEST_FIFO
> +
> +static int dir_fd = -1;
> +static uid_t uid1, uid2;
> +static gid_t gid1;
> +
> +static struct tcase {
> + char *level;
How about define level as int?
I would even not define it and use unsigned int n in verify_open() since the
values are the same, but up to you.
> + int exp_errno;
> + uid_t owner_uid;
> + int use_nobody_gid;
> + mode_t dir_mode;
> +} tcases[] = {
> + {"0", 0, 0, 0, 0777 | S_ISVTX},
> + {"1", EACCES, 0, 0, 0777 | S_ISVTX},
> + {"2", EACCES, -1, 1, 0030 | S_ISVTX},
> +};
I usually prefer using designated initializers to
1) not having to specify 0
2)
> +
> +static void verify_open(unsigned int n)
> +{
> + struct tcase *tc = &tcases[n];
> + pid_t pid;
> +
> + SAFE_FILE_PRINTF(PROTECTED_REGULAR, "%s", tc->level);
> + SAFE_FILE_PRINTF(PROTECTED_FIFOS, "%s", tc->level);
> +
> + if (tc->owner_uid != (uid_t)-1 || tc->use_nobody_gid) {
> + gid_t gid = tc->use_nobody_gid ? gid1 : 0;
> +
> + SAFE_CHOWN(DIR, tc->owner_uid, gid);
> + }
> +
> + if (tc->dir_mode)
> + SAFE_CHMOD(DIR, tc->dir_mode);
> +
> + pid = SAFE_FORK();
> + if (!pid) {
How about:
1) using SAFE_FORK() directly
2) return for parent to save indent (readability)
if (SAFE_FORK())
return;
SAFE_SETGID(gid1);
SAFE_SETUID(uid2);
...
> + SAFE_SETGID(gid1);
> + SAFE_SETUID(uid2);
> +
> + if (tc->exp_errno) {
> + TST_EXP_FAIL2(openat(dir_fd, TEST_FILE, O_RDWR | O_CREAT, 0777),
> + tc->exp_errno, "openat %s (Level %s)", TEST_FILE, tc->level);
> + TST_EXP_FAIL2(open(TEST_FIFO_PATH, O_RDWR | O_CREAT, 0777),
> + tc->exp_errno, "open %s (Level %s)", TEST_FIFO, tc->level);
> + } else {
> + int fd = TST_EXP_FD(openat(dir_fd, TEST_FILE, O_CREAT | O_RDWR, 0777));
nit: we don't have to define fd when it's not used, we can use TST_RET.
> +
> + if (TST_PASS)
> + SAFE_CLOSE(fd);
> +
> + fd = TST_EXP_FD(open(TEST_FIFO_PATH, O_RDWR | O_CREAT, 0777));
> + if (TST_PASS)
> + SAFE_CLOSE(fd);
> + }
This else branch use the same code, how about to use TST_EXP_FD_OR_FAIL()?
TST_EXP_FD_OR_FAIL(openat(dir_fd, TEST_FILE, O_RDWR | O_CREAT, 0777),
tc->exp_errno, "openat %s (Level %s)", TEST_FILE, tc->level);
if (TST_RET != -1)
SAFE_CLOSE(TST_RET);
TST_EXP_FD_OR_FAIL(open(TEST_FIFO_PATH, O_RDWR | O_CREAT, 0777),
tc->exp_errno, "open %s (Level %s)", TEST_FIFO, tc->level);
if (TST_RET != -1)
SAFE_CLOSE(TST_RET);
@Cyril @Li: I always wondered if TST_EXP_FD_OR_FAIL() should use TST_EXP_FAIL2().
> +
> + exit(0);
I guess we need to bother with exit(0) at this point, right?
> + }
> +}
> +
> +static void setup(void)
> +{
> + struct passwd *pw;
> +
> + pw = SAFE_GETPWNAM("nobody");
> + uid1 = pw->pw_uid;
> + gid1 = pw->pw_gid;
> + uid2 = tst_get_free_uid(uid1);
> +
> + umask(0);
> + SAFE_MKDIR(DIR, 0777 | S_ISVTX);
> + dir_fd = SAFE_OPEN(DIR, O_DIRECTORY);
> +
> + int fd = SAFE_OPENAT(dir_fd, TEST_FILE, O_CREAT | O_RDWR, 0777);
> +
> + SAFE_CLOSE(fd);
> + SAFE_MKFIFO(TEST_FIFO_PATH, 0777);
> + SAFE_CHOWN(TEST_FIFO_PATH, uid1, gid1);
> + SAFE_CHOWN(DIR "/" TEST_FILE, uid1, gid1);
> +}
> +
> +static void cleanup(void)
> +{
> + if (dir_fd != -1)
> + SAFE_CLOSE(dir_fd);
> +}
> +
> +static struct tst_test test = {
> + .setup = setup,
> + .cleanup = cleanup,
> + .needs_root = 1,
> + .tcnt = ARRAY_SIZE(tcases),
> + .test = verify_open,
> + .needs_tmpdir = 1,
> + .forks_child = 1,
> + .save_restore = (const struct tst_path_val[]) {
> + {PROTECTED_REGULAR, NULL, TST_SR_TCONF},
> + {PROTECTED_FIFOS, NULL, TST_SR_TCONF},
> + {}
> + },
> + .tags = (const struct tst_tag[]) {
> + {"linux-git", "30aba6656f61"},
30aba6656f61 notes many CVEs, IMHO we should add them as well:
CVE-2000-1134
CVE-2007-3852
CVE-2008-0525
CVE-2009-0416
CVE-2011-4834
CVE-2015-1838
CVE-2015-7442
CVE-2016-7489
> + {}
> + }
> +};
Feel free to speedup with this (still TODO CVE and using int for "level").
Kind regards,
Petr
diff --git testcases/kernel/syscalls/open/open16.c testcases/kernel/syscalls/open/open16.c
index 3d29a56ca7..0741102103 100644
--- testcases/kernel/syscalls/open/open16.c
+++ testcases/kernel/syscalls/open/open16.c
@@ -47,7 +47,6 @@ static struct tcase {
static void verify_open(unsigned int n)
{
struct tcase *tc = &tcases[n];
- pid_t pid;
SAFE_FILE_PRINTF(PROTECTED_REGULAR, "%s", tc->level);
SAFE_FILE_PRINTF(PROTECTED_FIFOS, "%s", tc->level);
@@ -61,29 +60,21 @@ static void verify_open(unsigned int n)
if (tc->dir_mode)
SAFE_CHMOD(DIR, tc->dir_mode);
- pid = SAFE_FORK();
- if (!pid) {
- SAFE_SETGID(gid1);
- SAFE_SETUID(uid2);
+ if (SAFE_FORK())
+ return;
- if (tc->exp_errno) {
- TST_EXP_FAIL2(openat(dir_fd, TEST_FILE, O_RDWR | O_CREAT, 0777),
- tc->exp_errno, "openat %s (Level %s)", TEST_FILE, tc->level);
- TST_EXP_FAIL2(open(TEST_FIFO_PATH, O_RDWR | O_CREAT, 0777),
- tc->exp_errno, "open %s (Level %s)", TEST_FIFO, tc->level);
- } else {
- int fd = TST_EXP_FD(openat(dir_fd, TEST_FILE, O_CREAT | O_RDWR, 0777));
+ SAFE_SETGID(gid1);
+ SAFE_SETUID(uid2);
- if (TST_PASS)
- SAFE_CLOSE(fd);
+ TST_EXP_FD_OR_FAIL(openat(dir_fd, TEST_FILE, O_RDWR | O_CREAT, 0777),
+ tc->exp_errno, "openat %s (Level %s)", TEST_FILE, tc->level);
+ if (TST_RET != -1)
+ SAFE_CLOSE(TST_RET);
- fd = TST_EXP_FD(open(TEST_FIFO_PATH, O_RDWR | O_CREAT, 0777));
- if (TST_PASS)
- SAFE_CLOSE(fd);
- }
-
- exit(0);
- }
+ TST_EXP_FD_OR_FAIL(open(TEST_FIFO_PATH, O_RDWR | O_CREAT, 0777),
+ tc->exp_errno, "open %s (Level %s)", TEST_FIFO, tc->level);
+ if (TST_RET != -1)
+ SAFE_CLOSE(TST_RET);
}
static void setup(void)
More information about the ltp
mailing list