Attachment 561090 Details for Bug 728774

[patch] [PATCH] vfs: allow /proc/pid/fd to dup a socket

vfs-allow-proc-pid-fd-to-dup-a-socket (text/plain), 5.94 KB, created by Jeff Mahoney on 2013-10-01 18:22:46 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jeff Mahoney

Created: 2013-10-01 18:22:46 UTC

Size: 5.94 KB

patch

obsolete

>From: Jeff Mahoney <jeffm@suse.com>
>Subject: vfs: allow /proc/pid/fd to dup a socket
>
>When a process has one of the stdio file descriptors set to one end
>of a socket, it is prohibited from using /dev/std{in,out,err}. 
>
>For example, a script that does:
>echo "some error" > /dev/stderr
>
>... will get ENXIO returned. The fact that this works for nearly every
>other type of file except for sockets has been a source of confusion
>among users and developers for some time. Other UNIX-like systems don't
>make the distinction between socket and other files in their /dev/fd/
>implementation, but this is largely due to using dup() rather than
>handling it as symlink-open.
>
>For a variety of reasons, the socket code depends on there being
>a 1:1 mapping between a struct file and struct socket. It's possible
>to rework the reference counting and export a few more socket
>functions to allow a many:1 mapping, but we use the flags associated
>with the file to determine if e.g. the socket is nonblocking. Having
>multiple files with different semantics operating on the same file
>would yield unexpected results.
>
>As a result, the option that's left is to dup() the existing file
>descriptor so that we preserve the 1:1 mapping. This is a special
>case that is limited to sockets opened by the current process. The
>symlink-open behavior has become a de-facto ABI and the dup
>behavior can't be used universally as it would surprise users
>expecting a /proc/pid/fd file open to return at offset 0 instead
>of the f_pos of that file descriptor.
>
>This patch adds the ability for a function in the lookup path to
>set a new LOOKUP_DUP_FD flag and stash the file descriptor number
>in the nameidata to provide the descriptor to dup after the open
>fails.
>
>It's hacky, but short of reworking the entire open path to accomodate
>this special case, it's the cleanest solution. The FreeBSD kernel
>reserves an int in their equivalent of task_struct for this, which is
>even worse.
>
>Signed-off-by: Jeff Mahoney <jeffm@suse.com>
>---
> fs/namei.c            |    5 +++++
> fs/proc/base.c        |    4 ++--
> fs/proc/fd.c          |   35 ++++++++++++++++++++++++++++++++++-
> fs/proc/internal.h    |    3 +++
> include/linux/namei.h |    6 ++++++
> 5 files changed, 50 insertions(+), 3 deletions(-)
>
>--- a/fs/namei.c	2013-09-30 21:24:50.298957907 -0400
>+++ b/fs/namei.c	2013-10-01 10:24:36.172619101 -0400
>@@ -3294,6 +3294,11 @@ out:
> 				error = -ECHILD;
> 			else
> 				error = -ESTALE;
>+		} else if (nd->flags & LOOKUP_DUP_FD) {
>+			file = fget_raw(nd->dup_fd);
>+			if (S_ISSOCK(file->f_inode->i_mode))
>+				return file;
>+			fput(file);
> 		}
> 		file = ERR_PTR(error);
> 	}
>--- a/fs/proc/base.c	2013-09-23 18:41:09.000000000 -0400
>+++ b/fs/proc/base.c	2013-10-01 10:24:10.837004578 -0400
>@@ -1454,7 +1454,7 @@ static int proc_exe_link(struct dentry *
> 		return -ENOENT;
> }
> 
>-static void *proc_pid_follow_link(struct dentry *dentry, struct nameidata *nd)
>+void *proc_pid_follow_link(struct dentry *dentry, struct nameidata *nd)
> {
> 	struct inode *inode = dentry->d_inode;
> 	struct path path;
>@@ -1498,7 +1498,7 @@ static int do_proc_readlink(struct path
> 	return len;
> }
> 
>-static int proc_pid_readlink(struct dentry * dentry, char __user * buffer, int buflen)
>+int proc_pid_readlink(struct dentry * dentry, char __user * buffer, int buflen)
> {
> 	int error = -EACCES;
> 	struct inode *inode = dentry->d_inode;
>--- a/fs/proc/fd.c	2013-09-23 18:41:09.000000000 -0400
>+++ b/fs/proc/fd.c	2013-10-01 13:25:59.594881564 -0400
>@@ -167,6 +167,39 @@ static int proc_fd_link(struct dentry *d
> 	return ret;
> }
> 
>+void *proc_pid_fd_follow_link(struct dentry *dentry, struct nameidata *nd)
>+{
>+	void *v;
>+
>+	v = proc_pid_follow_link(dentry, nd);
>+	if (v)
>+		return v;
>+
>+	/*
>+	 * Sockets can't be opened directly, which breaks
>+	 * /dev/stderr and friends. We stuff the fd in the nameidata
>+	 * here so that the open path can dup the fd after the open
>+	 * fails.
>+	 */
>+	if (S_ISSOCK(nd->inode->i_mode)) {
>+		struct task_struct *task = get_proc_task(dentry->d_inode);
>+		if (task == current) {
>+			nd->flags |= LOOKUP_DUP_FD;
>+			nd->dup_fd = proc_fd(dentry->d_inode);
>+		}
>+		put_task_struct(task);
>+	}
>+
>+
>+	return NULL;
>+}
>+
>+static const struct inode_operations proc_pid_fd_link_inode_operations = {
>+	.readlink	= proc_pid_readlink,
>+	.follow_link	= proc_pid_fd_follow_link,
>+	.setattr	= proc_setattr,
>+};
>+
> static int
> proc_fd_instantiate(struct inode *dir, struct dentry *dentry,
> 		    struct task_struct *task, const void *ptr)
>@@ -183,7 +216,7 @@ proc_fd_instantiate(struct inode *dir, s
> 	ei->fd = fd;
> 
> 	inode->i_mode = S_IFLNK;
>-	inode->i_op = &proc_pid_link_inode_operations;
>+	inode->i_op = &proc_pid_fd_link_inode_operations;
> 	inode->i_size = 64;
> 
> 	ei->op.proc_get_link = proc_fd_link;
>--- a/fs/proc/internal.h	2013-09-23 18:41:09.000000000 -0400
>+++ b/fs/proc/internal.h	2013-10-01 10:25:26.267857151 -0400
>@@ -168,6 +168,9 @@ extern int pid_delete_dentry(const struc
> extern int proc_pid_readdir(struct file *, struct dir_context *);
> extern struct dentry *proc_pid_lookup(struct inode *, struct dentry *, unsigned int);
> extern loff_t mem_lseek(struct file *, loff_t, int);
>+extern int proc_pid_readlink(struct dentry * dentry, char __user * buffer,
>+			     int buflen);
>+extern void *proc_pid_follow_link(struct dentry *dentry, struct nameidata *nd);
> 
> /* Lookups */
> typedef int instantiate_t(struct inode *, struct dentry *,
>--- a/include/linux/namei.h	2013-09-23 18:41:09.000000000 -0400
>+++ b/include/linux/namei.h	2013-10-01 10:20:17.396558152 -0400
>@@ -19,6 +19,7 @@ struct nameidata {
> 	unsigned	seq;
> 	int		last_type;
> 	unsigned	depth;
>+	int		dup_fd;
> 	char *saved_names[MAX_NESTED_LINKS + 1];
> };
> 
>@@ -56,6 +57,11 @@ enum {LAST_NORM, LAST_ROOT, LAST_DOT, LA
> #define LOOKUP_ROOT		0x2000
> #define LOOKUP_EMPTY		0x4000
> 
>+/*
>+ * Caller should dup the fd in dup_fd rather than open a new struct file
>+ */
>+#define LOOKUP_DUP_FD		0x8000
>+
> extern int user_path_at(int, const char __user *, unsigned, struct path *);
> extern int user_path_at_empty(int, const char __user *, unsigned, struct path *, int *empty);
>

Actions: View | Diff

Attachments on bug 728774: 492773 | 492774 | 492776 | 561090