Skip to content

Commit

Permalink
proc: move fd symlink i_mode calculations into tid_fd_revalidate()
Browse files Browse the repository at this point in the history
Instead of doing the i_mode calculations at proc_fd_instantiate() time,
move them into tid_fd_revalidate(), which is where the other inode state
(notably uid/gid information) is updated too.

Otherwise we'll end up with stale i_mode information if an fd is re-used
while the dentry still hangs around.  Not that anything really *cares*
(symlink permissions don't really matter), but Tetsuo Handa noticed that
the owner read/write bits don't always match the state of the
readability of the file descriptor, and we _used_ to get this right a
long time ago in a galaxy far, far away.

Besides, aside from fixing an ugly detail (that has apparently been this
way since commit 61a2878: "proc: Remove the hard coded inode
numbers" in 2006), this removes more lines of code than it adds.  And it
just makes sense to update i_mode in the same place we update i_uid/gid.

Al Viro correctly points out that we could just do the inode fill in the
inode iops ->getattr() function instead.  However, that does require
somewhat slightly more invasive changes, and adds yet *another* lookup
of the file descriptor.  We need to do the revalidate() for other
reasons anyway, and have the file descriptor handy, so we might as well
fill in the information at this point.

Reported-by: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Acked-by: Eric Biederman <ebiederm@xmission.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
  • Loading branch information
Linus Torvalds committed May 18, 2012
1 parent 3d99449 commit 30a08bf
Showing 1 changed file with 14 additions and 29 deletions.
43 changes: 14 additions & 29 deletions fs/proc/base.c
Original file line number Diff line number Diff line change
Expand Up @@ -1799,10 +1799,15 @@ static int tid_fd_revalidate(struct dentry *dentry, struct nameidata *nd)
if (task) {
files = get_files_struct(task);
if (files) {
struct file *file;
rcu_read_lock();
if (fcheck_files(files, fd)) {
file = fcheck_files(files, fd);
if (file) {
unsigned i_mode, f_mode = file->f_mode;

rcu_read_unlock();
put_files_struct(files);

if (task_dumpable(task)) {
rcu_read_lock();
cred = __task_cred(task);
Expand All @@ -1813,7 +1818,14 @@ static int tid_fd_revalidate(struct dentry *dentry, struct nameidata *nd)
inode->i_uid = 0;
inode->i_gid = 0;
}
inode->i_mode &= ~(S_ISUID | S_ISGID);

i_mode = S_IFLNK;
if (f_mode & FMODE_READ)
i_mode |= S_IRUSR | S_IXUSR;
if (f_mode & FMODE_WRITE)
i_mode |= S_IWUSR | S_IXUSR;
inode->i_mode = i_mode;

security_task_to_inode(task, inode);
put_task_struct(task);
return 1;
Expand All @@ -1837,8 +1849,6 @@ 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);
Expand All @@ -1848,25 +1858,6 @@ static struct dentry *proc_fd_instantiate(struct inode *dir,
goto out;
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;
Expand All @@ -1879,12 +1870,6 @@ static struct dentry *proc_fd_instantiate(struct inode *dir,

out:
return error;
out_unlock:
spin_unlock(&files->file_lock);
put_files_struct(files);
out_iput:
iput(inode);
goto out;
}

static struct dentry *proc_lookupfd_common(struct inode *dir,
Expand Down

0 comments on commit 30a08bf

Please sign in to comment.