lkml.org 
[lkml]   [2007]   [Nov]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH] proc: Remove races from proc_id_readdir()
    Date

    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>

    ---
    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 5b92b79..7e8eca4 100644
    --- a/fs/proc/base.c
    +++ b/fs/proc/base.c
    @@ -2454,19 +2454,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
    @@ -2479,23 +2483,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 */
    @@ -2503,8 +2509,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)
    @@ -2517,14 +2522,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;
    }
    }
    --
    1.5.3.rc6.17.g1911
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2007-11-19 22:49    [W:0.024 / U:32.176 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site