diff options
author | Eric W. Biederman <ebiederm@xmission.com> | 2007-11-28 16:21:26 -0800 |
---|---|---|
committer | Linus Torvalds <torvalds@woody.linux-foundation.org> | 2007-11-29 09:24:52 -0800 |
commit | 19fd4bb2a0cfede054e4904e0b167e0ca4f36cc7 (patch) | |
tree | e37d0ee2c7ea08f6d8721a4e9e807f85994b608a | |
parent | c895078355b6b6e05c60aa205892526dd3390f0a (diff) |
proc: remove races from proc_id_readdir()
Oleg noticed that the call of task_pid_nr_ns() in proc_pid_readdir
is racy with respect to tasks exiting.
After a bit of examination it also appears that the call itself
is completely unnecessary.
So to fix the problem this patch modifies next_tgid() to return
both a tgid and the task struct in question.
A structure is introduced to return these values because it is
slightly cleaner and easier to optimize, and the resulting code
is a little shorter.
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: Oleg Nesterov <oleg@tv-sign.ru>
Cc: Alexey Dobriyan <adobriyan@gmail.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
-rw-r--r-- | fs/proc/base.c | 51 |
1 files changed, 28 insertions, 23 deletions
diff --git a/fs/proc/base.c b/fs/proc/base.c index a17c26859074..02a63ac04178 100644 --- a/fs/proc/base.c +++ b/fs/proc/base.c @@ -2411,19 +2411,23 @@ out: * Find the first task with tgid >= tgid * */ -static struct task_struct *next_tgid(unsigned int tgid, - struct pid_namespace *ns) -{ +struct tgid_iter { + unsigned int tgid; struct task_struct *task; +}; +static struct tgid_iter next_tgid(struct pid_namespace *ns, struct tgid_iter iter) +{ struct pid *pid; + if (iter.task) + put_task_struct(iter.task); rcu_read_lock(); retry: - task = NULL; - pid = find_ge_pid(tgid, ns); + iter.task = NULL; + pid = find_ge_pid(iter.tgid, ns); if (pid) { - tgid = pid_nr_ns(pid, ns) + 1; - task = pid_task(pid, PIDTYPE_PID); + iter.tgid = pid_nr_ns(pid, ns); + iter.task = pid_task(pid, PIDTYPE_PID); /* What we to know is if the pid we have find is the * pid of a thread_group_leader. Testing for task * being a thread_group_leader is the obvious thing @@ -2436,23 +2440,25 @@ retry: * found doesn't happen to be a thread group leader. * As we don't care in the case of readdir. */ - if (!task || !has_group_leader_pid(task)) + if (!iter.task || !has_group_leader_pid(iter.task)) { + iter.tgid += 1; goto retry; - get_task_struct(task); + } + get_task_struct(iter.task); } rcu_read_unlock(); - return task; + return iter; } #define TGID_OFFSET (FIRST_PROCESS_ENTRY + ARRAY_SIZE(proc_base_stuff)) static int proc_pid_fill_cache(struct file *filp, void *dirent, filldir_t filldir, - struct task_struct *task, int tgid) + struct tgid_iter iter) { char name[PROC_NUMBUF]; - int len = snprintf(name, sizeof(name), "%d", tgid); + int len = snprintf(name, sizeof(name), "%d", iter.tgid); return proc_fill_cache(filp, dirent, filldir, name, len, - proc_pid_instantiate, task, NULL); + proc_pid_instantiate, iter.task, NULL); } /* for the /proc/ directory itself, after non-process stuff has been done */ @@ -2460,8 +2466,7 @@ int proc_pid_readdir(struct file * filp, void * dirent, filldir_t filldir) { unsigned int nr = filp->f_pos - FIRST_PROCESS_ENTRY; struct task_struct *reaper = get_proc_task(filp->f_path.dentry->d_inode); - struct task_struct *task; - int tgid; + struct tgid_iter iter; struct pid_namespace *ns; if (!reaper) @@ -2474,14 +2479,14 @@ int proc_pid_readdir(struct file * filp, void * dirent, filldir_t filldir) } ns = filp->f_dentry->d_sb->s_fs_info; - tgid = filp->f_pos - TGID_OFFSET; - for (task = next_tgid(tgid, ns); - task; - put_task_struct(task), task = next_tgid(tgid + 1, ns)) { - tgid = task_pid_nr_ns(task, ns); - filp->f_pos = tgid + TGID_OFFSET; - if (proc_pid_fill_cache(filp, dirent, filldir, task, tgid) < 0) { - put_task_struct(task); + iter.task = NULL; + iter.tgid = filp->f_pos - TGID_OFFSET; + for (iter = next_tgid(ns, iter); + iter.task; + iter.tgid += 1, iter = next_tgid(ns, iter)) { + filp->f_pos = iter.tgid + TGID_OFFSET; + if (proc_pid_fill_cache(filp, dirent, filldir, iter) < 0) { + put_task_struct(iter.task); goto out; } } |