linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Al Viro <viro@ZenIV.linux.org.uk>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org
Subject: [RFC] AT_NO_JUMPS/LOOKUP_NO_JUMPS
Date: Sun, 19 Mar 2017 17:24:15 +0000	[thread overview]
Message-ID: <20170319172414.GT29622@ZenIV.linux.org.uk> (raw)

	Bringing back an old conversation - what do you think about the
potential usefulness of the following ...at() option:
	* no mountpoint crossings allowed (mount --bind included)
	* only relative symlinks traversals are allowed
	* starting point acts as a chroot boundary (IOW, .. does not lead
out of it)

IIRC, you mentioned that something of that kind might be interesting for
git et.al. and it turns out to be surprisingly easy to implement -
 fs/namei.c                 | 31 +++++++++++++++++++++++++++----
 fs/stat.c                  |  4 +++-
 include/linux/namei.h      |  1 +
 include/uapi/linux/fcntl.h |  1 +
 4 files changed, 32 insertions(+), 5 deletions(-)
for implementation of LOOKUP_NO_JUMPS and its hookup for fstatat() and
friends.  And AFAICS it doesn't mess the fast paths either.  I've dropped
it into vfs.git#experimental-AT_NO_JUMPS; only built-tested at the moment
(and in any case, it would need manpage patches, xfstests and LTP ones,
etc.)

The points I'm not sure about:
	1) what would be a good error value (went with -ELOOP at the moment)
	2) handling of .. is quiet in that sucker; trying to walk out of
subtree does not fail with -ELOOP (as absolute symlinks or mountpoint
crossing would have), it simply stays at the root of subtree, same as it
would for chroot jail.  Wouldn't be too hard to make it fail (a couple of
if (...) break; in follow_dotdot,{_rcu}() turned into if (unlikely(...)) {
	if (unlikely(nd->flags & LOOKUP_NO_JUMPS)) return -ELOOP;
	break;
}); not sure if it's worth bothering with.
	3) absolute pathname with LOOKUP_NO_JUMPS => -ELOOP at the moment.
Wouldn't be hard to change, if somebody proposes a sane semantics.

Any comments would be welcome; is that a sane behaviour to implement, what 
changes (if any) would be needed to make it useful, etc.

Current variant of patch follows:

commit 8020273120c30b1444f67f7bbee2f20cee314d02
Author: Al Viro <viro@zeniv.linux.org.uk>
Date:   Sat Mar 18 16:27:55 2017 -0400

    namei: new flag (LOOKUP_NO_JUMPS)
    
    semantics:
            * no mountpoint crossing allowed
            * only relative symlinks allowed
            * starting point acts as chroot boundary (i.e.
    .. does not lead out of it)
    
    Matching AT_... flag: AT_NO_JUMPS introduced, fstatat(2) (and
    corresponding statx/stat64 variants) taught about it.
    
    Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

diff --git a/fs/namei.c b/fs/namei.c
index d41fab78798b..57264a82878b 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1054,10 +1054,15 @@ const char *get_link(struct nameidata *nd)
 		} else {
 			res = get(dentry, inode, &last->done);
 		}
+		if (unlikely(nd->flags & LOOKUP_NO_JUMPS) &&
+		    unlikely(nd->flags & LOOKUP_JUMPED))
+			return ERR_PTR(-ELOOP);
 		if (IS_ERR_OR_NULL(res))
 			return res;
 	}
 	if (*res == '/') {
+		if (unlikely(nd->flags & LOOKUP_NO_JUMPS))
+			return ERR_PTR(-ELOOP);
 		if (!nd->root.mnt)
 			set_root(nd);
 		if (unlikely(nd_jump_root(nd)))
@@ -1245,12 +1250,16 @@ static int follow_managed(struct path *path, struct nameidata *nd)
 		break;
 	}
 
-	if (need_mntput && path->mnt == mnt)
-		mntput(path->mnt);
+	if (need_mntput) {
+		if (path->mnt == mnt)
+			mntput(path->mnt);
+		if (unlikely(nd->flags & LOOKUP_NO_JUMPS))
+			ret = -ELOOP;
+		else
+			nd->flags |= LOOKUP_JUMPED;
+	}
 	if (ret == -EISDIR || !ret)
 		ret = 1;
-	if (need_mntput)
-		nd->flags |= LOOKUP_JUMPED;
 	if (unlikely(ret < 0))
 		path_put_conditional(path, nd);
 	return ret;
@@ -1307,6 +1316,8 @@ static bool __follow_mount_rcu(struct nameidata *nd, struct path *path,
 		mounted = __lookup_mnt(path->mnt, path->dentry);
 		if (!mounted)
 			break;
+		if (unlikely(nd->flags & LOOKUP_NO_JUMPS))
+			return false;
 		path->mnt = &mounted->mnt;
 		path->dentry = mounted->mnt.mnt_root;
 		nd->flags |= LOOKUP_JUMPED;
@@ -2177,6 +2188,8 @@ static const char *path_init(struct nameidata *nd, unsigned flags)
 
 	nd->m_seq = read_seqbegin(&mount_lock);
 	if (*s == '/') {
+		if (unlikely(flags & LOOKUP_NO_JUMPS))
+			return ERR_PTR(-ELOOP);
 		if (flags & LOOKUP_RCU)
 			rcu_read_lock();
 		set_root(nd);
@@ -2202,6 +2215,11 @@ static const char *path_init(struct nameidata *nd, unsigned flags)
 			get_fs_pwd(current->fs, &nd->path);
 			nd->inode = nd->path.dentry->d_inode;
 		}
+		if (unlikely(flags & LOOKUP_NO_JUMPS)) {
+			nd->root = nd->path;
+			if (!(flags & LOOKUP_RCU))
+				path_get(&nd->root);
+		}
 		return s;
 	} else {
 		/* Caller must check execute permissions on the starting path component */
@@ -2229,6 +2247,11 @@ static const char *path_init(struct nameidata *nd, unsigned flags)
 			path_get(&nd->path);
 			nd->inode = nd->path.dentry->d_inode;
 		}
+		if (unlikely(flags & LOOKUP_NO_JUMPS)) {
+			nd->root = nd->path;
+			if (!(flags & LOOKUP_RCU))
+				path_get(&nd->root);
+		}
 		fdput(f);
 		return s;
 	}
diff --git a/fs/stat.c b/fs/stat.c
index fa0be59340cc..1999ce5f77c9 100644
--- a/fs/stat.c
+++ b/fs/stat.c
@@ -168,7 +168,7 @@ int vfs_statx(int dfd, const char __user *filename, int flags,
 	unsigned int lookup_flags = LOOKUP_FOLLOW | LOOKUP_AUTOMOUNT;
 
 	if ((flags & ~(AT_SYMLINK_NOFOLLOW | AT_NO_AUTOMOUNT |
-		       AT_EMPTY_PATH | KSTAT_QUERY_FLAGS)) != 0)
+		       AT_EMPTY_PATH | KSTAT_QUERY_FLAGS | AT_NO_JUMPS)) != 0)
 		return -EINVAL;
 
 	if (flags & AT_SYMLINK_NOFOLLOW)
@@ -177,6 +177,8 @@ int vfs_statx(int dfd, const char __user *filename, int flags,
 		lookup_flags &= ~LOOKUP_AUTOMOUNT;
 	if (flags & AT_EMPTY_PATH)
 		lookup_flags |= LOOKUP_EMPTY;
+	if (flags & AT_NO_JUMPS)
+		lookup_flags |= LOOKUP_NO_JUMPS;
 
 retry:
 	error = user_path_at(dfd, filename, lookup_flags, &path);
diff --git a/include/linux/namei.h b/include/linux/namei.h
index f29abda31e6d..593a998da846 100644
--- a/include/linux/namei.h
+++ b/include/linux/namei.h
@@ -44,6 +44,7 @@ enum {LAST_NORM, LAST_ROOT, LAST_DOT, LAST_DOTDOT, LAST_BIND};
 #define LOOKUP_JUMPED		0x1000
 #define LOOKUP_ROOT		0x2000
 #define LOOKUP_EMPTY		0x4000
+#define LOOKUP_NO_JUMPS		0x8000
 
 extern int path_pts(struct path *path);
 
diff --git a/include/uapi/linux/fcntl.h b/include/uapi/linux/fcntl.h
index 813afd6eee71..ca35ef523e40 100644
--- a/include/uapi/linux/fcntl.h
+++ b/include/uapi/linux/fcntl.h
@@ -68,5 +68,6 @@
 #define AT_STATX_FORCE_SYNC	0x2000	/* - Force the attributes to be sync'd with the server */
 #define AT_STATX_DONT_SYNC	0x4000	/* - Don't sync attributes with the server */
 
+#define AT_NO_JUMPS		0x8000	/* No mountpoint crossing, no abs symlinks */
 
 #endif /* _UAPI_LINUX_FCNTL_H */

             reply	other threads:[~2017-03-19 17:24 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-19 17:24 Al Viro [this message]
2017-03-20  1:46 ` [RFC] AT_NO_JUMPS/LOOKUP_NO_JUMPS Linus Torvalds
2017-03-20 14:22   ` Omar Sandoval
2017-05-02 19:57 ` Pavel Machek
2017-05-02 20:49   ` Al Viro

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170319172414.GT29622@ZenIV.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).