From: Oleg Nesterov <oleg@redhat.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: David Rientjes <rientjes@google.com>,
"Eric W. Biederman" <ebiederm@xmission.com>,
KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>,
Michal Hocko <mhocko@suse.cz>, Sergey Dyasly <dserrg@gmail.com>,
Sha Zhengju <handai.szj@taobao.com>,
linux-kernel@vger.kernel.org
Subject: [PATCH 3/3] proc: simplify proc_task_readdir/first_tid paths
Date: Mon, 27 May 2013 22:28:22 +0200 [thread overview]
Message-ID: <20130527202822.GA19288@redhat.com> (raw)
In-Reply-To: <20130527202751.GA19250@redhat.com>
proc_task_readdir() does not really need "leader", first_tid()
has to revalidate it anyway. Just pass proc_pid(inode) to
first_tid() instead, it can do pid_task(PIDTYPE_PID) itself
and read ->group_leader only if necessary.
Note: I am not sure proc_task_readdir() really needs the initial
-ENOENT check, but this is what the current code does.
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
fs/proc/base.c | 46 ++++++++++++++++------------------------------
1 files changed, 16 insertions(+), 30 deletions(-)
diff --git a/fs/proc/base.c b/fs/proc/base.c
index bed1096..dbc4dae 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -3173,34 +3173,35 @@ out_no_task:
* In the case of a seek we start with the leader and walk nr
* threads past it.
*/
-static struct task_struct *first_tid(struct task_struct *leader,
- int tid, int nr, struct pid_namespace *ns)
+static struct task_struct *first_tid(struct pid *pid, int tid,
+ int nr, struct pid_namespace *ns)
{
- struct task_struct *pos;
+ struct task_struct *pos, *task;
rcu_read_lock();
- /* Attempt to start with the pid of a thread */
+ task = pid_task(pid, PIDTYPE_PID);
+ if (!task)
+ goto fail;
+
+ /* Attempt to start with the tid of a thread */
if (tid && (nr > 0)) {
pos = find_task_by_pid_ns(tid, ns);
- if (pos && (pos->group_leader == leader))
+ if (pos && same_thread_group(pos, task))
goto found;
}
/* If nr exceeds the number of threads there is nothing todo */
- if (nr && nr >= get_nr_threads(leader))
- goto fail;
- /* It could be unhashed before we take rcu lock */
- if (!pid_alive(leader))
+ if (nr && nr >= get_nr_threads(task))
goto fail;
/* If we haven't found our starting place yet start
* with the leader and walk nr threads forward.
*/
- pos = leader;
+ pos = task = task->group_leader;
do {
if (nr-- <= 0)
goto found;
- } while_each_thread(leader, pos);
+ } while_each_thread(task, pos);
fail:
pos = NULL;
goto out;
@@ -3247,26 +3248,13 @@ static int proc_task_readdir(struct file * filp, void * dirent, filldir_t filldi
{
struct dentry *dentry = filp->f_path.dentry;
struct inode *inode = dentry->d_inode;
- struct task_struct *leader = NULL;
struct task_struct *task;
- int retval = -ENOENT;
ino_t ino;
int tid;
struct pid_namespace *ns;
- task = get_proc_task(inode);
- if (!task)
- goto out_no_task;
- rcu_read_lock();
- if (pid_alive(task)) {
- leader = task->group_leader;
- get_task_struct(leader);
- }
- rcu_read_unlock();
- put_task_struct(task);
- if (!leader)
- goto out_no_task;
- retval = 0;
+ if (!pid_task(proc_pid(inode), PIDTYPE_PID))
+ return -ENOENT;
switch ((unsigned long)filp->f_pos) {
case 0:
@@ -3289,7 +3277,7 @@ static int proc_task_readdir(struct file * filp, void * dirent, filldir_t filldi
ns = filp->f_dentry->d_sb->s_fs_info;
tid = (int)filp->f_version;
filp->f_version = 0;
- for (task = first_tid(leader, tid, filp->f_pos - 2, ns);
+ for (task = first_tid(proc_pid(inode), tid, filp->f_pos - 2, ns);
task;
task = next_tid(task), filp->f_pos++) {
tid = task_pid_nr_ns(task, ns);
@@ -3302,9 +3290,7 @@ static int proc_task_readdir(struct file * filp, void * dirent, filldir_t filldi
}
}
out:
- put_task_struct(leader);
-out_no_task:
- return retval;
+ return 0;
}
static int proc_task_getattr(struct vfsmount *mnt, struct dentry *dentry, struct kstat *stat)
--
1.5.5.1
next prev parent reply other threads:[~2013-05-27 20:32 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-05-27 20:27 [PATCH 0/3] proc: first_tid() fix/cleanup Oleg Nesterov
2013-05-27 20:28 ` [PATCH 1/3] proc: first_tid: fix the potential use-after-free Oleg Nesterov
2013-05-29 4:08 ` Eric W. Biederman
2013-05-29 12:30 ` Oleg Nesterov
2013-05-27 20:28 ` [PATCH 2/3] proc: change first_tid() to use while_each_thread() Oleg Nesterov
2013-05-27 20:28 ` Oleg Nesterov [this message]
2013-05-29 4:42 ` [PATCH 3/3] proc: simplify proc_task_readdir/first_tid paths Eric W. Biederman
2013-05-29 13:39 ` Oleg Nesterov
2013-05-29 20:38 ` Eric W. Biederman
2013-05-31 16:38 ` Oleg Nesterov
2013-05-31 18:12 ` Eric W. Biederman
2013-05-31 18:34 ` Oleg Nesterov
2013-05-29 5:22 ` [PATCH 0/3] proc: first_tid() fix/cleanup Eric W. Biederman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20130527202822.GA19288@redhat.com \
--to=oleg@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=dserrg@gmail.com \
--cc=ebiederm@xmission.com \
--cc=handai.szj@taobao.com \
--cc=kamezawa.hiroyu@jp.fujitsu.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mhocko@suse.cz \
--cc=rientjes@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.