[RFC 2/2] fs,proc: Respect FMODE_WRITE when opening /proc/pid/fd/N

[Andy Lutomirski]

This patch does this:

    # (echo asdf >/proc/self/fd/3) 3</dev/null
    bash: /proc/self/fd/3: Permission denied

I'm not confident that my use of nd->flags is correct.  I'm also not
confident that this hooks in to path lookup in the right place.

It is not complete.  It doesn't handle FMODE_READ.  It also doesn't
handle FMODE_EXEC or FMODE_PATH, but the desired semantics there are
less clear.  For example, this should probably continue to work:

    (/proc/self/fd/3 Hello) 3</bin/echo

This patch does not do anything to /proc/self/exe.  It probably should.

Something like this can possibly fix the accidental flink-via-proc
thing, too.

This may break userspace.  If so, I would guess that anything broken
by it is either an actual exploit or is so broken that it doesn't
deserve to continue working.  If it breaks something important, then
maybe it will need a sysctl.

Signed-off-by: Andy Lutomirski <luto@xxxxxxxxxxxxxx>
---
 fs/namei.c            |  3 +++
 fs/proc/fd.c          | 19 ++++++++++++++++---
 include/linux/namei.h |  1 +
 3 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index c6157c8..3f01836 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -2894,6 +2894,9 @@ static int do_last(struct nameidata *nd, struct path *path,
 		goto finish_open;
 	}
 
+	if (open_flag & (O_WRONLY | O_RDWR))
+		nd->flags |= LOOKUP_WRITE;
+
 	if (!(open_flag & O_CREAT)) {
 		if (nd->last.name[nd->last.len])
 			nd->flags |= LOOKUP_FOLLOW | LOOKUP_DIRECTORY;
diff --git a/fs/proc/fd.c b/fs/proc/fd.c
index deca407..ab3373c 100644
--- a/fs/proc/fd.c
+++ b/fs/proc/fd.c
@@ -139,6 +139,17 @@ static const struct dentry_operations tid_fd_dentry_operations = {
 	.d_delete	= pid_delete_dentry,
 };
 
+static int proc_may_follow(struct nameidata *nd, struct file *f)
+{
+	if (!nd)
+		return 0;  /* This is readlink, */
+
+	if ((nd->flags & LOOKUP_WRITE) && !(f->f_mode & FMODE_WRITE))
+		return -EACCES;
+
+	return 0;
+}
+
 static int proc_fd_link(struct nameidata *nd,
 			struct dentry *dentry, struct path *path)
 {
@@ -159,9 +170,11 @@ static int proc_fd_link(struct nameidata *nd,
 		spin_lock(&files->file_lock);
 		fd_file = fcheck_files(files, fd);
 		if (fd_file) {
-			*path = fd_file->f_path;
-			path_get(&fd_file->f_path);
-			ret = 0;
+			ret = proc_may_follow(nd, fd_file);
+			if (ret == 0) {
+				*path = fd_file->f_path;
+				path_get(&fd_file->f_path);
+			}
 		}
 		spin_unlock(&files->file_lock);
 		put_files_struct(files);
diff --git a/include/linux/namei.h b/include/linux/namei.h
index 492de72..5077201 100644
--- a/include/linux/namei.h
+++ b/include/linux/namei.h
@@ -51,6 +51,7 @@ enum {LAST_NORM, LAST_ROOT, LAST_DOT, LAST_DOTDOT, LAST_BIND};
 #define LOOKUP_CREATE		0x0200
 #define LOOKUP_EXCL		0x0400
 #define LOOKUP_RENAME_TARGET	0x0800
+#define LOOKUP_WRITE		0x8000
 
 #define LOOKUP_JUMPED		0x1000
 #define LOOKUP_ROOT		0x2000
-- 
1.9.0

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/