[PATCHv3 0/3] fs: add O_BENEATH flag to openat(2)

[PATCHv3 0/3] fs: add O_BENEATH flag to openat(2)

[David Drysdale]

This change adds a new O_BENEATH flag for openat(2) which restricts the
provided path, rejecting (with -EPERM) paths that are not beneath
the provided dfd.

This change was originally 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 v2:
 - Move tests into xfstests [Dave Chinner, with thanks for feedback
   on initial version]
 - Merge up to v4.0-rc3 & latest man-pages

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 (1):
  fs: add O_BENEATH flag to 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                           |  4 ++--
 fs/namei.c                           | 21 ++++++++++++++++++---
 fs/open.c                            |  4 +++-
 fs/proc/base.c                       |  4 +++-
 fs/proc/namespaces.c                 |  8 ++++++--
 include/linux/namei.h                |  3 ++-
 include/uapi/asm-generic/fcntl.h     |  4 ++++
 10 files changed, 41 insertions(+), 10 deletions(-)

--
2.2.0.rc0.207.ga3a616c

[PATCHv3 1/3] fs: add O_BENEATH flag to openat(2)

[David Drysdale]

Add a new O_BENEATH flag for openat(2) which restricts the
provided path, rejecting (with -EPERM) 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                           |  4 ++--
 fs/namei.c                           | 21 ++++++++++++++++++---
 fs/open.c                            |  4 +++-
 fs/proc/base.c                       |  4 +++-
 fs/proc/namespaces.c                 |  8 ++++++--
 include/linux/namei.h                |  3 ++-
 include/uapi/asm-generic/fcntl.h     |  4 ++++
 10 files changed, 41 insertions(+), 10 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 ee85cd4e136a..3169693e9390 100644
--- a/fs/fcntl.c
+++ b/fs/fcntl.c
@@ -740,7 +740,7 @@ 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(21 - 1 /* for O_RDONLY being 0 */ != HWEIGHT32(
+	BUILD_BUG_ON(22 - 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	| */
@@ -748,7 +748,7 @@ static int __init fcntl_init(void)
 		O_DIRECT	| O_LARGEFILE	| O_DIRECTORY	|
 		O_NOFOLLOW	| O_NOATIME	| O_CLOEXEC	|
 		__FMODE_EXEC	| O_PATH	| __O_TMPFILE	|
-		__FMODE_NONOTIFY
+		__FMODE_NONOTIFY| O_BENEATH
 		));

 	fasync_cache = kmem_cache_create("fasync_cache",
diff --git a/fs/namei.c b/fs/namei.c
index c83145af4bfc..67e9a8e27329 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -705,13 +705,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;
 }

 void nd_set_link(struct nameidata *nd, char *path)
@@ -1775,9 +1778,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;

@@ -1796,6 +1804,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;
 				}
@@ -1847,6 +1859,7 @@ static int link_path_walk(const char *name, struct nameidata *nd)
 			break;
 		}
 	}
+exit:
 	terminate_walk(nd);
 	return err;
 }
@@ -1886,6 +1899,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 33f9cbf2610b..16e06610ddbb 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -902,7 +902,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);
@@ -933,6 +933,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 3f3d7aeb0712..eb8a9926ebb1 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -1385,7 +1385,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 c9eac4563fa8..8351534dc43b 100644
--- a/fs/proc/namespaces.c
+++ b/fs/proc/namespaces.c
@@ -36,6 +36,7 @@ static void *proc_ns_follow_link(struct dentry *dentry, struct nameidata *nd)
 	const struct proc_ns_operations *ns_ops = PROC_I(inode)->ns_ops;
 	struct task_struct *task;
 	struct path ns_path;
+	int err;
 	void *error = ERR_PTR(-EACCES);

 	task = get_proc_task(inode);
@@ -44,8 +45,11 @@ static void *proc_ns_follow_link(struct dentry *dentry, struct nameidata *nd)

 	if (ptrace_may_access(task, PTRACE_MODE_READ)) {
 		error = ns_get_path(&ns_path, task, ns_ops);
-		if (!error)
-			nd_jump_link(nd, &ns_path);
+		if (!error) {
+			err = nd_jump_link(nd, &ns_path);
+			if (err)
+				error = ERR_PTR(err);
+		}
 	}
 	put_task_struct(task);
 	return error;
diff --git a/include/linux/namei.h b/include/linux/namei.h
index c8990779f0c3..bd656840a271 100644
--- a/include/linux/namei.h
+++ b/include/linux/namei.h
@@ -28,6 +28,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
@@ -70,7 +71,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);
 extern void nd_set_link(struct nameidata *nd, char *path);
 extern char *nd_get_link(struct nameidata *nd);

diff --git a/include/uapi/asm-generic/fcntl.h b/include/uapi/asm-generic/fcntl.h
index e063effe0cc1..4542bc6a2950 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.2.0.rc0.207.ga3a616c

[PATCHv3 xfstests 2/3] generic: test openat and new O_BENEATH flag

[David Drysdale]

Test basic openat(2) behaviour.

Test that if O_BENEATH flag is set, openat() will only
open paths that have no .. component and do not start
with /.  Symlinks are also checked for the same restrictions.

Signed-off-by: David Drysdale <drysdale@google.com>
---
 .gitignore            |   1 +
 common/openat         |  61 ++++++++++++++++++++++++++++++
 src/Makefile          |   3 +-
 src/openat.c          | 100 +++++++++++++++++++++++++++++++++++++++++++++++++
 tests/generic/151     |  89 ++++++++++++++++++++++++++++++++++++++++++++
 tests/generic/151.out |   9 +++++
 tests/generic/152     | 101 ++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/generic/152.out |  23 ++++++++++++
 tests/generic/group   |   2 +
 9 files changed, 388 insertions(+), 1 deletion(-)
 create mode 100644 common/openat
 create mode 100644 src/openat.c
 create mode 100755 tests/generic/151
 create mode 100644 tests/generic/151.out
 create mode 100755 tests/generic/152
 create mode 100644 tests/generic/152.out

diff --git a/.gitignore b/.gitignore
index 2c9d640..6ea79ee 100644
--- a/.gitignore
+++ b/.gitignore
@@ -109,6 +109,7 @@
 /src/cloner
 /src/renameat2
 /src/t_rename_overwrite
+/src/openat

 # dmapi/ binaries
 /dmapi/src/common/cmd/read_invis
diff --git a/common/openat b/common/openat
new file mode 100644
index 0000000..8f04457
--- /dev/null
+++ b/common/openat
@@ -0,0 +1,61 @@
+######
+#
+# openat helpers
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2014 Google, Inc.  All Rights Reserved.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#-----------------------------------------------------------------------
+#
+
+#
+# Setup a collection of files to be opened.
+#
+_openat_setup()
+{
+	local dir=$1
+
+	mkdir -p $dir/subdir
+	echo 0123456789 > $dir/topfile
+	echo 0123456789 > $dir/subdir/bottomfile
+
+	ln -s subdir/bottomfile $dir/symlinkdown
+	ln -s ../topfile $dir/subdir/symlinkup
+	ln -s $dir/topfile $dir/subdir/symlinkout
+	ln -s bottomfile  $dir/subdir/symlinkin
+}
+
+#
+# Check whether the openat wrapper program is available
+#
+_requires_openat()
+{
+	OPENAT_PROG=$here/src/openat
+        _require_command $OPENAT_PROG
+}
+
+#
+# This checks whether the O_BENEATH flag is supported by the openat syscall
+#
+_requires_o_beneath()
+{
+	# Kernels that don't support O_BENEATH will silently accept it, so
+	# check for O_BENEATH behavior: attempting to open an absolute
+	# path should fail with EPERM.
+	$OPENAT_PROG -t -b $TEST_DIR
+	if [ $? -ne 0 ]; then
+		_notrun "kernel doesn't support O_BENEATH flag in openat syscall"
+	fi
+}
diff --git a/src/Makefile b/src/Makefile
index 4781736..d9f0508 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -11,7 +11,8 @@ TARGETS = dirstress fill fill2 getpagesize holes lstat64 \
 	devzero feature alloc fault fstest t_access_root \
 	godown resvtest writemod makeextents itrash rename \
 	multi_open_unlink dmiperf unwritten_sync genhashnames t_holes \
-	t_mmap_writev t_truncate_cmtime dirhash_collide t_rename_overwrite
+	t_mmap_writev t_truncate_cmtime dirhash_collide t_rename_overwrite \
+	openat

 LINUX_TARGETS = xfsctl bstat t_mtab getdevicesize preallo_rw_pattern_reader \
 	preallo_rw_pattern_writer ftrunc trunc fs_perms testx looptest \
diff --git a/src/openat.c b/src/openat.c
new file mode 100644
index 0000000..06e77c6
--- /dev/null
+++ b/src/openat.c
@@ -0,0 +1,100 @@
+/*
+ * Copyright (c) 2014 Google, Inc.
+ * All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it would be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write the Free Software Foundation,
+ * Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ * This is a trivial wrapper around the openat syscall.
+ */
+
+#include "global.h"
+
+#if !defined(O_BENEATH)
+#if defined(__x86_64__) || defined(__i386__)
+#define O_BENEATH	040000000
+#endif
+#endif
+
+void usage(const char *progname)
+{
+	fprintf(stderr, "Usage: %s [-f dirname] [-b] [-n] [-t] <file>\n",
+		progname);
+	fprintf(stderr,"    -f dirname : use this dir for dfd\n");
+	fprintf(stderr,"    -b         : open with O_BENEATH\n");
+	fprintf(stderr,"    -n         : open with O_NOFOLLOW\n");
+	fprintf(stderr,"    -t         : test for expected EPERM failure\n");
+	fprintf(stderr,"    -h         : show this usage message\n");
+	exit(1);
+}
+
+int main(int argc, char *argv[])
+{
+	int dfd = AT_FDCWD;
+	const char *path = NULL;
+	int flags = O_RDONLY;
+	int test = 0;
+	const char *progname;
+	int opt;
+	int fd;
+
+	progname = strrchr(argv[0], '/');
+	progname = (progname ? progname + 1 : argv[0]);
+
+	while ((opt = getopt(argc, argv, "f:bnth")) != EOF) {
+		switch (opt) {
+		case 'f':
+			dfd = open(optarg, O_RDONLY);
+			if (dfd < 0) {
+				perror("Failed to open -f argument");
+				exit(1);
+			}
+			break;
+		case 'b':
+#ifdef O_BENEATH
+			flags |= O_BENEATH;
+#else
+			printf("O_BENEATH flag unavailable");
+			exit(1);
+#endif
+			break;
+		case 'n':
+			flags |= O_NOFOLLOW;
+			break;
+		case 't':
+			test = 1;
+			break;
+		case 'h':
+		default:
+			usage(progname);
+		}
+	}
+	if (argc - optind != 1)
+		usage(progname);
+	path = argv[optind];
+
+	/* Perform the openat operation */
+	fd = openat(dfd, path, flags);
+
+	if (fd >= 0) {
+		close(fd);
+		return test;
+	} else {
+		if (test) {
+			return (errno != EPERM);
+		} else {
+			perror("");
+			return 1;
+		}
+	}
+}
diff --git a/tests/generic/151 b/tests/generic/151
new file mode 100755
index 0000000..26584f4
--- /dev/null
+++ b/tests/generic/151
@@ -0,0 +1,89 @@
+#! /bin/bash
+# FS QA Test No. generic/039
+#
+# Check openat system call
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2014 Google, Inc.  All Rights Reserved.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#-----------------------------------------------------------------------
+#
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1	# failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+    cd /
+    rm -f $tmp.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/openat
+
+_supported_fs generic
+_supported_os Linux
+
+_require_test
+_requires_openat
+_require_test_symlinks
+
+# real QA test starts here
+
+openat_dir=$TEST_DIR/$$
+rm -rf $openat_dir
+mkdir -p $openat_dir
+_openat_setup $openat_dir
+cd $openat_dir
+
+echo "normal behavior, AT_FDCWD"
+$OPENAT_PROG topfile
+$OPENAT_PROG subdir/bottomfile
+$OPENAT_PROG $openat_dir/topfile
+$OPENAT_PROG $openat_dir/subdir/bottomfile
+
+echo "normal behavior, dfd to main dir"
+$OPENAT_PROG -f $openat_dir topfile
+$OPENAT_PROG -f $openat_dir subdir/bottomfile
+$OPENAT_PROG -f $openat_dir subdir/../topfile
+
+echo "normal behavior, dfd to subdir"
+$OPENAT_PROG -f $openat_dir/subdir ../topfile
+$OPENAT_PROG -f $openat_dir/subdir bottomfile
+$OPENAT_PROG -f $openat_dir/subdir ../subdir/bottomfile
+$OPENAT_PROG -f $openat_dir/subdir symlinkup
+$OPENAT_PROG -f $openat_dir/subdir symlinkout
+
+echo "normal behavior, absolute path"
+$OPENAT_PROG $openat_dir/topfile
+$OPENAT_PROG -f $openat_dir $openat_dir/topfile
+$OPENAT_PROG -f $openat_dir/subdir $openat_dir/topfile
+
+echo "normal behavior, non-existent file (ENOENT)"
+$OPENAT_PROG bogus
+$OPENAT_PROG -f $openat_dir bogus
+$OPENAT_PROG -f $openat_dir/subdir bogus
+
+# success, all done
+status=0
+exit
+
diff --git a/tests/generic/151.out b/tests/generic/151.out
new file mode 100644
index 0000000..dc1168c
--- /dev/null
+++ b/tests/generic/151.out
@@ -0,0 +1,9 @@
+QA output created by 151
+normal behavior, AT_FDCWD
+normal behavior, dfd to main dir
+normal behavior, dfd to subdir
+normal behavior, absolute path
+normal behavior, non-existent file (ENOENT)
+No such file or directory
+No such file or directory
+No such file or directory
diff --git a/tests/generic/152 b/tests/generic/152
new file mode 100755
index 0000000..f5ccb25
--- /dev/null
+++ b/tests/generic/152
@@ -0,0 +1,101 @@
+#! /bin/bash
+# FS QA Test No. generic/040
+#
+# Check O_BENEATH argument to openat system call
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2014 Google, Inc.  All Rights Reserved.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#-----------------------------------------------------------------------
+#
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1	# failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+    cd /
+    rm -f $tmp.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/openat
+
+_supported_fs generic
+_supported_os Linux
+
+_require_test
+_requires_openat
+_requires_o_beneath
+_require_test_symlinks
+
+# real QA test starts here
+
+openat_dir=$TEST_DIR/$$
+rm -rf $openat_dir
+mkdir -p $openat_dir
+_openat_setup $openat_dir
+cd $openat_dir
+
+echo "Test O_BENEATH:"
+$OPENAT_PROG -b topfile
+$OPENAT_PROG -b subdir/bottomfile
+
+$OPENAT_PROG -b -f $openat_dir topfile
+$OPENAT_PROG -b -f $openat_dir subdir/bottomfile
+$OPENAT_PROG -b -f $openat_dir/subdir bottomfile
+$OPENAT_PROG -b -f $openat_dir/subdir .
+
+echo "  Symlinks without .. or leading / are OK"
+$OPENAT_PROG -b -f $openat_dir symlinkdown
+$OPENAT_PROG -b -f $openat_dir subdir/symlinkin
+$OPENAT_PROG -b -f $openat_dir/subdir symlinkin
+
+echo "  ...unless of course we specify O_NOFOLLOW (ELOOP)"
+$OPENAT_PROG -b -n -f $openat_dir symlinkdown
+$OPENAT_PROG -b -n -f $openat_dir subdir/symlinkin
+$OPENAT_PROG -b -n -f $openat_dir/subdir symlinkin
+
+echo "  Can't open paths with .. in them (EPERM)"
+$OPENAT_PROG -b -f $openat_dir subdir/../topfile
+$OPENAT_PROG -b -f $openat_dir/subdir ../topfile
+$OPENAT_PROG -b -f $openat_dir/subdir ../subdir/bottomfile
+$OPENAT_PROG -b -f $openat_dir/subdir ..
+
+echo "  Can't open paths starting with / (EPERM)"
+$OPENAT_PROG -b $openat_dir/topfile
+$OPENAT_PROG -b -f $openat_dir $openat_dir/topfile
+$OPENAT_PROG -b -f $openat_dir/subdir $openat_dir/topfile
+
+echo "  Can't sneak around constraints with symlinks (EPERM)"
+$OPENAT_PROG -b -f $openat_dir/subdir symlinkup
+$OPENAT_PROG -b -f $openat_dir/subdir symlinkout
+$OPENAT_PROG -b -f $openat_dir/subdir ../symlinkdown
+$OPENAT_PROG -b -f $openat_dir subdir/symlinkup
+
+echo "  Can't open files below pre-resolved symlinks in /proc"
+$OPENAT_PROG -b -f /proc/self root/etc/passwd
+
+# success, all done
+status=0
+exit
+
diff --git a/tests/generic/152.out b/tests/generic/152.out
new file mode 100644
index 0000000..57d9f50
--- /dev/null
+++ b/tests/generic/152.out
@@ -0,0 +1,23 @@
+QA output created by 152
+Test O_BENEATH:
+  Symlinks without .. or leading / are OK
+  ...unless of course we specify O_NOFOLLOW (ELOOP)
+Too many levels of symbolic links
+Too many levels of symbolic links
+Too many levels of symbolic links
+  Can't open paths with .. in them (EPERM)
+Operation not permitted
+Operation not permitted
+Operation not permitted
+Operation not permitted
+  Can't open paths starting with / (EPERM)
+Operation not permitted
+Operation not permitted
+Operation not permitted
+  Can't sneak around constraints with symlinks (EPERM)
+Operation not permitted
+Operation not permitted
+Operation not permitted
+Operation not permitted
+  Can't open files below pre-resolved symlinks in /proc
+Operation not permitted
diff --git a/tests/generic/group b/tests/generic/group
index f2eb87a..030a076 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -96,6 +96,8 @@
 133 rw auto
 135 metadata auto quick
 141 rw auto quick
+151 metadata auto quick
+152 metadata auto quick
 169 rw metadata auto quick
 184 metadata auto quick
 192 atime auto
--
1.9.1

[PATCHv3 xfstests 2/3] generic: test openat and new O_BENEATH flag

[David Drysdale]

Test basic openat(2) behaviour.

Test that if O_BENEATH flag is set, openat() will only
open paths that have no .. component and do not start
with /.  Symlinks are also checked for the same restrictions.

Signed-off-by: David Drysdale <drysdale-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
---
 .gitignore            |   1 +
 common/openat         |  61 ++++++++++++++++++++++++++++++
 src/Makefile          |   3 +-
 src/openat.c          | 100 +++++++++++++++++++++++++++++++++++++++++++++++++
 tests/generic/151     |  89 ++++++++++++++++++++++++++++++++++++++++++++
 tests/generic/151.out |   9 +++++
 tests/generic/152     | 101 ++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/generic/152.out |  23 ++++++++++++
 tests/generic/group   |   2 +
 9 files changed, 388 insertions(+), 1 deletion(-)
 create mode 100644 common/openat
 create mode 100644 src/openat.c
 create mode 100755 tests/generic/151
 create mode 100644 tests/generic/151.out
 create mode 100755 tests/generic/152
 create mode 100644 tests/generic/152.out

diff --git a/.gitignore b/.gitignore
index 2c9d640..6ea79ee 100644
--- a/.gitignore
+++ b/.gitignore
@@ -109,6 +109,7 @@
 /src/cloner
 /src/renameat2
 /src/t_rename_overwrite
+/src/openat

 # dmapi/ binaries
 /dmapi/src/common/cmd/read_invis
diff --git a/common/openat b/common/openat
new file mode 100644
index 0000000..8f04457
--- /dev/null
+++ b/common/openat
@@ -0,0 +1,61 @@
+######
+#
+# openat helpers
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2014 Google, Inc.  All Rights Reserved.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#-----------------------------------------------------------------------
+#
+
+#
+# Setup a collection of files to be opened.
+#
+_openat_setup()
+{
+	local dir=$1
+
+	mkdir -p $dir/subdir
+	echo 0123456789 > $dir/topfile
+	echo 0123456789 > $dir/subdir/bottomfile
+
+	ln -s subdir/bottomfile $dir/symlinkdown
+	ln -s ../topfile $dir/subdir/symlinkup
+	ln -s $dir/topfile $dir/subdir/symlinkout
+	ln -s bottomfile  $dir/subdir/symlinkin
+}
+
+#
+# Check whether the openat wrapper program is available
+#
+_requires_openat()
+{
+	OPENAT_PROG=$here/src/openat
+        _require_command $OPENAT_PROG
+}
+
+#
+# This checks whether the O_BENEATH flag is supported by the openat syscall
+#
+_requires_o_beneath()
+{
+	# Kernels that don't support O_BENEATH will silently accept it, so
+	# check for O_BENEATH behavior: attempting to open an absolute
+	# path should fail with EPERM.
+	$OPENAT_PROG -t -b $TEST_DIR
+	if [ $? -ne 0 ]; then
+		_notrun "kernel doesn't support O_BENEATH flag in openat syscall"
+	fi
+}
diff --git a/src/Makefile b/src/Makefile
index 4781736..d9f0508 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -11,7 +11,8 @@ TARGETS = dirstress fill fill2 getpagesize holes lstat64 \
 	devzero feature alloc fault fstest t_access_root \
 	godown resvtest writemod makeextents itrash rename \
 	multi_open_unlink dmiperf unwritten_sync genhashnames t_holes \
-	t_mmap_writev t_truncate_cmtime dirhash_collide t_rename_overwrite
+	t_mmap_writev t_truncate_cmtime dirhash_collide t_rename_overwrite \
+	openat

 LINUX_TARGETS = xfsctl bstat t_mtab getdevicesize preallo_rw_pattern_reader \
 	preallo_rw_pattern_writer ftrunc trunc fs_perms testx looptest \
diff --git a/src/openat.c b/src/openat.c
new file mode 100644
index 0000000..06e77c6
--- /dev/null
+++ b/src/openat.c
@@ -0,0 +1,100 @@
+/*
+ * Copyright (c) 2014 Google, Inc.
+ * All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it would be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write the Free Software Foundation,
+ * Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ * This is a trivial wrapper around the openat syscall.
+ */
+
+#include "global.h"
+
+#if !defined(O_BENEATH)
+#if defined(__x86_64__) || defined(__i386__)
+#define O_BENEATH	040000000
+#endif
+#endif
+
+void usage(const char *progname)
+{
+	fprintf(stderr, "Usage: %s [-f dirname] [-b] [-n] [-t] <file>\n",
+		progname);
+	fprintf(stderr,"    -f dirname : use this dir for dfd\n");
+	fprintf(stderr,"    -b         : open with O_BENEATH\n");
+	fprintf(stderr,"    -n         : open with O_NOFOLLOW\n");
+	fprintf(stderr,"    -t         : test for expected EPERM failure\n");
+	fprintf(stderr,"    -h         : show this usage message\n");
+	exit(1);
+}
+
+int main(int argc, char *argv[])
+{
+	int dfd = AT_FDCWD;
+	const char *path = NULL;
+	int flags = O_RDONLY;
+	int test = 0;
+	const char *progname;
+	int opt;
+	int fd;
+
+	progname = strrchr(argv[0], '/');
+	progname = (progname ? progname + 1 : argv[0]);
+
+	while ((opt = getopt(argc, argv, "f:bnth")) != EOF) {
+		switch (opt) {
+		case 'f':
+			dfd = open(optarg, O_RDONLY);
+			if (dfd < 0) {
+				perror("Failed to open -f argument");
+				exit(1);
+			}
+			break;
+		case 'b':
+#ifdef O_BENEATH
+			flags |= O_BENEATH;
+#else
+			printf("O_BENEATH flag unavailable");
+			exit(1);
+#endif
+			break;
+		case 'n':
+			flags |= O_NOFOLLOW;
+			break;
+		case 't':
+			test = 1;
+			break;
+		case 'h':
+		default:
+			usage(progname);
+		}
+	}
+	if (argc - optind != 1)
+		usage(progname);
+	path = argv[optind];
+
+	/* Perform the openat operation */
+	fd = openat(dfd, path, flags);
+
+	if (fd >= 0) {
+		close(fd);
+		return test;
+	} else {
+		if (test) {
+			return (errno != EPERM);
+		} else {
+			perror("");
+			return 1;
+		}
+	}
+}
diff --git a/tests/generic/151 b/tests/generic/151
new file mode 100755
index 0000000..26584f4
--- /dev/null
+++ b/tests/generic/151
@@ -0,0 +1,89 @@
+#! /bin/bash
+# FS QA Test No. generic/039
+#
+# Check openat system call
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2014 Google, Inc.  All Rights Reserved.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#-----------------------------------------------------------------------
+#
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1	# failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+    cd /
+    rm -f $tmp.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/openat
+
+_supported_fs generic
+_supported_os Linux
+
+_require_test
+_requires_openat
+_require_test_symlinks
+
+# real QA test starts here
+
+openat_dir=$TEST_DIR/$$
+rm -rf $openat_dir
+mkdir -p $openat_dir
+_openat_setup $openat_dir
+cd $openat_dir
+
+echo "normal behavior, AT_FDCWD"
+$OPENAT_PROG topfile
+$OPENAT_PROG subdir/bottomfile
+$OPENAT_PROG $openat_dir/topfile
+$OPENAT_PROG $openat_dir/subdir/bottomfile
+
+echo "normal behavior, dfd to main dir"
+$OPENAT_PROG -f $openat_dir topfile
+$OPENAT_PROG -f $openat_dir subdir/bottomfile
+$OPENAT_PROG -f $openat_dir subdir/../topfile
+
+echo "normal behavior, dfd to subdir"
+$OPENAT_PROG -f $openat_dir/subdir ../topfile
+$OPENAT_PROG -f $openat_dir/subdir bottomfile
+$OPENAT_PROG -f $openat_dir/subdir ../subdir/bottomfile
+$OPENAT_PROG -f $openat_dir/subdir symlinkup
+$OPENAT_PROG -f $openat_dir/subdir symlinkout
+
+echo "normal behavior, absolute path"
+$OPENAT_PROG $openat_dir/topfile
+$OPENAT_PROG -f $openat_dir $openat_dir/topfile
+$OPENAT_PROG -f $openat_dir/subdir $openat_dir/topfile
+
+echo "normal behavior, non-existent file (ENOENT)"
+$OPENAT_PROG bogus
+$OPENAT_PROG -f $openat_dir bogus
+$OPENAT_PROG -f $openat_dir/subdir bogus
+
+# success, all done
+status=0
+exit
+
diff --git a/tests/generic/151.out b/tests/generic/151.out
new file mode 100644
index 0000000..dc1168c
--- /dev/null
+++ b/tests/generic/151.out
@@ -0,0 +1,9 @@
+QA output created by 151
+normal behavior, AT_FDCWD
+normal behavior, dfd to main dir
+normal behavior, dfd to subdir
+normal behavior, absolute path
+normal behavior, non-existent file (ENOENT)
+No such file or directory
+No such file or directory
+No such file or directory
diff --git a/tests/generic/152 b/tests/generic/152
new file mode 100755
index 0000000..f5ccb25
--- /dev/null
+++ b/tests/generic/152
@@ -0,0 +1,101 @@
+#! /bin/bash
+# FS QA Test No. generic/040
+#
+# Check O_BENEATH argument to openat system call
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2014 Google, Inc.  All Rights Reserved.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#-----------------------------------------------------------------------
+#
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1	# failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+    cd /
+    rm -f $tmp.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/openat
+
+_supported_fs generic
+_supported_os Linux
+
+_require_test
+_requires_openat
+_requires_o_beneath
+_require_test_symlinks
+
+# real QA test starts here
+
+openat_dir=$TEST_DIR/$$
+rm -rf $openat_dir
+mkdir -p $openat_dir
+_openat_setup $openat_dir
+cd $openat_dir
+
+echo "Test O_BENEATH:"
+$OPENAT_PROG -b topfile
+$OPENAT_PROG -b subdir/bottomfile
+
+$OPENAT_PROG -b -f $openat_dir topfile
+$OPENAT_PROG -b -f $openat_dir subdir/bottomfile
+$OPENAT_PROG -b -f $openat_dir/subdir bottomfile
+$OPENAT_PROG -b -f $openat_dir/subdir .
+
+echo "  Symlinks without .. or leading / are OK"
+$OPENAT_PROG -b -f $openat_dir symlinkdown
+$OPENAT_PROG -b -f $openat_dir subdir/symlinkin
+$OPENAT_PROG -b -f $openat_dir/subdir symlinkin
+
+echo "  ...unless of course we specify O_NOFOLLOW (ELOOP)"
+$OPENAT_PROG -b -n -f $openat_dir symlinkdown
+$OPENAT_PROG -b -n -f $openat_dir subdir/symlinkin
+$OPENAT_PROG -b -n -f $openat_dir/subdir symlinkin
+
+echo "  Can't open paths with .. in them (EPERM)"
+$OPENAT_PROG -b -f $openat_dir subdir/../topfile
+$OPENAT_PROG -b -f $openat_dir/subdir ../topfile
+$OPENAT_PROG -b -f $openat_dir/subdir ../subdir/bottomfile
+$OPENAT_PROG -b -f $openat_dir/subdir ..
+
+echo "  Can't open paths starting with / (EPERM)"
+$OPENAT_PROG -b $openat_dir/topfile
+$OPENAT_PROG -b -f $openat_dir $openat_dir/topfile
+$OPENAT_PROG -b -f $openat_dir/subdir $openat_dir/topfile
+
+echo "  Can't sneak around constraints with symlinks (EPERM)"
+$OPENAT_PROG -b -f $openat_dir/subdir symlinkup
+$OPENAT_PROG -b -f $openat_dir/subdir symlinkout
+$OPENAT_PROG -b -f $openat_dir/subdir ../symlinkdown
+$OPENAT_PROG -b -f $openat_dir subdir/symlinkup
+
+echo "  Can't open files below pre-resolved symlinks in /proc"
+$OPENAT_PROG -b -f /proc/self root/etc/passwd
+
+# success, all done
+status=0
+exit
+
diff --git a/tests/generic/152.out b/tests/generic/152.out
new file mode 100644
index 0000000..57d9f50
--- /dev/null
+++ b/tests/generic/152.out
@@ -0,0 +1,23 @@
+QA output created by 152
+Test O_BENEATH:
+  Symlinks without .. or leading / are OK
+  ...unless of course we specify O_NOFOLLOW (ELOOP)
+Too many levels of symbolic links
+Too many levels of symbolic links
+Too many levels of symbolic links
+  Can't open paths with .. in them (EPERM)
+Operation not permitted
+Operation not permitted
+Operation not permitted
+Operation not permitted
+  Can't open paths starting with / (EPERM)
+Operation not permitted
+Operation not permitted
+Operation not permitted
+  Can't sneak around constraints with symlinks (EPERM)
+Operation not permitted
+Operation not permitted
+Operation not permitted
+Operation not permitted
+  Can't open files below pre-resolved symlinks in /proc
+Operation not permitted
diff --git a/tests/generic/group b/tests/generic/group
index f2eb87a..030a076 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -96,6 +96,8 @@
 133 rw auto
 135 metadata auto quick
 141 rw auto quick
+151 metadata auto quick
+152 metadata auto quick
 169 rw metadata auto quick
 184 metadata auto quick
 192 atime auto
--
1.9.1

[PATCHv3 man-pages 3/3] open.2: describe O_BENEATH flag

[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 956531b24b26..be40dd7710df 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
@@ -984,6 +1009,13 @@ did not match the owner of the file and the caller was not privileged
 The operation was prevented by a file seal; see
 .BR fcntl (2).
 .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.2.0.rc0.207.ga3a616c

Re: [PATCHv3 man-pages 3/3] open.2: describe O_BENEATH flag

[Michael Kerrisk (man-pages)]

On 03/09/2015 03:00 PM, David Drysdale wrote:
> Signed-off-by: David Drysdale <drysdale@google.com>

Hi David,

The text looks good insofar as it goes. But, it would be helpful
to have sentence or to that explains why this flag exists.
Could you add that, please?

Thanks,

Michael

> ---
>  man2/open.2 | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/man2/open.2 b/man2/open.2
> index 956531b24b26..be40dd7710df 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
> @@ -984,6 +1009,13 @@ did not match the owner of the file and the caller was not privileged
>  The operation was prevented by a file seal; see
>  .BR fcntl (2).
>  .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.2.0.rc0.207.ga3a616c
> 


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

Re: [PATCHv3 man-pages 3/3] open.2: describe O_BENEATH flag

[David Drysdale]

On Mon, Mar 9, 2015 at 2:32 PM, Michael Kerrisk (man-pages)
<mtk.manpages@gmail.com> wrote:
> On 03/09/2015 03:00 PM, David Drysdale wrote:
>> Signed-off-by: David Drysdale <drysdale@google.com>
>
> Hi David,
>
> The text looks good insofar as it goes. But, it would be helpful
> to have sentence or to that explains why this flag exists.
> Could you add that, please?
>
> Thanks,
>
> Michael

How about something like:

  This feature allows applications to be sure that the opened file
  is  within  the  specified directory, regardless of the original
  source of the pathname argument.  Some  security-conscious  pro‐
  grams  may  further ensure this by imposing a system call filter
  (with seccomp(2)) that requires this flag for all open()  opera‐
  tions,  so  that the program cannot open files outside of speci‐
  fied directories even if subverted.

(Also, I realize that I somehow failed to notice that the flags
are listed in alphabetical order, so I'll move the text up, as
in the updated diff below).

Thanks,
David

---
 man2/open.2 | 44 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/man2/open.2 b/man2/open.2
index 956531b24b26..ece1fa90775a 100644
--- a/man2/open.2
+++ b/man2/open.2
@@ -201,6 +201,43 @@ See
 for further details.
 See also BUGS, below.
 .TP
+.B O_BENEATH " (since Linux 4.??)"
+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.
+
+This feature allows applications to be sure that the opened file is
+within the specified directory, regardless of the original source of the
+.I pathname
+argument.
+Some security-conscious programs may further ensure
+this by imposing a system call filter (with
+.BR seccomp (2))
+that requires this flag for all
+.BR open ()
+operations, so that the program cannot open files outside of
+specified directories even if subverted.
+.TP
 .BR O_CLOEXEC " (since Linux 2.6.23)"
 .\" NOTE! several other man pages refer to this text
 Enable the close-on-exec flag for the new file descriptor.
@@ -984,6 +1021,13 @@ did not match the owner of the file and the
caller was not privileged
 The operation was prevented by a file seal; see
 .BR fcntl (2).
 .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.2.0.rc0.207.ga3a616c

Re: [PATCHv3 man-pages 3/3] open.2: describe O_BENEATH flag

[David Drysdale]

On Mon, Mar 9, 2015 at 2:32 PM, Michael Kerrisk (man-pages)
<mtk.manpages@gmail.com> wrote:
> On 03/09/2015 03:00 PM, David Drysdale wrote:
>> Signed-off-by: David Drysdale <drysdale@google.com>
>
> Hi David,
>
> The text looks good insofar as it goes. But, it would be helpful
> to have sentence or to that explains why this flag exists.
> Could you add that, please?
>
> Thanks,
>
> Michael

How about something like:

  This feature allows applications to be sure that the opened file
  is  within  the  specified directory, regardless of the original
  source of the pathname argument.  Some  security-conscious  pro‐
  grams  may  further ensure this by imposing a system call filter
  (with seccomp(2)) that requires this flag for all open()  opera‐
  tions,  so  that the program cannot open files outside of speci‐
  fied directories even if subverted.

(Also, I realize that I somehow failed to notice that the flags
are listed in alphabetical order, so I'll move the text up, as
in the updated diff below).

Thanks,
David

---
 man2/open.2 | 44 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/man2/open.2 b/man2/open.2
index 956531b24b26..ece1fa90775a 100644
--- a/man2/open.2
+++ b/man2/open.2
@@ -201,6 +201,43 @@ See
 for further details.
 See also BUGS, below.
 .TP
+.B O_BENEATH " (since Linux 4.??)"
+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.
+
+This feature allows applications to be sure that the opened file is
+within the specified directory, regardless of the original source of the
+.I pathname
+argument.
+Some security-conscious programs may further ensure
+this by imposing a system call filter (with
+.BR seccomp (2))
+that requires this flag for all
+.BR open ()
+operations, so that the program cannot open files outside of
+specified directories even if subverted.
+.TP
 .BR O_CLOEXEC " (since Linux 2.6.23)"
 .\" NOTE! several other man pages refer to this text
 Enable the close-on-exec flag for the new file descriptor.
@@ -984,6 +1021,13 @@ did not match the owner of the file and the
caller was not privileged
 The operation was prevented by a file seal; see
 .BR fcntl (2).
 .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.2.0.rc0.207.ga3a616c
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv3 man-pages 3/3] open.2: describe O_BENEATH flag

[Michael Kerrisk (man-pages)]

On 9 March 2015 at 16:16, David Drysdale <drysdale@google.com> wrote:
> On Mon, Mar 9, 2015 at 2:32 PM, Michael Kerrisk (man-pages)
> <mtk.manpages@gmail.com> wrote:
>> On 03/09/2015 03:00 PM, David Drysdale wrote:
>>> Signed-off-by: David Drysdale <drysdale@google.com>
>>
>> Hi David,
>>
>> The text looks good insofar as it goes. But, it would be helpful
>> to have sentence or to that explains why this flag exists.
>> Could you add that, please?
>>
>> Thanks,
>>
>> Michael
>
> How about something like:
>
>   This feature allows applications to be sure that the opened file
>   is  within  the  specified directory, regardless of the original
>   source of the pathname argument.  Some  security-conscious  pro‐
>   grams  may  further ensure this by imposing a system call filter
>   (with seccomp(2)) that requires this flag for all open()  opera‐
>   tions,  so  that the program cannot open files outside of speci‐
>   fied directories even if subverted.
>
> (Also, I realize that I somehow failed to notice that the flags
> are listed in alphabetical order, so I'll move the text up, as
> in the updated diff below).

That looks good to me. Thanks!

Cheers,

Michael


> ---
>  man2/open.2 | 44 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 44 insertions(+)
>
> diff --git a/man2/open.2 b/man2/open.2
> index 956531b24b26..ece1fa90775a 100644
> --- a/man2/open.2
> +++ b/man2/open.2
> @@ -201,6 +201,43 @@ See
>  for further details.
>  See also BUGS, below.
>  .TP
> +.B O_BENEATH " (since Linux 4.??)"
> +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.
> +
> +This feature allows applications to be sure that the opened file is
> +within the specified directory, regardless of the original source of the
> +.I pathname
> +argument.
> +Some security-conscious programs may further ensure
> +this by imposing a system call filter (with
> +.BR seccomp (2))
> +that requires this flag for all
> +.BR open ()
> +operations, so that the program cannot open files outside of
> +specified directories even if subverted.
> +.TP
>  .BR O_CLOEXEC " (since Linux 2.6.23)"
>  .\" NOTE! several other man pages refer to this text
>  Enable the close-on-exec flag for the new file descriptor.
> @@ -984,6 +1021,13 @@ did not match the owner of the file and the
> caller was not privileged
>  The operation was prevented by a file seal; see
>  .BR fcntl (2).
>  .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.2.0.rc0.207.ga3a616c



-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

Re: [PATCHv3 xfstests 2/3] generic: test openat and new O_BENEATH flag

[Dave Chinner]

On Mon, Mar 09, 2015 at 02:00:11PM +0000, David Drysdale wrote:
> Test basic openat(2) behaviour.
> 
> Test that if O_BENEATH flag is set, openat() will only
> open paths that have no .. component and do not start
> with /.  Symlinks are also checked for the same restrictions.
> 
> Signed-off-by: David Drysdale <drysdale@google.com>
> ---
>  .gitignore            |   1 +
>  common/openat         |  61 ++++++++++++++++++++++++++++++
>  src/Makefile          |   3 +-
>  src/openat.c          | 100 +++++++++++++++++++++++++++++++++++++++++++++++++

This strikes me as something that shoul dbe added to xfs_io for
testing, as it already supports a heap of other open flags and
xfstests is already dependent on it.

>  tests/generic/151     |  89 ++++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/151.out |   9 +++++
>  tests/generic/152     | 101 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/152.out |  23 ++++++++++++
>  tests/generic/group   |   2 +

I'd also prefer one patch per new test - it's easier to review...

> +_openat_setup()
> +{
> +	local dir=$1
> +
> +	mkdir -p $dir/subdir
> +	echo 0123456789 > $dir/topfile
> +	echo 0123456789 > $dir/subdir/bottomfile
> +
> +	ln -s subdir/bottomfile $dir/symlinkdown
> +	ln -s ../topfile $dir/subdir/symlinkup
> +	ln -s $dir/topfile $dir/subdir/symlinkout
> +	ln -s bottomfile  $dir/subdir/symlinkin
> +}
> +
> +#
> +# Check whether the openat wrapper program is available
> +#
> +_requires_openat()
> +{
> +	OPENAT_PROG=$here/src/openat
> +        _require_command $OPENAT_PROG
> +}

if this is part of xfs_io, then _requires_xfs_io_command "open -b"
could be used to test if the command is supported, and no need for
this function at all.

> +#
> +# This checks whether the O_BENEATH flag is supported by the openat syscall
> +#
> +_requires_o_beneath()
> +{
> +	# Kernels that don't support O_BENEATH will silently accept it, so
> +	# check for O_BENEATH behavior: attempting to open an absolute
> +	# path should fail with EPERM.
> +	$OPENAT_PROG -t -b $TEST_DIR
> +	if [ $? -ne 0 ]; then
> +		_notrun "kernel doesn't support O_BENEATH flag in openat syscall"
> +	fi
> +}

as running the command would tell us if the kernel supports it, too.

> +#endif
> +#endif
> +
> +void usage(const char *progname)
> +{
> +	fprintf(stderr, "Usage: %s [-f dirname] [-b] [-n] [-t] <file>\n",
> +		progname);
> +	fprintf(stderr,"    -f dirname : use this dir for dfd\n");
> +	fprintf(stderr,"    -b         : open with O_BENEATH\n");
> +	fprintf(stderr,"    -n         : open with O_NOFOLLOW\n");
> +	fprintf(stderr,"    -t         : test for expected EPERM failure\n");
> +	fprintf(stderr,"    -h         : show this usage message\n");
> +	exit(1);

Hmm - you're also testing O_NOFOLLOW behaviour too? Perhaps that
should be mentioned/added to xfs_io, too?

The reason I suggest this, even though it's a little more work, is
tht we can then re-use the new flags in other tests easily without
needing to write new helper functions...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCHv3 xfstests 2/3] generic: test openat and new O_BENEATH flag

[Dave Chinner]

On Mon, Mar 09, 2015 at 02:00:11PM +0000, David Drysdale wrote:
> Test basic openat(2) behaviour.
> 
> Test that if O_BENEATH flag is set, openat() will only
> open paths that have no .. component and do not start
> with /.  Symlinks are also checked for the same restrictions.
> 
> Signed-off-by: David Drysdale <drysdale-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
> ---
>  .gitignore            |   1 +
>  common/openat         |  61 ++++++++++++++++++++++++++++++
>  src/Makefile          |   3 +-
>  src/openat.c          | 100 +++++++++++++++++++++++++++++++++++++++++++++++++

This strikes me as something that shoul dbe added to xfs_io for
testing, as it already supports a heap of other open flags and
xfstests is already dependent on it.

>  tests/generic/151     |  89 ++++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/151.out |   9 +++++
>  tests/generic/152     | 101 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/152.out |  23 ++++++++++++
>  tests/generic/group   |   2 +

I'd also prefer one patch per new test - it's easier to review...

> +_openat_setup()
> +{
> +	local dir=$1
> +
> +	mkdir -p $dir/subdir
> +	echo 0123456789 > $dir/topfile
> +	echo 0123456789 > $dir/subdir/bottomfile
> +
> +	ln -s subdir/bottomfile $dir/symlinkdown
> +	ln -s ../topfile $dir/subdir/symlinkup
> +	ln -s $dir/topfile $dir/subdir/symlinkout
> +	ln -s bottomfile  $dir/subdir/symlinkin
> +}
> +
> +#
> +# Check whether the openat wrapper program is available
> +#
> +_requires_openat()
> +{
> +	OPENAT_PROG=$here/src/openat
> +        _require_command $OPENAT_PROG
> +}

if this is part of xfs_io, then _requires_xfs_io_command "open -b"
could be used to test if the command is supported, and no need for
this function at all.

> +#
> +# This checks whether the O_BENEATH flag is supported by the openat syscall
> +#
> +_requires_o_beneath()
> +{
> +	# Kernels that don't support O_BENEATH will silently accept it, so
> +	# check for O_BENEATH behavior: attempting to open an absolute
> +	# path should fail with EPERM.
> +	$OPENAT_PROG -t -b $TEST_DIR
> +	if [ $? -ne 0 ]; then
> +		_notrun "kernel doesn't support O_BENEATH flag in openat syscall"
> +	fi
> +}

as running the command would tell us if the kernel supports it, too.

> +#endif
> +#endif
> +
> +void usage(const char *progname)
> +{
> +	fprintf(stderr, "Usage: %s [-f dirname] [-b] [-n] [-t] <file>\n",
> +		progname);
> +	fprintf(stderr,"    -f dirname : use this dir for dfd\n");
> +	fprintf(stderr,"    -b         : open with O_BENEATH\n");
> +	fprintf(stderr,"    -n         : open with O_NOFOLLOW\n");
> +	fprintf(stderr,"    -t         : test for expected EPERM failure\n");
> +	fprintf(stderr,"    -h         : show this usage message\n");
> +	exit(1);

Hmm - you're also testing O_NOFOLLOW behaviour too? Perhaps that
should be mentioned/added to xfs_io, too?

The reason I suggest this, even though it's a little more work, is
tht we can then re-use the new flags in other tests easily without
needing to write new helper functions...

Cheers,

Dave.
-- 
Dave Chinner
david-FqsqvQoI3Ljby3iVrkZq2A@public.gmane.org

Re: [PATCHv3 xfstests 2/3] generic: test openat and new O_BENEATH flag

[Kees Cook]

On Mon, Mar 16, 2015 at 4:24 PM, Dave Chinner <david@fromorbit.com> wrote:
> On Mon, Mar 09, 2015 at 02:00:11PM +0000, David Drysdale wrote:
>> Test basic openat(2) behaviour.
>>
>> Test that if O_BENEATH flag is set, openat() will only
>> open paths that have no .. component and do not start
>> with /.  Symlinks are also checked for the same restrictions.
>>
>> Signed-off-by: David Drysdale <drysdale@google.com>
>> ---
>>  .gitignore            |   1 +
>>  common/openat         |  61 ++++++++++++++++++++++++++++++
>>  src/Makefile          |   3 +-
>>  src/openat.c          | 100 +++++++++++++++++++++++++++++++++++++++++++++++++
>
> This strikes me as something that shoul dbe added to xfs_io for
> testing, as it already supports a heap of other open flags and
> xfstests is already dependent on it.

While I don't see a problem adding this to xfs_io, I'd still like to
see this test live in the kernel tree too. Having it in the same
source means more testing, IMO.

-Kees

-- 
Kees Cook
Chrome OS Security

Re: [PATCHv3 xfstests 2/3] generic: test openat and new O_BENEATH flag

[Dave Chinner]

On Tue, Mar 17, 2015 at 08:33:27AM -0700, Kees Cook wrote:
> On Mon, Mar 16, 2015 at 4:24 PM, Dave Chinner
> <david@fromorbit.com> wrote:
> > On Mon, Mar 09, 2015 at 02:00:11PM +0000, David Drysdale wrote:
> >> Test basic openat(2) behaviour.
> >>
> >> Test that if O_BENEATH flag is set, openat() will only open
> >> paths that have no .. component and do not start with /.
> >> Symlinks are also checked for the same restrictions.
> >>
> >> Signed-off-by: David Drysdale <drysdale@google.com> ---
> >> .gitignore            |   1 + common/openat         |  61
> >> ++++++++++++++++++++++++++++++ src/Makefile          |   3 +-
> >> src/openat.c          | 100
> >> +++++++++++++++++++++++++++++++++++++++++++++++++
> >
> > This strikes me as something that shoul dbe added to xfs_io for
> > testing, as it already supports a heap of other open flags and
> > xfstests is already dependent on it.
> 
> While I don't see a problem adding this to xfs_io, I'd still like
> to see this test live in the kernel tree too.

You can do whatever you want with the kernel tree - it doesn't
concern me at all.

> Having it in the
> same source means more testing, IMO.

That's complete bunk, though.  This helper binary is a limited use,
one shot test that can't be combined with anything else.  "sharing"
the source code across multiple different test suites doesn't change
that. However, putting support into xfs_io means it will get more
testing because we can *easily* use the functionality in many, many
different ways.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCHv3 xfstests 2/3] generic: test openat and new O_BENEATH flag

[Dave Chinner]

On Tue, Mar 17, 2015 at 08:33:27AM -0700, Kees Cook wrote:
> On Mon, Mar 16, 2015 at 4:24 PM, Dave Chinner
> <david-FqsqvQoI3Ljby3iVrkZq2A@public.gmane.org> wrote:
> > On Mon, Mar 09, 2015 at 02:00:11PM +0000, David Drysdale wrote:
> >> Test basic openat(2) behaviour.
> >>
> >> Test that if O_BENEATH flag is set, openat() will only open
> >> paths that have no .. component and do not start with /.
> >> Symlinks are also checked for the same restrictions.
> >>
> >> Signed-off-by: David Drysdale <drysdale-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> ---
> >> .gitignore            |   1 + common/openat         |  61
> >> ++++++++++++++++++++++++++++++ src/Makefile          |   3 +-
> >> src/openat.c          | 100
> >> +++++++++++++++++++++++++++++++++++++++++++++++++
> >
> > This strikes me as something that shoul dbe added to xfs_io for
> > testing, as it already supports a heap of other open flags and
> > xfstests is already dependent on it.
> 
> While I don't see a problem adding this to xfs_io, I'd still like
> to see this test live in the kernel tree too.

You can do whatever you want with the kernel tree - it doesn't
concern me at all.

> Having it in the
> same source means more testing, IMO.

That's complete bunk, though.  This helper binary is a limited use,
one shot test that can't be combined with anything else.  "sharing"
the source code across multiple different test suites doesn't change
that. However, putting support into xfs_io means it will get more
testing because we can *easily* use the functionality in many, many
different ways.

Cheers,

Dave.
-- 
Dave Chinner
david-FqsqvQoI3Ljby3iVrkZq2A@public.gmane.org

Re: [PATCHv3 xfstests 2/3] generic: test openat and new O_BENEATH flag

[David Drysdale]

[resend, remembering to tick the Plaintext button this time -- sorry]

On Wed, Mar 18, 2015 at 2:52 AM, Dave Chinner <david@fromorbit.com> wrote:
> On Tue, Mar 17, 2015 at 08:33:27AM -0700, Kees Cook wrote:
>> On Mon, Mar 16, 2015 at 4:24 PM, Dave Chinner
>> <david@fromorbit.com> wrote:
>> > On Mon, Mar 09, 2015 at 02:00:11PM +0000, David Drysdale wrote:
>> >> Test basic openat(2) behaviour.
>> >>
>> >> Test that if O_BENEATH flag is set, openat() will only open
>> >> paths that have no .. component and do not start with /.
>> >> Symlinks are also checked for the same restrictions.
>> >>
>> >> Signed-off-by: David Drysdale <drysdale@google.com> ---
>> >> .gitignore            |   1 + common/openat         |  61
>> >> ++++++++++++++++++++++++++++++ src/Makefile          |   3 +-
>> >> src/openat.c          | 100
>> >> +++++++++++++++++++++++++++++++++++++++++++++++++
>> >
>> > This strikes me as something that shoul dbe added to xfs_io for
>> > testing, as it already supports a heap of other open flags and
>> > xfstests is already dependent on it.
>>
>> While I don't see a problem adding this to xfs_io, I'd still like
>> to see this test live in the kernel tree too.
>
> You can do whatever you want with the kernel tree - it doesn't
> concern me at all.
>
>> Having it in the
>> same source means more testing, IMO.
>
> That's complete bunk, though.  This helper binary is a limited use,
> one shot test that can't be combined with anything else.  "sharing"
> the source code across multiple different test suites doesn't change
> that. However, putting support into xfs_io means it will get more
> testing because we can *easily* use the functionality in many, many
> different ways.

It's useful to have a simple self-test in the kernel tree, so
that kernel developers (and their buildbots) can catch any
obvious regressions that might arise when working in related
areas, without requiring any other dependencies.

(Personally, I also think it's good that new kernel APIs
come with demonstrations of their use in userspace in a
nearby commit.)

OTOH, testing openat() behaviour should clearly be a part of
the xfstests suite (as a general/comprehensive filesystem
test repository), and Dave makes a good point that putting
the openat() functionality into xfs_io allows for more flexible
test combinations.

So I guess ideally we should end up with:
 a) Reinstate the kernel-tree selftests from the earlier iteration.
 b) Add openat test functionality into xfs_io
 c) Migrate the new xfstests cases to use xfs_io rather than
   a local openat utility program

However, given that I've already moved the test code once,
and that a)-c) involve three different source trees, I'd like
to de-couple things:
- Concentrate on kernel changes and a) on the LKML.
 - If & when the kernel changes make progress, return to
   b) and c).

Sound sensible?

Thanks,
David.

> Cheers,
>
> Dave.
> --
> Dave Chinner
> david@fromorbit.com