Message-Id: <1459819769-30387-1-git-send-email-ebiederm@xmission.com>
Date:	Mon,  4 Apr 2016 20:29:17 -0500
From:	"Eric W. Biederman" <ebiederm@...ssion.com>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	"H. Peter Anvin" <hpa@...or.com>,
	Peter Hurley <peter@...leysoftware.com>,
	Greg KH <greg@...ah.com>, Jiri Slaby <jslaby@...e.com>,
	Aurelien Jarno <aurelien@...el32.net>,
	Andy Lutomirski <luto@...capital.net>,
	Florian Weimer <fw@...eb.enyo.de>,
	Al Viro <viro@...iv.linux.org.uk>,
	Serge Hallyn <serge.hallyn@...ntu.com>,
	Jann Horn <jann@...jh.net>,
	"security@...nel.org" <security@...nel.org>, security@...ntu.com,
	security@...ian.org, Willy Tarreau <w@....eu>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	"Eric W. Biederman" <ebiederm@...ssion.com>
Subject: [PATCH 01/13] devpts: Teach /dev/ptmx to find the associated devpts via path lookup

This is in preparation for forcing each mount of devpts to be a
distinct filesystem.  The goal of this change is to have as few user
visible changes from the kernel today as possible.

On each open of /dev/ptmx look at the relative path pts and see if
devpts is mounted there.

If the filesystem found via the path lookup is the system devpts
do exactly what we do today.

If the filesystem found is not the system devpts make it appear
to the rest of the system that the /dev/pts/ptmx node was opened.
This includes respecting the permission checks of /dev/pts/ptmx
and updating the file's path to point to /dev/pts/ptmx.

In practice I expect the permission checks are a non-issue as the
permissions on /dev/ptmx and /dev/pts/ptmx are always 0666.  If
someone happens to change the permission on /dev/pts/ptmx is seems
important to honor them for the sake of backwards compatibility.  As
/dev/ptmx can be bind mounted to set next to any devpts filesystem not
honoring the permissions would provide a nice permission bypass under
the right circumstances if we did not check the permissions.

Similarly reflect the instance of devpts that was opened in f_path so
that we preserve the only way available today to test if someone is
attempting to confuse a pty creating program.

This winds up using 3 new vfs helpers path_parent, path_pts, and
update_file_path.

Signed-off-by: "Eric W. Biederman" <ebiederm@...ssion.com>
---
 drivers/tty/pty.c         |  4 +++
 fs/devpts/inode.c         | 81 +++++++++++++++++++++++++++++++++++++++++++++++
 fs/namei.c                | 64 ++++++++++++++++++++++++++++++-------
 fs/open.c                 | 19 +++++++++++
 include/linux/devpts_fs.h |  2 ++
 include/linux/fs.h        |  2 ++
 include/linux/namei.h     |  3 ++
 7 files changed, 164 insertions(+), 11 deletions(-)

diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c
index e16a49b507ef..557858ef00f5 100644
--- a/drivers/tty/pty.c
+++ b/drivers/tty/pty.c
@@ -725,6 +725,10 @@ static int ptmx_open(struct inode *inode, struct file *filp)
 	int retval;
 	int index;
 
+	inode = devpts_ptmx(inode, filp);
+	if (IS_ERR(inode))
+		return PTR_ERR(inode);
+
 	nonseekable_open(inode, filp);
 
 	/* We refuse fsnotify events on ptmx, since it's a shared resource */
diff --git a/fs/devpts/inode.c b/fs/devpts/inode.c
index 655f21f99160..c14d51795577 100644
--- a/fs/devpts/inode.c
+++ b/fs/devpts/inode.c
@@ -17,6 +17,7 @@
 #include <linux/fs.h>
 #include <linux/sched.h>
 #include <linux/namei.h>
+#include <linux/fs_struct.h>
 #include <linux/slab.h>
 #include <linux/mount.h>
 #include <linux/tty.h>
@@ -136,6 +137,86 @@ static inline struct pts_fs_info *DEVPTS_SB(struct super_block *sb)
 	return sb->s_fs_info;
 }
 
+#ifdef CONFIG_DEVPTS_MULTIPLE_INSTANCES
+static int devpts_path_ptmx(struct file *filp)
+{
+	struct pts_fs_info *fsi;
+	struct path root, path;
+	struct dentry *old;
+	int err = -ENOENT;
+	int ret;
+
+	/* Can the pts filesystem be found with a path walk? */
+	path = filp->f_path;
+	path_get(&path);
+	get_fs_root(current->fs, &root);
+	ret = path_parent(&root, &path);
+	path_put(&root);
+	if (ret != 1)
+		goto fail;
+
+	/* Remember the result of this permission check for later */
+	ret = inode_permission(path.dentry->d_inode, MAY_EXEC);
+	if (path_pts(&path))
+		goto fail;
+
+	/* Is path the root of a devpts filesystem? */
+	if ((path.mnt->mnt_sb->s_magic != DEVPTS_SUPER_MAGIC) ||
+	    (path.mnt->mnt_root != path.mnt->mnt_sb->s_root))
+		goto fail;
+	fsi = DEVPTS_SB(path.mnt->mnt_sb);
+
+	/* Get out if the path walk resulted in the default devpts instance */
+	if (devpts_mnt->mnt_sb == path.mnt->mnt_sb)
+		goto fail;
+
+	/* Don't allow bypassing the existing /dev/pts/ptmx permission check */
+	err = ret;
+	if (!err)
+		err = inode_permission(path.dentry->d_inode, MAY_EXEC);
+	if (!err)
+		err = inode_permission(fsi->ptmx_dentry->d_inode,
+				       ACC_MODE(filp->f_flags));
+	if (err)
+		goto fail;
+
+	/* Advance path to the ptmx dentry */
+	old = path.dentry;
+	path.dentry = dget(fsi->ptmx_dentry);
+	dput(old);
+
+	/* Make it look like /dev/pts/ptmx was opened */
+	err = update_file_path(filp, &path);
+	if (err)
+		goto fail;
+
+	return 0;
+fail:
+	path_put(&path);
+	return err;
+}
+#else
+static inline int devpts_path_ptmx(struct file *filp)
+{
+	return -ENOENT;
+}
+#endif
+
+struct inode *devpts_ptmx(struct inode *inode, struct file *filp)
+{
+	int err;
+	if (inode->i_sb->s_magic == DEVPTS_SUPER_MAGIC)
+		return inode;
+
+	err = devpts_path_ptmx(filp);
+	if (err == 0)
+		return filp->f_inode;
+	if (err != -ENOENT)
+		return ERR_PTR(err);
+
+	return inode;
+}
+
 static inline struct super_block *pts_sb_from_inode(struct inode *inode)
 {
 #ifdef CONFIG_DEVPTS_MULTIPLE_INSTANCES
diff --git a/fs/namei.c b/fs/namei.c
index 794f81dce766..afb5137ca199 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1415,29 +1415,41 @@ static void follow_mount(struct path *path)
 	}
 }
 
-static int follow_dotdot(struct nameidata *nd)
+int path_parent(struct path *root, struct path *path)
 {
+	int ret = 0;
+
 	while(1) {
-		struct dentry *old = nd->path.dentry;
+		struct dentry *old = path->dentry;
 
-		if (nd->path.dentry == nd->root.dentry &&
-		    nd->path.mnt == nd->root.mnt) {
+		if (old == root->dentry &&
+		    path->mnt == root->mnt) {
 			break;
 		}
-		if (nd->path.dentry != nd->path.mnt->mnt_root) {
+		if (old != path->mnt->mnt_root) {
 			/* rare case of legitimate dget_parent()... */
-			nd->path.dentry = dget_parent(nd->path.dentry);
+			path->dentry = dget_parent(path->dentry);
 			dput(old);
-			if (unlikely(!path_connected(&nd->path)))
+			if (unlikely(!path_connected(path)))
 				return -ENOENT;
+			ret = 1;
 			break;
 		}
-		if (!follow_up(&nd->path))
+		if (!follow_up(path))
 			break;
 	}
-	follow_mount(&nd->path);
-	nd->inode = nd->path.dentry->d_inode;
-	return 0;
+	follow_mount(path);
+	return ret;
+}
+
+static int follow_dotdot(struct nameidata *nd)
+{
+	int ret = path_parent(&nd->root, &nd->path);
+	if (ret >= 0) {
+		ret = 0;
+		nd->inode = nd->path.dentry->d_inode;
+	}
+	return ret;
 }
 
 /*
@@ -2374,6 +2386,36 @@ struct dentry *lookup_one_len_unlocked(const char *name,
 }
 EXPORT_SYMBOL(lookup_one_len_unlocked);
 
+#ifdef CONFIG_UNIX98_PTYS
+int path_pts(struct path *path)
+{
+	struct dentry *child, *parent = path->dentry;
+	struct qstr this;
+
+	if (!d_can_lookup(parent))
+		return -ENOENT;
+
+	this.name = "pts";
+	this.len = 3;
+	this.hash = full_name_hash(this.name, this.len);
+	if (parent->d_flags & DCACHE_OP_HASH) {
+		int err = parent->d_op->d_hash(parent, &this);
+		if (err < 0)
+			return err;
+	}
+	inode_lock(parent->d_inode);
+	child = d_lookup(parent, &this);
+	inode_unlock(parent->d_inode);
+	if (!child)
+		return -ENOENT;
+
+	path->dentry = child;
+	dput(parent);
+	follow_mount(path);
+	return 0;
+}
+#endif
+
 int user_path_at_empty(int dfd, const char __user *name, unsigned flags,
 		 struct path *path, int *empty)
 {
diff --git a/fs/open.c b/fs/open.c
index 17cb6b1dab75..e1ed78fa474b 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -679,6 +679,24 @@ int open_check_o_direct(struct file *f)
 	return 0;
 }
 
+int update_file_path(struct file *filp, struct path *path)
+{
+	/* Only valid during f_op->open, and even in open use very carefully */
+	struct path old;
+	struct inode *inode;
+
+	if (filp->f_mode & FMODE_WRITER)
+		return -EINVAL;
+
+	old = filp->f_path;
+	inode = path->dentry->d_inode;
+	filp->f_path = *path;
+	filp->f_inode = inode;
+	filp->f_mapping = inode->i_mapping;
+	path_put(&old);
+	return 0;
+}
+
 static int do_dentry_open(struct file *f,
 			  struct inode *inode,
 			  int (*open)(struct inode *, struct file *),
@@ -736,6 +754,7 @@ static int do_dentry_open(struct file *f,
 		error = open(inode, f);
 		if (error)
 			goto cleanup_all;
+		inode = f->f_inode;
 	}
 	if ((f->f_mode & (FMODE_READ | FMODE_WRITE)) == FMODE_READ)
 		i_readcount_inc(inode);
diff --git a/include/linux/devpts_fs.h b/include/linux/devpts_fs.h
index e0ee0b3000b2..260190690674 100644
--- a/include/linux/devpts_fs.h
+++ b/include/linux/devpts_fs.h
@@ -17,6 +17,8 @@
 
 #ifdef CONFIG_UNIX98_PTYS
 
+struct inode *devpts_ptmx(struct inode *inode, struct file *filp);
+
 int devpts_new_index(struct inode *ptmx_inode);
 void devpts_kill_index(struct inode *ptmx_inode, int idx);
 void devpts_add_ref(struct inode *ptmx_inode);
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 14a97194b34b..045bbfe2ecfc 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2272,6 +2272,8 @@ extern struct filename *getname(const char __user *);
 extern struct filename *getname_kernel(const char *);
 extern void putname(struct filename *name);
 
+extern int update_file_path(struct file *filp, struct path *path);
+
 enum {
 	FILE_CREATED = 1,
 	FILE_OPENED = 2
diff --git a/include/linux/namei.h b/include/linux/namei.h
index 77d01700daf7..647d97a4869b 100644
--- a/include/linux/namei.h
+++ b/include/linux/namei.h
@@ -45,6 +45,9 @@ enum {LAST_NORM, LAST_ROOT, LAST_DOT, LAST_DOTDOT, LAST_BIND};
 #define LOOKUP_ROOT		0x2000
 #define LOOKUP_EMPTY		0x4000
 
+extern int path_parent(struct path *root, struct path *path);
+extern int path_pts(struct path *path);
+
 extern int user_path_at_empty(int, const char __user *, unsigned, struct path *, int *empty);
 
 static inline int user_path_at(int dfd, const char __user *name, unsigned flags,
-- 
2.6.3

Powered by blists - more mailing lists