Commit 19fd4bb2 authored by Eric W. Biederman's avatar Eric W. Biederman Committed by Linus Torvalds

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: default avatar"Eric W. Biederman" <ebiederm@xmission.com>
Cc: Oleg Nesterov <oleg@tv-sign.ru>
Cc: Alexey Dobriyan <adobriyan@gmail.com>
Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
parent c8950783
...@@ -2411,19 +2411,23 @@ struct dentry *proc_pid_lookup(struct inode *dir, struct dentry * dentry, struct ...@@ -2411,19 +2411,23 @@ struct dentry *proc_pid_lookup(struct inode *dir, struct dentry * dentry, struct
* Find the first task with tgid >= tgid * Find the first task with tgid >= tgid
* *
*/ */
static struct task_struct *next_tgid(unsigned int tgid, struct tgid_iter {
struct pid_namespace *ns) unsigned int tgid;
{
struct task_struct *task; struct task_struct *task;
};
static struct tgid_iter next_tgid(struct pid_namespace *ns, struct tgid_iter iter)
{
struct pid *pid; struct pid *pid;
if (iter.task)
put_task_struct(iter.task);
rcu_read_lock(); rcu_read_lock();
retry: retry:
task = NULL; iter.task = NULL;
pid = find_ge_pid(tgid, ns); pid = find_ge_pid(iter.tgid, ns);
if (pid) { if (pid) {
tgid = pid_nr_ns(pid, ns) + 1; iter.tgid = pid_nr_ns(pid, ns);
task = pid_task(pid, PIDTYPE_PID); iter.task = pid_task(pid, PIDTYPE_PID);
/* What we to know is if the pid we have find is the /* What we to know is if the pid we have find is the
* pid of a thread_group_leader. Testing for task * pid of a thread_group_leader. Testing for task
* being a thread_group_leader is the obvious thing * being a thread_group_leader is the obvious thing
...@@ -2436,23 +2440,25 @@ static struct task_struct *next_tgid(unsigned int tgid, ...@@ -2436,23 +2440,25 @@ static struct task_struct *next_tgid(unsigned int tgid,
* found doesn't happen to be a thread group leader. * found doesn't happen to be a thread group leader.
* As we don't care in the case of readdir. * 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; goto retry;
get_task_struct(task); }
get_task_struct(iter.task);
} }
rcu_read_unlock(); rcu_read_unlock();
return task; return iter;
} }
#define TGID_OFFSET (FIRST_PROCESS_ENTRY + ARRAY_SIZE(proc_base_stuff)) #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, 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]; 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, 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 */ /* 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) ...@@ -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; 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 *reaper = get_proc_task(filp->f_path.dentry->d_inode);
struct task_struct *task; struct tgid_iter iter;
int tgid;
struct pid_namespace *ns; struct pid_namespace *ns;
if (!reaper) if (!reaper)
...@@ -2474,14 +2479,14 @@ int proc_pid_readdir(struct file * filp, void * dirent, filldir_t filldir) ...@@ -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; ns = filp->f_dentry->d_sb->s_fs_info;
tgid = filp->f_pos - TGID_OFFSET; iter.task = NULL;
for (task = next_tgid(tgid, ns); iter.tgid = filp->f_pos - TGID_OFFSET;
task; for (iter = next_tgid(ns, iter);
put_task_struct(task), task = next_tgid(tgid + 1, ns)) { iter.task;
tgid = task_pid_nr_ns(task, ns); iter.tgid += 1, iter = next_tgid(ns, iter)) {
filp->f_pos = tgid + TGID_OFFSET; filp->f_pos = iter.tgid + TGID_OFFSET;
if (proc_pid_fill_cache(filp, dirent, filldir, task, tgid) < 0) { if (proc_pid_fill_cache(filp, dirent, filldir, iter) < 0) {
put_task_struct(task); put_task_struct(iter.task);
goto out; goto out;
} }
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment