| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20170319172414.GT29622@ZenIV.linux.org.uk>
Date: Sun, 19 Mar 2017 17:24:15 +0000
From: Al Viro <viro@...IV.linux.org.uk>
To: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org
Subject: [RFC] AT_NO_JUMPS/LOOKUP_NO_JUMPS
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@...iv.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@...iv.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 */
Powered by blists - more mailing lists