From mboxrd@z Thu Jan 1 00:00:00 1970 From: Al Viro Subject: Re: /proc/pid/fd/ shows strange mode when executed via sudo. Date: Fri, 18 May 2012 19:45:26 +0100 Message-ID: <20120518184526.GS22082@ZenIV.linux.org.uk> References: <201205022240.GBB13566.VOtHFOLFOMSJFQ@I-love.SAKURA.ne.jp> <20120503154222.GA8776@sergelap> <201205040125.FEI34871.QHOFFJOFMVtOSL@I-love.SAKURA.ne.jp> <201205181139.IAI65153.OQOtFVJFHLSMOF@I-love.SAKURA.ne.jp> <201205181827.BAF57803.OLQMFVtFFOHSJO@I-love.SAKURA.ne.jp> <20120518180812.GR22082@ZenIV.linux.org.uk> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Tetsuo Handa , ebiederm@xmission.com, linux-fsdevel@vger.kernel.org, akpm@linux-foundation.org To: Linus Torvalds Return-path: Received: from zeniv.linux.org.uk ([195.92.253.2]:40334 "EHLO ZenIV.linux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754237Ab2ERSpb (ORCPT ); Fri, 18 May 2012 14:45:31 -0400 Content-Disposition: inline In-Reply-To: Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Fri, May 18, 2012 at 11:23:27AM -0700, Linus Torvalds wrote: > On Fri, May 18, 2012 at 11:18 AM, Linus Torvalds > wrote: > > > > Doing it at getattr() time does sound good. > > > > Let me try to cook something up. > > Ugh. It's a much bigger patch, because we share the inode operations > with other cases too. > > So I think that would fall under the "further cleanup" heading, and > I'm not going to do it now. Not with 3.4 imminent. > > But if you were to do such a cleanup later... It's actually not big. Completely untested minimal variant (without touching ->d_revalidate()): diff --git a/fs/proc/base.c b/fs/proc/base.c index 1c8b280..b2a8b8f 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 */ @@ -1851,25 +1889,16 @@ static struct dentry *proc_fd_instantiate(struct inode *dir, 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); + rcu_read_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); + rcu_read_unlock(); put_files_struct(files); + if (!file) + goto out_iput; - inode->i_op = &proc_pid_link_inode_operations; - inode->i_size = 64; + inode->i_mode = S_IFLNK; + 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); @@ -1879,9 +1908,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;