Re: /proc/pid/fd/ shows strange mode when executed via sudo.

[Al Viro <viro@ZenIV.linux.org.uk>]

On Fri, May 18, 2012 at 11:55:21AM -0700, Linus Torvalds wrote:
> On Fri, May 18, 2012 at 11:45 AM, Al Viro <viro@zeniv.linux.org.uk> wrote:
> >
> > It's actually not big. ?Completely untested minimal variant (without
> > touching ->d_revalidate()):
> 
> Get rid of all the now unnecessary files stuff and error handling in
> proc_fd_instantiate() too (it's unnecessary - it only ends up being
> re-done in the revalidate function anyway), and I'll take it.

Umm...  I'd rather do it other way - do that test *before* bothering
with allocating an inode.  IOW, use it as early cutoff, with
tid_fd_revalidate() called in the end closing any races about
file getting closed while we'd been allocating an inode, etc.

IOW, how about this?  Note that it's _still_ completely untested wrt
weird LSM stuff.  I simply don't have tomoyo/apparmor/whatnot setups
to test on, so I'd rather see an ACK from the LSM people.

diff --git a/fs/proc/base.c b/fs/proc/base.c
index 1c8b280..e663a04 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -1491,6 +1491,44 @@ static const struct inode_operations proc_pid_link_inode_operations = {
 	.setattr	= proc_setattr,
 };
 
+static int proc_fd_getattr(struct vfsmount *mnt, struct dentry *dentry, struct kstat *stat)
+{
+	struct inode *inode = dentry->d_inode;
+	struct files_struct *files;
+	struct task_struct *task;
+	struct file *file;
+	fmode_t mode = 0;
+
+	generic_fillattr(inode, stat);
+
+	rcu_read_lock();
+	task = pid_task(proc_pid(inode), PIDTYPE_PID);
+	files = task ? get_files_struct(task) : NULL;
+	file = files ? fcheck_files(files, PROC_I(inode)->fd) : NULL;
+	if (file)
+		mode = file->f_mode;
+	rcu_read_unlock();
+
+	if (files)
+		put_files_struct(files);
+	if (!file)
+		return -ENOENT;
+
+	if (mode & FMODE_READ)
+		stat->mode |= S_IRUSR | S_IXUSR;
+	if (mode & FMODE_WRITE)
+		stat->mode |= S_IWUSR | S_IXUSR;
+	stat->size = 64;
+	return 0;
+}
+
+static const struct inode_operations proc_pid_fd_inode_operations = {
+	.readlink	= proc_pid_readlink,
+	.follow_link	= proc_pid_follow_link,
+	.setattr	= proc_setattr,
+	.getattr	= proc_fd_getattr,
+};
+
 
 /* building an inode */
 
@@ -1837,54 +1875,38 @@ static struct dentry *proc_fd_instantiate(struct inode *dir,
 	struct dentry *dentry, struct task_struct *task, const void *ptr)
 {
 	unsigned fd = *(const unsigned *)ptr;
-	struct file *file;
-	struct files_struct *files;
+	struct file *file = NULL;
+	struct files_struct *files = get_files_struct(task);
  	struct inode *inode;
  	struct proc_inode *ei;
-	struct dentry *error = ERR_PTR(-ENOENT);
+
+	if (files) {
+		rcu_read_lock();
+		file = fcheck_files(files, fd);
+		rcu_read_unlock();
+		put_files_struct(files);
+	}
+
+	if (!file)
+		return ERR_PTR(-ENOENT);
 
 	inode = proc_pid_make_inode(dir->i_sb, task);
 	if (!inode)
-		goto out;
+		return ERR_PTR(-ENOMEM);
 	ei = PROC_I(inode);
 	ei->fd = fd;
-	files = get_files_struct(task);
-	if (!files)
-		goto out_iput;
 	inode->i_mode = S_IFLNK;
-
-	/*
-	 * We are not taking a ref to the file structure, so we must
-	 * hold ->file_lock.
-	 */
-	spin_lock(&files->file_lock);
-	file = fcheck_files(files, fd);
-	if (!file)
-		goto out_unlock;
-	if (file->f_mode & FMODE_READ)
-		inode->i_mode |= S_IRUSR | S_IXUSR;
-	if (file->f_mode & FMODE_WRITE)
-		inode->i_mode |= S_IWUSR | S_IXUSR;
-	spin_unlock(&files->file_lock);
-	put_files_struct(files);
-
-	inode->i_op = &proc_pid_link_inode_operations;
-	inode->i_size = 64;
+	inode->i_op = &proc_pid_fd_inode_operations;
 	ei->op.proc_get_link = proc_fd_link;
 	d_set_d_op(dentry, &tid_fd_dentry_operations);
 	d_add(dentry, inode);
-	/* Close the race of the process dying before we return the dentry */
-	if (tid_fd_revalidate(dentry, NULL))
-		error = NULL;
-
- out:
-	return error;
-out_unlock:
-	spin_unlock(&files->file_lock);
-	put_files_struct(files);
-out_iput:
-	iput(inode);
-	goto out;
+	/*
+	 * Close the race of the process dying or file getting closed
+	 * before we return the dentry
+	 */
+	if (!tid_fd_revalidate(dentry, NULL))
+		return ERR_PTR(-ENOENT);
+	return NULL;
 }
 
 static struct dentry *proc_lookupfd_common(struct inode *dir,