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

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

On Fri, May 18, 2012 at 02:32:58PM -0700, Linus Torvalds wrote:
> On Fri, May 18, 2012 at 2:26 PM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> >
> > However, wouldn't it be better to move the check you added to the
> > readdir() path, then, so that we don't do it unnecessarily for
> > lookups..
> >
> > I'll check how that looks.
> 
> Actually, we already do that. It's the fcheck_files() check in
> proc_readfd_common(), no?

Missed that.  Yes, you are right - there's no point keeping that
in proc_fd_instantiate().  OK, messing with fcheck_files() in there
dropped now...

diff --git a/fs/proc/base.c b/fs/proc/base.c
index 1c8b280..3986db1 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,26 @@ 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 inode *inode;
  	struct proc_inode *ei;
-	struct dentry *error = 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,