* [PATCHv2 0/3] fs: add O_BENEATH flag to openat(2)
@ 2014-11-04 9:54 David Drysdale
2014-11-04 9:54 ` [PATCHv2 1/3] " David Drysdale
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: David Drysdale @ 2014-11-04 9:54 UTC (permalink / raw)
To: linux-kernel, Alexander Viro, Kees Cook, Eric W. Biederman
Cc: Greg Kroah-Hartman, Meredydd Luff, Will Drewry,
Jorge Lucangeli Obes, Ricky Zhou, Lee Campbell, Julien Tinnes,
Mike Depinet, James Morris, Andy Lutomirski, Paolo Bonzini,
Paul Moore, Christoph Hellwig, linux-api, linux-security-module,
David Drysdale
This change adds a new O_BENEATH flag for openat(2) which restricts the
provided path, rejecting (with -EACCES) paths that are not beneath
the provided dfd.
This change was previously included as part of a larger patchset
(https://lkml.org/lkml/2014/7/25/426) for Capsicum support; however, it
is potentially useful as an independent change so I've pulled it out
separately here.
In particular, various folks from Chrome[OS] have indicated an interest
in having this functionality -- when combined with a seccomp filter it
allows a directory to be accessed by a sandboxed process.
Changes since v1:
- Don't needlessly duplicate flags [Al Viro]
- Use EPERM rather than EACCES as error code [Paolo Bonzini]
- Disallow nd_jump_link for O_BENEATH [Al Viro/Andy Lutomirski]
- Add test of a jumped symlink (/proc/self/root)
Changes since the version included in the Capsicum v2 patchset:
- Add tests of normal symlinks
- Fix man-page typo
- Update patch to 3.17
Changes from v1 to v2 of Capsicum patchset:
- renamed O_BENEATH_ONLY to O_BENEATH [Christoph Hellwig]
David Drysdale (2):
fs: add O_BENEATH flag to openat(2)
selftests: Add test of O_BENEATH & openat(2)
arch/alpha/include/uapi/asm/fcntl.h | 1 +
arch/parisc/include/uapi/asm/fcntl.h | 1 +
arch/sparc/include/uapi/asm/fcntl.h | 1 +
fs/fcntl.c | 5 +-
fs/namei.c | 43 ++++++---
fs/open.c | 4 +-
include/linux/namei.h | 1 +
include/uapi/asm-generic/fcntl.h | 4 +
tools/testing/selftests/Makefile | 1 +
tools/testing/selftests/openat/.gitignore | 3 +
tools/testing/selftests/openat/Makefile | 24 +++++
tools/testing/selftests/openat/openat.c | 149 ++++++++++++++++++++++++++++++
12 files changed, 220 insertions(+), 17 deletions(-)
create mode 100644 tools/testing/selftests/openat/.gitignore
create mode 100644 tools/testing/selftests/openat/Makefile
create mode 100644 tools/testing/selftests/openat/openat.c
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCHv2 1/3] fs: add O_BENEATH flag to openat(2) 2014-11-04 9:54 [PATCHv2 0/3] fs: add O_BENEATH flag to openat(2) David Drysdale @ 2014-11-04 9:54 ` David Drysdale 2014-11-04 9:54 ` [PATCHv2 2/3] selftests: Add test of O_BENEATH & openat(2) David Drysdale 2014-11-04 9:54 ` [PATCHv2 man-pages 3/3] open.2: describe O_BENEATH flag David Drysdale 2 siblings, 0 replies; 8+ messages in thread From: David Drysdale @ 2014-11-04 9:54 UTC (permalink / raw) To: linux-kernel, Alexander Viro, Kees Cook, Eric W. Biederman Cc: Greg Kroah-Hartman, Meredydd Luff, Will Drewry, Jorge Lucangeli Obes, Ricky Zhou, Lee Campbell, Julien Tinnes, Mike Depinet, James Morris, Andy Lutomirski, Paolo Bonzini, Paul Moore, Christoph Hellwig, linux-api, linux-security-module, David Drysdale Add a new O_BENEATH flag for openat(2) which restricts the provided path, rejecting (with -EACCES) paths that are not beneath the provided dfd. In particular, reject: - paths that contain .. components - paths that begin with / - symlinks that have paths as above. Also disallow use of nd_jump_link() for following symlinks without path expansion, when O_BENEATH is set. Signed-off-by: David Drysdale <drysdale@google.com> --- arch/alpha/include/uapi/asm/fcntl.h | 1 + arch/parisc/include/uapi/asm/fcntl.h | 1 + arch/sparc/include/uapi/asm/fcntl.h | 1 + fs/fcntl.c | 5 +++-- fs/namei.c | 21 ++++++++++++++++++--- fs/open.c | 4 +++- fs/proc/base.c | 4 +++- fs/proc/namespaces.c | 7 ++++++- include/linux/namei.h | 3 ++- include/uapi/asm-generic/fcntl.h | 4 ++++ 10 files changed, 42 insertions(+), 9 deletions(-) diff --git a/arch/alpha/include/uapi/asm/fcntl.h b/arch/alpha/include/uapi/asm/fcntl.h index 09f49a6b87d1..76a87038d2c1 100644 --- a/arch/alpha/include/uapi/asm/fcntl.h +++ b/arch/alpha/include/uapi/asm/fcntl.h @@ -33,6 +33,7 @@ #define O_PATH 040000000 #define __O_TMPFILE 0100000000 +#define O_BENEATH 0200000000 /* no / or .. in openat path */ #define F_GETLK 7 #define F_SETLK 8 diff --git a/arch/parisc/include/uapi/asm/fcntl.h b/arch/parisc/include/uapi/asm/fcntl.h index 34a46cbc76ed..3adadf72f929 100644 --- a/arch/parisc/include/uapi/asm/fcntl.h +++ b/arch/parisc/include/uapi/asm/fcntl.h @@ -21,6 +21,7 @@ #define O_PATH 020000000 #define __O_TMPFILE 040000000 +#define O_BENEATH 080000000 /* no / or .. in openat path */ #define F_GETLK64 8 #define F_SETLK64 9 diff --git a/arch/sparc/include/uapi/asm/fcntl.h b/arch/sparc/include/uapi/asm/fcntl.h index 7e8ace5bf760..ea38f0bd6cec 100644 --- a/arch/sparc/include/uapi/asm/fcntl.h +++ b/arch/sparc/include/uapi/asm/fcntl.h @@ -36,6 +36,7 @@ #define O_PATH 0x1000000 #define __O_TMPFILE 0x2000000 +#define O_BENEATH 0x4000000 /* no / or .. in openat path */ #define F_GETOWN 5 /* for sockets. */ #define F_SETOWN 6 /* for sockets. */ diff --git a/fs/fcntl.c b/fs/fcntl.c index 22d1c3df61ac..c07a32efc34b 100644 --- a/fs/fcntl.c +++ b/fs/fcntl.c @@ -747,14 +747,15 @@ static int __init fcntl_init(void) * Exceptions: O_NONBLOCK is a two bit define on parisc; O_NDELAY * is defined as O_NONBLOCK on some platforms and not on others. */ - BUILD_BUG_ON(20 - 1 /* for O_RDONLY being 0 */ != HWEIGHT32( + BUILD_BUG_ON(21 - 1 /* for O_RDONLY being 0 */ != HWEIGHT32( O_RDONLY | O_WRONLY | O_RDWR | O_CREAT | O_EXCL | O_NOCTTY | O_TRUNC | O_APPEND | /* O_NONBLOCK | */ __O_SYNC | O_DSYNC | FASYNC | O_DIRECT | O_LARGEFILE | O_DIRECTORY | O_NOFOLLOW | O_NOATIME | O_CLOEXEC | - __FMODE_EXEC | O_PATH | __O_TMPFILE + __FMODE_EXEC | O_PATH | __O_TMPFILE | + O_BENEATH )); fasync_cache = kmem_cache_create("fasync_cache", diff --git a/fs/namei.c b/fs/namei.c index a7b05bf82d31..01d1d97eab3e 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -685,13 +685,16 @@ static inline void path_to_nameidata(const struct path *path, * Helper to directly jump to a known parsed path from ->follow_link, * caller must have taken a reference to path beforehand. */ -void nd_jump_link(struct nameidata *nd, struct path *path) +int nd_jump_link(struct nameidata *nd, struct path *path) { + if (nd->flags & LOOKUP_BENEATH) + return -EPERM; path_put(&nd->path); nd->path = *path; nd->inode = nd->path.dentry->d_inode; nd->flags |= LOOKUP_JUMPED; + return 0; } static inline void put_link(struct nameidata *nd, struct path *link, void *cookie) @@ -1743,9 +1746,14 @@ static int link_path_walk(const char *name, struct nameidata *nd) { struct path next; int err; - - while (*name=='/') + + while (*name == '/') { + if (nd->flags & LOOKUP_BENEATH) { + err = -EPERM; + goto exit; + } name++; + } if (!*name) return 0; @@ -1764,6 +1772,10 @@ static int link_path_walk(const char *name, struct nameidata *nd) if (name[0] == '.') switch (hashlen_len(hash_len)) { case 2: if (name[1] == '.') { + if (nd->flags & LOOKUP_BENEATH) { + err = -EPERM; + goto exit; + } type = LAST_DOTDOT; nd->flags |= LOOKUP_JUMPED; } @@ -1815,6 +1827,7 @@ static int link_path_walk(const char *name, struct nameidata *nd) break; } } +exit: terminate_walk(nd); return err; } @@ -1853,6 +1866,8 @@ static int path_init(int dfd, const char *name, unsigned int flags, nd->m_seq = read_seqbegin(&mount_lock); if (*name=='/') { + if (flags & LOOKUP_BENEATH) + return -EPERM; if (flags & LOOKUP_RCU) { rcu_read_lock(); nd->seq = set_root_rcu(nd); diff --git a/fs/open.c b/fs/open.c index d6fd3acde134..8afca5b87a0b 100644 --- a/fs/open.c +++ b/fs/open.c @@ -874,7 +874,7 @@ static inline int build_open_flags(int flags, umode_t mode, struct open_flags *o * If we have O_PATH in the open flag. Then we * cannot have anything other than the below set of flags */ - flags &= O_DIRECTORY | O_NOFOLLOW | O_PATH; + flags &= O_DIRECTORY | O_NOFOLLOW | O_PATH | O_BENEATH; acc_mode = 0; } else { acc_mode = MAY_OPEN | ACC_MODE(flags); @@ -905,6 +905,8 @@ static inline int build_open_flags(int flags, umode_t mode, struct open_flags *o lookup_flags |= LOOKUP_DIRECTORY; if (!(flags & O_NOFOLLOW)) lookup_flags |= LOOKUP_FOLLOW; + if (flags & O_BENEATH) + lookup_flags |= LOOKUP_BENEATH; op->lookup_flags = lookup_flags; return 0; } diff --git a/fs/proc/base.c b/fs/proc/base.c index baf852b648ad..75d430155f9a 100644 --- a/fs/proc/base.c +++ b/fs/proc/base.c @@ -1410,7 +1410,9 @@ static void *proc_pid_follow_link(struct dentry *dentry, struct nameidata *nd) if (error) goto out; - nd_jump_link(nd, &path); + error = nd_jump_link(nd, &path); + if (error) + goto out; return NULL; out: return ERR_PTR(error); diff --git a/fs/proc/namespaces.c b/fs/proc/namespaces.c index 89026095f2b5..5cdbe5950756 100644 --- a/fs/proc/namespaces.c +++ b/fs/proc/namespaces.c @@ -113,6 +113,7 @@ static void *proc_ns_follow_link(struct dentry *dentry, struct nameidata *nd) struct proc_inode *ei = PROC_I(inode); struct task_struct *task; struct path ns_path; + int err; void *error = ERR_PTR(-EACCES); task = get_proc_task(inode); @@ -129,7 +130,11 @@ static void *proc_ns_follow_link(struct dentry *dentry, struct nameidata *nd) } ns_path.mnt = mntget(nd->path.mnt); - nd_jump_link(nd, &ns_path); + err = nd_jump_link(nd, &ns_path); + if (err) { + error = ERR_PTR(err); + goto out_put_task; + } error = NULL; out_put_task: diff --git a/include/linux/namei.h b/include/linux/namei.h index 492de72560fa..a018cd8219ec 100644 --- a/include/linux/namei.h +++ b/include/linux/namei.h @@ -39,6 +39,7 @@ enum {LAST_NORM, LAST_ROOT, LAST_DOT, LAST_DOTDOT, LAST_BIND}; #define LOOKUP_FOLLOW 0x0001 #define LOOKUP_DIRECTORY 0x0002 #define LOOKUP_AUTOMOUNT 0x0004 +#define LOOKUP_BENEATH 0x0008 #define LOOKUP_PARENT 0x0010 #define LOOKUP_REVAL 0x0020 @@ -81,7 +82,7 @@ extern int follow_up(struct path *); extern struct dentry *lock_rename(struct dentry *, struct dentry *); extern void unlock_rename(struct dentry *, struct dentry *); -extern void nd_jump_link(struct nameidata *nd, struct path *path); +extern int nd_jump_link(struct nameidata *nd, struct path *path); static inline void nd_set_link(struct nameidata *nd, char *path) { diff --git a/include/uapi/asm-generic/fcntl.h b/include/uapi/asm-generic/fcntl.h index 7543b3e51331..f63aa749a4fb 100644 --- a/include/uapi/asm-generic/fcntl.h +++ b/include/uapi/asm-generic/fcntl.h @@ -92,6 +92,10 @@ #define O_TMPFILE (__O_TMPFILE | O_DIRECTORY) #define O_TMPFILE_MASK (__O_TMPFILE | O_DIRECTORY | O_CREAT) +#ifndef O_BENEATH +#define O_BENEATH 040000000 /* no / or .. in openat path */ +#endif + #ifndef O_NDELAY #define O_NDELAY O_NONBLOCK #endif -- 2.1.0.rc2.206.gedb03e5 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCHv2 2/3] selftests: Add test of O_BENEATH & openat(2) 2014-11-04 9:54 [PATCHv2 0/3] fs: add O_BENEATH flag to openat(2) David Drysdale 2014-11-04 9:54 ` [PATCHv2 1/3] " David Drysdale @ 2014-11-04 9:54 ` David Drysdale 2014-11-04 18:47 ` Kees Cook 2014-11-11 5:36 ` Dave Chinner 2014-11-04 9:54 ` [PATCHv2 man-pages 3/3] open.2: describe O_BENEATH flag David Drysdale 2 siblings, 2 replies; 8+ messages in thread From: David Drysdale @ 2014-11-04 9:54 UTC (permalink / raw) To: linux-kernel, Alexander Viro, Kees Cook, Eric W. Biederman Cc: Greg Kroah-Hartman, Meredydd Luff, Will Drewry, Jorge Lucangeli Obes, Ricky Zhou, Lee Campbell, Julien Tinnes, Mike Depinet, James Morris, Andy Lutomirski, Paolo Bonzini, Paul Moore, Christoph Hellwig, linux-api, linux-security-module, David Drysdale Add simple tests of openat(2) variations, including examples that check the new O_BENEATH flag. Signed-off-by: David Drysdale <drysdale@google.com> --- tools/testing/selftests/Makefile | 1 + tools/testing/selftests/openat/.gitignore | 4 + tools/testing/selftests/openat/Makefile | 28 +++++ tools/testing/selftests/openat/openat.c | 180 ++++++++++++++++++++++++++++++ 4 files changed, 213 insertions(+) create mode 100644 tools/testing/selftests/openat/.gitignore create mode 100644 tools/testing/selftests/openat/Makefile create mode 100644 tools/testing/selftests/openat/openat.c diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile index 36ff2e4c7b6f..812e973233d2 100644 --- a/tools/testing/selftests/Makefile +++ b/tools/testing/selftests/Makefile @@ -14,6 +14,7 @@ TARGETS += powerpc TARGETS += user TARGETS += sysctl TARGETS += firmware +TARGETS += openat TARGETS_HOTPLUG = cpu-hotplug TARGETS_HOTPLUG += memory-hotplug diff --git a/tools/testing/selftests/openat/.gitignore b/tools/testing/selftests/openat/.gitignore new file mode 100644 index 000000000000..835b2dcd8678 --- /dev/null +++ b/tools/testing/selftests/openat/.gitignore @@ -0,0 +1,4 @@ +openat +subdir +topfile +symlinkdown \ No newline at end of file diff --git a/tools/testing/selftests/openat/Makefile b/tools/testing/selftests/openat/Makefile new file mode 100644 index 000000000000..84cd06e7ee82 --- /dev/null +++ b/tools/testing/selftests/openat/Makefile @@ -0,0 +1,28 @@ +CC = $(CROSS_COMPILE)gcc +CFLAGS = -Wall +BINARIES = openat +DEPS = subdir topfile symlinkdown subdir/bottomfile subdir/symlinkup subdir/symlinkout subdir/symlinkin +all: $(BINARIES) $(DEPS) + +subdir: + mkdir -p subdir +topfile: + echo 0123456789 > $@ +subdir/bottomfile: | subdir + echo 0123456789 > $@ +subdir/symlinkup: | subdir + ln -s ../topfile $@ +subdir/symlinkout: | subdir + ln -s /etc/passwd $@ +subdir/symlinkin: | subdir + ln -s bottomfile $@ +symlinkdown: + ln -s subdir/bottomfile $@ +%: %.c + $(CC) $(CFLAGS) -o $@ $^ + +run_tests: all + ./openat + +clean: + rm -rf $(BINARIES) $(DEPS) diff --git a/tools/testing/selftests/openat/openat.c b/tools/testing/selftests/openat/openat.c new file mode 100644 index 000000000000..0c030cbd10dc --- /dev/null +++ b/tools/testing/selftests/openat/openat.c @@ -0,0 +1,180 @@ +#include <sys/types.h> +#include <sys/stat.h> +#include <sys/syscall.h> +#include <stdio.h> +#include <stdlib.h> +#include <unistd.h> +#include <string.h> +#include <errno.h> + +#include <linux/fcntl.h> + +/* Bypass glibc */ +static int openat_(int dirfd, const char *pathname, int flags) +{ + return syscall(__NR_openat, dirfd, pathname, flags); +} + +static int openat_or_die(int dfd, const char *path, int flags) +{ + int fd = openat_(dfd, path, flags); + + if (fd < 0) { + printf("Failed to openat(%d, '%s'); " + "check prerequisites are available\n", dfd, path); + exit(1); + } + return fd; +} + +static int check_openat(int dfd, const char *path, int flags) +{ + int rc; + int fd; + char buffer[4]; + + errno = 0; + printf("Check success of openat(%d, '%s', %x)... ", + dfd, path?:"(null)", flags); + fd = openat_(dfd, path, flags); + if (fd < 0) { + printf("[FAIL]: openat() failed, rc=%d errno=%d (%s)\n", + fd, errno, strerror(errno)); + return 1; + } + errno = 0; + rc = read(fd, buffer, sizeof(buffer)); + if (rc < 0) { + printf("[FAIL]: read() failed, rc=%d errno=%d (%s)\n", + rc, errno, strerror(errno)); + return 1; + } + close(fd); + printf("[OK]\n"); + return 0; +} + +#define check_openat_fail(dfd, path, flags, errno) \ + _check_openat_fail(dfd, path, flags, errno, #errno) +static int _check_openat_fail(int dfd, const char *path, int flags, + int expected_errno, const char *errno_str) +{ + int rc; + + errno = 0; + printf("Check failure of openat(%d, '%s', %x) with %s... ", + dfd, path?:"(null)", flags, errno_str); + rc = openat_(dfd, path, flags); + if (rc > 0) { + printf("[FAIL] (unexpected success from openat(2))\n"); + close(rc); + return 1; + } + if (errno != expected_errno) { + printf("[FAIL] (expected errno %d (%s) not %d (%s)\n", + expected_errno, strerror(expected_errno), + errno, strerror(errno)); + return 1; + } + printf("[OK]\n"); + return 0; +} + +int check_proc(void) +{ + int proc_dfd = openat_(AT_FDCWD, "/proc/self", O_RDONLY); + int fail = 0; + + if (proc_dfd < 0) { + printf("'/proc/self' unavailable (errno=%d '%s'), skipping\n", + errno, strerror(errno)); + return 1; + } + fail |= check_openat(proc_dfd, "root/etc/passwd", O_RDONLY); +#ifdef O_BENEATH + fail |= check_openat_fail(proc_dfd, "root/etc/passwd", + O_RDONLY|O_BENEATH, EPERM); +#endif + return fail; +} + +int main(int argc, char *argv[]) +{ + int fail = 0; + int dot_dfd = openat_or_die(AT_FDCWD, ".", O_RDONLY); + int subdir_dfd = openat_or_die(AT_FDCWD, "subdir", O_RDONLY); + int file_fd = openat_or_die(AT_FDCWD, "topfile", O_RDONLY); + + /* Sanity check normal behavior */ + fail |= check_openat(AT_FDCWD, "topfile", O_RDONLY); + fail |= check_openat(AT_FDCWD, "subdir/bottomfile", O_RDONLY); + + fail |= check_openat(dot_dfd, "topfile", O_RDONLY); + fail |= check_openat(dot_dfd, "subdir/bottomfile", O_RDONLY); + fail |= check_openat(dot_dfd, "subdir/../topfile", O_RDONLY); + + fail |= check_openat(subdir_dfd, "../topfile", O_RDONLY); + fail |= check_openat(subdir_dfd, "bottomfile", O_RDONLY); + fail |= check_openat(subdir_dfd, "../subdir/bottomfile", O_RDONLY); + fail |= check_openat(subdir_dfd, "symlinkup", O_RDONLY); + fail |= check_openat(subdir_dfd, "symlinkout", O_RDONLY); + + fail |= check_openat(AT_FDCWD, "/etc/passwd", O_RDONLY); + fail |= check_openat(dot_dfd, "/etc/passwd", O_RDONLY); + fail |= check_openat(subdir_dfd, "/etc/passwd", O_RDONLY); + + fail |= check_openat_fail(AT_FDCWD, "bogus", O_RDONLY, ENOENT); + fail |= check_openat_fail(dot_dfd, "bogus", O_RDONLY, ENOENT); + fail |= check_openat_fail(999, "bogus", O_RDONLY, EBADF); + fail |= check_openat_fail(file_fd, "bogus", O_RDONLY, ENOTDIR); + +#ifdef O_BENEATH + /* Test out O_BENEATH */ + fail |= check_openat(AT_FDCWD, "topfile", O_RDONLY|O_BENEATH); + fail |= check_openat(AT_FDCWD, "subdir/bottomfile", + O_RDONLY|O_BENEATH); + + fail |= check_openat(dot_dfd, "topfile", O_RDONLY|O_BENEATH); + fail |= check_openat(dot_dfd, "subdir/bottomfile", + O_RDONLY|O_BENEATH); + fail |= check_openat(subdir_dfd, "bottomfile", O_RDONLY|O_BENEATH); + + /* Symlinks without .. or leading / are OK */ + fail |= check_openat(dot_dfd, "symlinkdown", O_RDONLY|O_BENEATH); + fail |= check_openat(dot_dfd, "subdir/symlinkin", O_RDONLY|O_BENEATH); + fail |= check_openat(subdir_dfd, "symlinkin", O_RDONLY|O_BENEATH); + /* ... unless of course we specify O_NOFOLLOW */ + fail |= check_openat_fail(dot_dfd, "symlinkdown", + O_RDONLY|O_BENEATH|O_NOFOLLOW, ELOOP); + fail |= check_openat_fail(dot_dfd, "subdir/symlinkin", + O_RDONLY|O_BENEATH|O_NOFOLLOW, ELOOP); + fail |= check_openat_fail(subdir_dfd, "symlinkin", + O_RDONLY|O_BENEATH|O_NOFOLLOW, ELOOP); + + /* Can't open paths with ".." in them */ + fail |= check_openat_fail(dot_dfd, "subdir/../topfile", + O_RDONLY|O_BENEATH, EPERM); + fail |= check_openat_fail(subdir_dfd, "../topfile", + O_RDONLY|O_BENEATH, EPERM); + fail |= check_openat_fail(subdir_dfd, "../subdir/bottomfile", + O_RDONLY|O_BENEATH, EPERM); + + /* Can't open paths starting with "/" */ + fail |= check_openat_fail(AT_FDCWD, "/etc/passwd", + O_RDONLY|O_BENEATH, EPERM); + fail |= check_openat_fail(dot_dfd, "/etc/passwd", + O_RDONLY|O_BENEATH, EPERM); + fail |= check_openat_fail(subdir_dfd, "/etc/passwd", + O_RDONLY|O_BENEATH, EPERM); + /* Can't sneak around constraints with symlinks */ + fail |= check_openat_fail(subdir_dfd, "symlinkup", + O_RDONLY|O_BENEATH, EPERM); + fail |= check_openat_fail(subdir_dfd, "symlinkout", + O_RDONLY|O_BENEATH, EPERM); +#else + printf("Skipping O_BENEATH tests due to missing #define\n"); +#endif + fail |= check_proc(); + + return fail ? -1 : 0; +} -- 2.1.0.rc2.206.gedb03e5 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCHv2 2/3] selftests: Add test of O_BENEATH & openat(2) 2014-11-04 9:54 ` [PATCHv2 2/3] selftests: Add test of O_BENEATH & openat(2) David Drysdale @ 2014-11-04 18:47 ` Kees Cook 2014-11-11 5:36 ` Dave Chinner 1 sibling, 0 replies; 8+ messages in thread From: Kees Cook @ 2014-11-04 18:47 UTC (permalink / raw) To: David Drysdale Cc: LKML, Alexander Viro, Eric W. Biederman, Greg Kroah-Hartman, Meredydd Luff, Will Drewry, Jorge Lucangeli Obes, Ricky Zhou, Lee Campbell, Julien Tinnes, Mike Depinet, James Morris, Andy Lutomirski, Paolo Bonzini, Paul Moore, Christoph Hellwig, Linux API, linux-security-module On Tue, Nov 4, 2014 at 1:54 AM, David Drysdale <drysdale@google.com> wrote: > Add simple tests of openat(2) variations, including examples that > check the new O_BENEATH flag. > > Signed-off-by: David Drysdale <drysdale@google.com> Excellent! Thank you for including self tests. :) Acked-by: Kees Cook <keescook@chromium.org> -Kees > --- > tools/testing/selftests/Makefile | 1 + > tools/testing/selftests/openat/.gitignore | 4 + > tools/testing/selftests/openat/Makefile | 28 +++++ > tools/testing/selftests/openat/openat.c | 180 ++++++++++++++++++++++++++++++ > 4 files changed, 213 insertions(+) > create mode 100644 tools/testing/selftests/openat/.gitignore > create mode 100644 tools/testing/selftests/openat/Makefile > create mode 100644 tools/testing/selftests/openat/openat.c > > diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile > index 36ff2e4c7b6f..812e973233d2 100644 > --- a/tools/testing/selftests/Makefile > +++ b/tools/testing/selftests/Makefile > @@ -14,6 +14,7 @@ TARGETS += powerpc > TARGETS += user > TARGETS += sysctl > TARGETS += firmware > +TARGETS += openat > > TARGETS_HOTPLUG = cpu-hotplug > TARGETS_HOTPLUG += memory-hotplug > diff --git a/tools/testing/selftests/openat/.gitignore b/tools/testing/selftests/openat/.gitignore > new file mode 100644 > index 000000000000..835b2dcd8678 > --- /dev/null > +++ b/tools/testing/selftests/openat/.gitignore > @@ -0,0 +1,4 @@ > +openat > +subdir > +topfile > +symlinkdown > \ No newline at end of file > diff --git a/tools/testing/selftests/openat/Makefile b/tools/testing/selftests/openat/Makefile > new file mode 100644 > index 000000000000..84cd06e7ee82 > --- /dev/null > +++ b/tools/testing/selftests/openat/Makefile > @@ -0,0 +1,28 @@ > +CC = $(CROSS_COMPILE)gcc > +CFLAGS = -Wall > +BINARIES = openat > +DEPS = subdir topfile symlinkdown subdir/bottomfile subdir/symlinkup subdir/symlinkout subdir/symlinkin > +all: $(BINARIES) $(DEPS) > + > +subdir: > + mkdir -p subdir > +topfile: > + echo 0123456789 > $@ > +subdir/bottomfile: | subdir > + echo 0123456789 > $@ > +subdir/symlinkup: | subdir > + ln -s ../topfile $@ > +subdir/symlinkout: | subdir > + ln -s /etc/passwd $@ > +subdir/symlinkin: | subdir > + ln -s bottomfile $@ > +symlinkdown: > + ln -s subdir/bottomfile $@ > +%: %.c > + $(CC) $(CFLAGS) -o $@ $^ > + > +run_tests: all > + ./openat > + > +clean: > + rm -rf $(BINARIES) $(DEPS) > diff --git a/tools/testing/selftests/openat/openat.c b/tools/testing/selftests/openat/openat.c > new file mode 100644 > index 000000000000..0c030cbd10dc > --- /dev/null > +++ b/tools/testing/selftests/openat/openat.c > @@ -0,0 +1,180 @@ > +#include <sys/types.h> > +#include <sys/stat.h> > +#include <sys/syscall.h> > +#include <stdio.h> > +#include <stdlib.h> > +#include <unistd.h> > +#include <string.h> > +#include <errno.h> > + > +#include <linux/fcntl.h> > + > +/* Bypass glibc */ > +static int openat_(int dirfd, const char *pathname, int flags) > +{ > + return syscall(__NR_openat, dirfd, pathname, flags); > +} > + > +static int openat_or_die(int dfd, const char *path, int flags) > +{ > + int fd = openat_(dfd, path, flags); > + > + if (fd < 0) { > + printf("Failed to openat(%d, '%s'); " > + "check prerequisites are available\n", dfd, path); > + exit(1); > + } > + return fd; > +} > + > +static int check_openat(int dfd, const char *path, int flags) > +{ > + int rc; > + int fd; > + char buffer[4]; > + > + errno = 0; > + printf("Check success of openat(%d, '%s', %x)... ", > + dfd, path?:"(null)", flags); > + fd = openat_(dfd, path, flags); > + if (fd < 0) { > + printf("[FAIL]: openat() failed, rc=%d errno=%d (%s)\n", > + fd, errno, strerror(errno)); > + return 1; > + } > + errno = 0; > + rc = read(fd, buffer, sizeof(buffer)); > + if (rc < 0) { > + printf("[FAIL]: read() failed, rc=%d errno=%d (%s)\n", > + rc, errno, strerror(errno)); > + return 1; > + } > + close(fd); > + printf("[OK]\n"); > + return 0; > +} > + > +#define check_openat_fail(dfd, path, flags, errno) \ > + _check_openat_fail(dfd, path, flags, errno, #errno) > +static int _check_openat_fail(int dfd, const char *path, int flags, > + int expected_errno, const char *errno_str) > +{ > + int rc; > + > + errno = 0; > + printf("Check failure of openat(%d, '%s', %x) with %s... ", > + dfd, path?:"(null)", flags, errno_str); > + rc = openat_(dfd, path, flags); > + if (rc > 0) { > + printf("[FAIL] (unexpected success from openat(2))\n"); > + close(rc); > + return 1; > + } > + if (errno != expected_errno) { > + printf("[FAIL] (expected errno %d (%s) not %d (%s)\n", > + expected_errno, strerror(expected_errno), > + errno, strerror(errno)); > + return 1; > + } > + printf("[OK]\n"); > + return 0; > +} > + > +int check_proc(void) > +{ > + int proc_dfd = openat_(AT_FDCWD, "/proc/self", O_RDONLY); > + int fail = 0; > + > + if (proc_dfd < 0) { > + printf("'/proc/self' unavailable (errno=%d '%s'), skipping\n", > + errno, strerror(errno)); > + return 1; > + } > + fail |= check_openat(proc_dfd, "root/etc/passwd", O_RDONLY); > +#ifdef O_BENEATH > + fail |= check_openat_fail(proc_dfd, "root/etc/passwd", > + O_RDONLY|O_BENEATH, EPERM); > +#endif > + return fail; > +} > + > +int main(int argc, char *argv[]) > +{ > + int fail = 0; > + int dot_dfd = openat_or_die(AT_FDCWD, ".", O_RDONLY); > + int subdir_dfd = openat_or_die(AT_FDCWD, "subdir", O_RDONLY); > + int file_fd = openat_or_die(AT_FDCWD, "topfile", O_RDONLY); > + > + /* Sanity check normal behavior */ > + fail |= check_openat(AT_FDCWD, "topfile", O_RDONLY); > + fail |= check_openat(AT_FDCWD, "subdir/bottomfile", O_RDONLY); > + > + fail |= check_openat(dot_dfd, "topfile", O_RDONLY); > + fail |= check_openat(dot_dfd, "subdir/bottomfile", O_RDONLY); > + fail |= check_openat(dot_dfd, "subdir/../topfile", O_RDONLY); > + > + fail |= check_openat(subdir_dfd, "../topfile", O_RDONLY); > + fail |= check_openat(subdir_dfd, "bottomfile", O_RDONLY); > + fail |= check_openat(subdir_dfd, "../subdir/bottomfile", O_RDONLY); > + fail |= check_openat(subdir_dfd, "symlinkup", O_RDONLY); > + fail |= check_openat(subdir_dfd, "symlinkout", O_RDONLY); > + > + fail |= check_openat(AT_FDCWD, "/etc/passwd", O_RDONLY); > + fail |= check_openat(dot_dfd, "/etc/passwd", O_RDONLY); > + fail |= check_openat(subdir_dfd, "/etc/passwd", O_RDONLY); > + > + fail |= check_openat_fail(AT_FDCWD, "bogus", O_RDONLY, ENOENT); > + fail |= check_openat_fail(dot_dfd, "bogus", O_RDONLY, ENOENT); > + fail |= check_openat_fail(999, "bogus", O_RDONLY, EBADF); > + fail |= check_openat_fail(file_fd, "bogus", O_RDONLY, ENOTDIR); > + > +#ifdef O_BENEATH > + /* Test out O_BENEATH */ > + fail |= check_openat(AT_FDCWD, "topfile", O_RDONLY|O_BENEATH); > + fail |= check_openat(AT_FDCWD, "subdir/bottomfile", > + O_RDONLY|O_BENEATH); > + > + fail |= check_openat(dot_dfd, "topfile", O_RDONLY|O_BENEATH); > + fail |= check_openat(dot_dfd, "subdir/bottomfile", > + O_RDONLY|O_BENEATH); > + fail |= check_openat(subdir_dfd, "bottomfile", O_RDONLY|O_BENEATH); > + > + /* Symlinks without .. or leading / are OK */ > + fail |= check_openat(dot_dfd, "symlinkdown", O_RDONLY|O_BENEATH); > + fail |= check_openat(dot_dfd, "subdir/symlinkin", O_RDONLY|O_BENEATH); > + fail |= check_openat(subdir_dfd, "symlinkin", O_RDONLY|O_BENEATH); > + /* ... unless of course we specify O_NOFOLLOW */ > + fail |= check_openat_fail(dot_dfd, "symlinkdown", > + O_RDONLY|O_BENEATH|O_NOFOLLOW, ELOOP); > + fail |= check_openat_fail(dot_dfd, "subdir/symlinkin", > + O_RDONLY|O_BENEATH|O_NOFOLLOW, ELOOP); > + fail |= check_openat_fail(subdir_dfd, "symlinkin", > + O_RDONLY|O_BENEATH|O_NOFOLLOW, ELOOP); > + > + /* Can't open paths with ".." in them */ > + fail |= check_openat_fail(dot_dfd, "subdir/../topfile", > + O_RDONLY|O_BENEATH, EPERM); > + fail |= check_openat_fail(subdir_dfd, "../topfile", > + O_RDONLY|O_BENEATH, EPERM); > + fail |= check_openat_fail(subdir_dfd, "../subdir/bottomfile", > + O_RDONLY|O_BENEATH, EPERM); > + > + /* Can't open paths starting with "/" */ > + fail |= check_openat_fail(AT_FDCWD, "/etc/passwd", > + O_RDONLY|O_BENEATH, EPERM); > + fail |= check_openat_fail(dot_dfd, "/etc/passwd", > + O_RDONLY|O_BENEATH, EPERM); > + fail |= check_openat_fail(subdir_dfd, "/etc/passwd", > + O_RDONLY|O_BENEATH, EPERM); > + /* Can't sneak around constraints with symlinks */ > + fail |= check_openat_fail(subdir_dfd, "symlinkup", > + O_RDONLY|O_BENEATH, EPERM); > + fail |= check_openat_fail(subdir_dfd, "symlinkout", > + O_RDONLY|O_BENEATH, EPERM); > +#else > + printf("Skipping O_BENEATH tests due to missing #define\n"); > +#endif > + fail |= check_proc(); > + > + return fail ? -1 : 0; > +} > -- > 2.1.0.rc2.206.gedb03e5 > -- Kees Cook Chrome OS Security ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCHv2 2/3] selftests: Add test of O_BENEATH & openat(2) 2014-11-04 9:54 ` [PATCHv2 2/3] selftests: Add test of O_BENEATH & openat(2) David Drysdale 2014-11-04 18:47 ` Kees Cook @ 2014-11-11 5:36 ` Dave Chinner 2014-11-21 14:19 ` David Drysdale 1 sibling, 1 reply; 8+ messages in thread From: Dave Chinner @ 2014-11-11 5:36 UTC (permalink / raw) To: David Drysdale Cc: linux-kernel, Alexander Viro, Kees Cook, Eric W. Biederman, Greg Kroah-Hartman, Meredydd Luff, Will Drewry, Jorge Lucangeli Obes, Ricky Zhou, Lee Campbell, Julien Tinnes, Mike Depinet, James Morris, Andy Lutomirski, Paolo Bonzini, Paul Moore, Christoph Hellwig, linux-api, linux-security-module, fstests [cc fstests@vger.kernel.org] On Tue, Nov 04, 2014 at 09:54:43AM +0000, David Drysdale wrote: > Add simple tests of openat(2) variations, including examples that > check the new O_BENEATH flag. > > Signed-off-by: David Drysdale <drysdale@google.com> Wouldn't this be better added to fstests? That's the regression test suite used by filesystem developers and most distro QA organisations and where the fs developers aggregate all their new regression tests. IMO, the fewer places we aggregate VFS/filesystem tests the better. I really don't think the kernel tree is the best place for adding VFS behavioural tests because it has none of the infrastructure around it to test arbitrary filesystems and configurations and hence is not particularly useful to the people whoa re likely to notice and care about fs regression tests suddenly breaking. As an example, the recent renameat() syscall additions (e.g. RENAME_EXCHANGE, RENAME_NOREPLACE) have unit tests in fstests, so these new O_BENEATH tests should really follow the same model... Cheers, Dave. > --- > tools/testing/selftests/Makefile | 1 + > tools/testing/selftests/openat/.gitignore | 4 + > tools/testing/selftests/openat/Makefile | 28 +++++ > tools/testing/selftests/openat/openat.c | 180 ++++++++++++++++++++++++++++++ > 4 files changed, 213 insertions(+) > create mode 100644 tools/testing/selftests/openat/.gitignore > create mode 100644 tools/testing/selftests/openat/Makefile > create mode 100644 tools/testing/selftests/openat/openat.c > > diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile > index 36ff2e4c7b6f..812e973233d2 100644 > --- a/tools/testing/selftests/Makefile > +++ b/tools/testing/selftests/Makefile > @@ -14,6 +14,7 @@ TARGETS += powerpc > TARGETS += user > TARGETS += sysctl > TARGETS += firmware > +TARGETS += openat > > TARGETS_HOTPLUG = cpu-hotplug > TARGETS_HOTPLUG += memory-hotplug > diff --git a/tools/testing/selftests/openat/.gitignore b/tools/testing/selftests/openat/.gitignore > new file mode 100644 > index 000000000000..835b2dcd8678 > --- /dev/null > +++ b/tools/testing/selftests/openat/.gitignore > @@ -0,0 +1,4 @@ > +openat > +subdir > +topfile > +symlinkdown > \ No newline at end of file > diff --git a/tools/testing/selftests/openat/Makefile b/tools/testing/selftests/openat/Makefile > new file mode 100644 > index 000000000000..84cd06e7ee82 > --- /dev/null > +++ b/tools/testing/selftests/openat/Makefile > @@ -0,0 +1,28 @@ > +CC = $(CROSS_COMPILE)gcc > +CFLAGS = -Wall > +BINARIES = openat > +DEPS = subdir topfile symlinkdown subdir/bottomfile subdir/symlinkup subdir/symlinkout subdir/symlinkin > +all: $(BINARIES) $(DEPS) > + > +subdir: > + mkdir -p subdir > +topfile: > + echo 0123456789 > $@ > +subdir/bottomfile: | subdir > + echo 0123456789 > $@ > +subdir/symlinkup: | subdir > + ln -s ../topfile $@ > +subdir/symlinkout: | subdir > + ln -s /etc/passwd $@ > +subdir/symlinkin: | subdir > + ln -s bottomfile $@ > +symlinkdown: > + ln -s subdir/bottomfile $@ > +%: %.c > + $(CC) $(CFLAGS) -o $@ $^ > + > +run_tests: all > + ./openat > + > +clean: > + rm -rf $(BINARIES) $(DEPS) > diff --git a/tools/testing/selftests/openat/openat.c b/tools/testing/selftests/openat/openat.c > new file mode 100644 > index 000000000000..0c030cbd10dc > --- /dev/null > +++ b/tools/testing/selftests/openat/openat.c > @@ -0,0 +1,180 @@ > +#include <sys/types.h> > +#include <sys/stat.h> > +#include <sys/syscall.h> > +#include <stdio.h> > +#include <stdlib.h> > +#include <unistd.h> > +#include <string.h> > +#include <errno.h> > + > +#include <linux/fcntl.h> > + > +/* Bypass glibc */ > +static int openat_(int dirfd, const char *pathname, int flags) > +{ > + return syscall(__NR_openat, dirfd, pathname, flags); > +} > + > +static int openat_or_die(int dfd, const char *path, int flags) > +{ > + int fd = openat_(dfd, path, flags); > + > + if (fd < 0) { > + printf("Failed to openat(%d, '%s'); " > + "check prerequisites are available\n", dfd, path); > + exit(1); > + } > + return fd; > +} > + > +static int check_openat(int dfd, const char *path, int flags) > +{ > + int rc; > + int fd; > + char buffer[4]; > + > + errno = 0; > + printf("Check success of openat(%d, '%s', %x)... ", > + dfd, path?:"(null)", flags); > + fd = openat_(dfd, path, flags); > + if (fd < 0) { > + printf("[FAIL]: openat() failed, rc=%d errno=%d (%s)\n", > + fd, errno, strerror(errno)); > + return 1; > + } > + errno = 0; > + rc = read(fd, buffer, sizeof(buffer)); > + if (rc < 0) { > + printf("[FAIL]: read() failed, rc=%d errno=%d (%s)\n", > + rc, errno, strerror(errno)); > + return 1; > + } > + close(fd); > + printf("[OK]\n"); > + return 0; > +} > + > +#define check_openat_fail(dfd, path, flags, errno) \ > + _check_openat_fail(dfd, path, flags, errno, #errno) > +static int _check_openat_fail(int dfd, const char *path, int flags, > + int expected_errno, const char *errno_str) > +{ > + int rc; > + > + errno = 0; > + printf("Check failure of openat(%d, '%s', %x) with %s... ", > + dfd, path?:"(null)", flags, errno_str); > + rc = openat_(dfd, path, flags); > + if (rc > 0) { > + printf("[FAIL] (unexpected success from openat(2))\n"); > + close(rc); > + return 1; > + } > + if (errno != expected_errno) { > + printf("[FAIL] (expected errno %d (%s) not %d (%s)\n", > + expected_errno, strerror(expected_errno), > + errno, strerror(errno)); > + return 1; > + } > + printf("[OK]\n"); > + return 0; > +} > + > +int check_proc(void) > +{ > + int proc_dfd = openat_(AT_FDCWD, "/proc/self", O_RDONLY); > + int fail = 0; > + > + if (proc_dfd < 0) { > + printf("'/proc/self' unavailable (errno=%d '%s'), skipping\n", > + errno, strerror(errno)); > + return 1; > + } > + fail |= check_openat(proc_dfd, "root/etc/passwd", O_RDONLY); > +#ifdef O_BENEATH > + fail |= check_openat_fail(proc_dfd, "root/etc/passwd", > + O_RDONLY|O_BENEATH, EPERM); > +#endif > + return fail; > +} > + > +int main(int argc, char *argv[]) > +{ > + int fail = 0; > + int dot_dfd = openat_or_die(AT_FDCWD, ".", O_RDONLY); > + int subdir_dfd = openat_or_die(AT_FDCWD, "subdir", O_RDONLY); > + int file_fd = openat_or_die(AT_FDCWD, "topfile", O_RDONLY); > + > + /* Sanity check normal behavior */ > + fail |= check_openat(AT_FDCWD, "topfile", O_RDONLY); > + fail |= check_openat(AT_FDCWD, "subdir/bottomfile", O_RDONLY); > + > + fail |= check_openat(dot_dfd, "topfile", O_RDONLY); > + fail |= check_openat(dot_dfd, "subdir/bottomfile", O_RDONLY); > + fail |= check_openat(dot_dfd, "subdir/../topfile", O_RDONLY); > + > + fail |= check_openat(subdir_dfd, "../topfile", O_RDONLY); > + fail |= check_openat(subdir_dfd, "bottomfile", O_RDONLY); > + fail |= check_openat(subdir_dfd, "../subdir/bottomfile", O_RDONLY); > + fail |= check_openat(subdir_dfd, "symlinkup", O_RDONLY); > + fail |= check_openat(subdir_dfd, "symlinkout", O_RDONLY); > + > + fail |= check_openat(AT_FDCWD, "/etc/passwd", O_RDONLY); > + fail |= check_openat(dot_dfd, "/etc/passwd", O_RDONLY); > + fail |= check_openat(subdir_dfd, "/etc/passwd", O_RDONLY); > + > + fail |= check_openat_fail(AT_FDCWD, "bogus", O_RDONLY, ENOENT); > + fail |= check_openat_fail(dot_dfd, "bogus", O_RDONLY, ENOENT); > + fail |= check_openat_fail(999, "bogus", O_RDONLY, EBADF); > + fail |= check_openat_fail(file_fd, "bogus", O_RDONLY, ENOTDIR); > + > +#ifdef O_BENEATH > + /* Test out O_BENEATH */ > + fail |= check_openat(AT_FDCWD, "topfile", O_RDONLY|O_BENEATH); > + fail |= check_openat(AT_FDCWD, "subdir/bottomfile", > + O_RDONLY|O_BENEATH); > + > + fail |= check_openat(dot_dfd, "topfile", O_RDONLY|O_BENEATH); > + fail |= check_openat(dot_dfd, "subdir/bottomfile", > + O_RDONLY|O_BENEATH); > + fail |= check_openat(subdir_dfd, "bottomfile", O_RDONLY|O_BENEATH); > + > + /* Symlinks without .. or leading / are OK */ > + fail |= check_openat(dot_dfd, "symlinkdown", O_RDONLY|O_BENEATH); > + fail |= check_openat(dot_dfd, "subdir/symlinkin", O_RDONLY|O_BENEATH); > + fail |= check_openat(subdir_dfd, "symlinkin", O_RDONLY|O_BENEATH); > + /* ... unless of course we specify O_NOFOLLOW */ > + fail |= check_openat_fail(dot_dfd, "symlinkdown", > + O_RDONLY|O_BENEATH|O_NOFOLLOW, ELOOP); > + fail |= check_openat_fail(dot_dfd, "subdir/symlinkin", > + O_RDONLY|O_BENEATH|O_NOFOLLOW, ELOOP); > + fail |= check_openat_fail(subdir_dfd, "symlinkin", > + O_RDONLY|O_BENEATH|O_NOFOLLOW, ELOOP); > + > + /* Can't open paths with ".." in them */ > + fail |= check_openat_fail(dot_dfd, "subdir/../topfile", > + O_RDONLY|O_BENEATH, EPERM); > + fail |= check_openat_fail(subdir_dfd, "../topfile", > + O_RDONLY|O_BENEATH, EPERM); > + fail |= check_openat_fail(subdir_dfd, "../subdir/bottomfile", > + O_RDONLY|O_BENEATH, EPERM); > + > + /* Can't open paths starting with "/" */ > + fail |= check_openat_fail(AT_FDCWD, "/etc/passwd", > + O_RDONLY|O_BENEATH, EPERM); > + fail |= check_openat_fail(dot_dfd, "/etc/passwd", > + O_RDONLY|O_BENEATH, EPERM); > + fail |= check_openat_fail(subdir_dfd, "/etc/passwd", > + O_RDONLY|O_BENEATH, EPERM); > + /* Can't sneak around constraints with symlinks */ > + fail |= check_openat_fail(subdir_dfd, "symlinkup", > + O_RDONLY|O_BENEATH, EPERM); > + fail |= check_openat_fail(subdir_dfd, "symlinkout", > + O_RDONLY|O_BENEATH, EPERM); > +#else > + printf("Skipping O_BENEATH tests due to missing #define\n"); > +#endif > + fail |= check_proc(); > + > + return fail ? -1 : 0; > +} > -- > 2.1.0.rc2.206.gedb03e5 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ > -- Dave Chinner david@fromorbit.com ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCHv2 2/3] selftests: Add test of O_BENEATH & openat(2) 2014-11-11 5:36 ` Dave Chinner @ 2014-11-21 14:19 ` David Drysdale 2014-12-12 0:05 ` Dave Chinner 0 siblings, 1 reply; 8+ messages in thread From: David Drysdale @ 2014-11-21 14:19 UTC (permalink / raw) To: Dave Chinner Cc: linux-kernel, Alexander Viro, Kees Cook, Eric W. Biederman, Greg Kroah-Hartman, Meredydd Luff, Will Drewry, Jorge Lucangeli Obes, Ricky Zhou, Lee Campbell, Julien Tinnes, Mike Depinet, James Morris, Andy Lutomirski, Paolo Bonzini, Paul Moore, Christoph Hellwig, Linux API, LSM List, fstests On Tue, Nov 11, 2014 at 5:36 AM, Dave Chinner <david@fromorbit.com> wrote: > [cc fstests@vger.kernel.org] > > On Tue, Nov 04, 2014 at 09:54:43AM +0000, David Drysdale wrote: >> Add simple tests of openat(2) variations, including examples that >> check the new O_BENEATH flag. >> >> Signed-off-by: David Drysdale <drysdale@google.com> > > Wouldn't this be better added to fstests? That's the regression > test suite used by filesystem developers and most distro QA > organisations and where the fs developers aggregate all their new > regression tests. > > IMO, the fewer places we aggregate VFS/filesystem tests the better. > I really don't think the kernel tree is the best place for adding > VFS behavioural tests because it has none of the infrastructure > around it to test arbitrary filesystems and configurations and hence > is not particularly useful to the people whoa re likely to notice > and care about fs regression tests suddenly breaking. > > As an example, the recent renameat() syscall additions (e.g. > RENAME_EXCHANGE, RENAME_NOREPLACE) have unit tests in fstests, so > these new O_BENEATH tests should really follow the same model... > > Cheers, > > Dave. Fair enough, that makes sense -- I've now got a version of the selftest running within xfstests (git://oss.sgi.com/xfs/cmds/xfstests.git is the master repo, right?). Given that xfstests is independent of the kernel, what's the expected way to deal with flags (or syscalls) that are only in specific kernel versions? At the moment I've just got a primitive override at compile time (#ifndef O_BENEATH #define O_BENEATH ...), and then the test will fail at run-time against an older kernel -- is there a need for anything more sophisticated? (And if so, are there any examples I can crib from?) Also, is there an archive of the fstests@ mailing list somewhere? Thanks, David ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCHv2 2/3] selftests: Add test of O_BENEATH & openat(2) 2014-11-21 14:19 ` David Drysdale @ 2014-12-12 0:05 ` Dave Chinner 0 siblings, 0 replies; 8+ messages in thread From: Dave Chinner @ 2014-12-12 0:05 UTC (permalink / raw) To: David Drysdale Cc: linux-kernel, Alexander Viro, Kees Cook, Eric W. Biederman, Greg Kroah-Hartman, Meredydd Luff, Will Drewry, Jorge Lucangeli Obes, Ricky Zhou, Lee Campbell, Julien Tinnes, Mike Depinet, James Morris, Andy Lutomirski, Paolo Bonzini, Paul Moore, Christoph Hellwig, Linux API, LSM List, fstests On Fri, Nov 21, 2014 at 02:19:41PM +0000, David Drysdale wrote: > On Tue, Nov 11, 2014 at 5:36 AM, Dave Chinner <david@fromorbit.com> wrote: > > [cc fstests@vger.kernel.org] > > > > On Tue, Nov 04, 2014 at 09:54:43AM +0000, David Drysdale wrote: > >> Add simple tests of openat(2) variations, including examples that > >> check the new O_BENEATH flag. > >> > >> Signed-off-by: David Drysdale <drysdale@google.com> > > > > Wouldn't this be better added to fstests? That's the regression > > test suite used by filesystem developers and most distro QA > > organisations and where the fs developers aggregate all their new > > regression tests. > > > > IMO, the fewer places we aggregate VFS/filesystem tests the better. > > I really don't think the kernel tree is the best place for adding > > VFS behavioural tests because it has none of the infrastructure > > around it to test arbitrary filesystems and configurations and hence > > is not particularly useful to the people whoa re likely to notice > > and care about fs regression tests suddenly breaking. > > > > As an example, the recent renameat() syscall additions (e.g. > > RENAME_EXCHANGE, RENAME_NOREPLACE) have unit tests in fstests, so > > these new O_BENEATH tests should really follow the same model... > > Fair enough, that makes sense -- I've now got a version of the selftest > running within xfstests (git://oss.sgi.com/xfs/cmds/xfstests.git is the > master repo, right?). Or git://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git, which is where I typically update first and push dev branches to. > Given that xfstests is independent of the kernel, what's the expected > way to deal with flags (or syscalls) that are only in specific kernel > versions? At the moment I've just got a primitive override at > compile time (#ifndef O_BENEATH #define O_BENEATH ...), and > then the test will fail at run-time against an older kernel -- is there a > need for anything more sophisticated? (And if so, are there any > examples I can crib from?) See the code in the src/renameat2.c for an example of how syscalls and their flags are added prior to their being kernel and userspace header support. Also, note the "-t" CLI option for the "test" option in that little program. This is used by the _require_renameat2 function that the tests that make use of this functionality call to determine if th etest shoul dbe run on this kernel or not. > Also, is there an archive of the fstests@ mailing list somewhere? I wish. I've tried to get it archived on all the major sites like marc, spinics, etc repeatedly since the list was created and not a single one of them have responded to any of my requests, let alone started to archive the list.... Cheers, Dave. -- Dave Chinner david@fromorbit.com ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCHv2 man-pages 3/3] open.2: describe O_BENEATH flag 2014-11-04 9:54 [PATCHv2 0/3] fs: add O_BENEATH flag to openat(2) David Drysdale 2014-11-04 9:54 ` [PATCHv2 1/3] " David Drysdale 2014-11-04 9:54 ` [PATCHv2 2/3] selftests: Add test of O_BENEATH & openat(2) David Drysdale @ 2014-11-04 9:54 ` David Drysdale 2 siblings, 0 replies; 8+ messages in thread From: David Drysdale @ 2014-11-04 9:54 UTC (permalink / raw) To: linux-kernel, Alexander Viro, Kees Cook, Eric W. Biederman Cc: Greg Kroah-Hartman, Meredydd Luff, Will Drewry, Jorge Lucangeli Obes, Ricky Zhou, Lee Campbell, Julien Tinnes, Mike Depinet, James Morris, Andy Lutomirski, Paolo Bonzini, Paul Moore, Christoph Hellwig, linux-api, linux-security-module, David Drysdale Signed-off-by: David Drysdale <drysdale@google.com> --- man2/open.2 | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/man2/open.2 b/man2/open.2 index abc3c35b8b3a..f06edc4aa843 100644 --- a/man2/open.2 +++ b/man2/open.2 @@ -716,6 +716,31 @@ XFS support was added .\" commit ab29743117f9f4c22ac44c13c1647fb24fb2bafe in Linux 3.15. .TP +.B O_BENEATH " (since Linux 3.??)" +Ensure that the +.I pathname +is beneath the current working directory (for +.BR open (2)) +or the +.I dirfd +(for +.BR openat (2)). +If the +.I pathname +is absolute or contains a path component of "..", the +.BR open () +fails with the error +.BR EPERM. +This occurs even if ".." path component would not actually +escape the original directory; for example, a +.I pathname +of "subdir/../filename" would be rejected. +Path components that are symbolic links to absolute paths, or that are +relative paths containing a ".." component, will also cause the +.BR open () +operation to fail with the error +.BR EPERM. +.TP .B O_TRUNC If the file already exists and is a regular file and the access mode allows writing (i.e., is @@ -976,6 +1001,13 @@ flag was specified, but the effective user ID of the caller did not match the owner of the file and the caller was not privileged .RB ( CAP_FOWNER ). .TP +.B EPERM +The +.B O_BENEATH +flag was specified and the +.I pathname +was not beneath the relevant directory. +.TP .B EROFS .I pathname refers to a file on a read-only filesystem and write access was -- 2.1.0.rc2.206.gedb03e5 ^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2014-12-12 0:10 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-11-04 9:54 [PATCHv2 0/3] fs: add O_BENEATH flag to openat(2) David Drysdale 2014-11-04 9:54 ` [PATCHv2 1/3] " David Drysdale 2014-11-04 9:54 ` [PATCHv2 2/3] selftests: Add test of O_BENEATH & openat(2) David Drysdale 2014-11-04 18:47 ` Kees Cook 2014-11-11 5:36 ` Dave Chinner 2014-11-21 14:19 ` David Drysdale 2014-12-12 0:05 ` Dave Chinner 2014-11-04 9:54 ` [PATCHv2 man-pages 3/3] open.2: describe O_BENEATH flag David Drysdale
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).