lkml.org 
[lkml]   [2006]   [Apr]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Subject[PATCH] de_thread: Don't confuse users do_each_thread.
    From
    Date

    I can't seem to send out a correct patch out today with out
    sending it twice. I accidently grabbed my old version
    that sent to many arguments to detach_pid and so would not
    compile. Oops.

    ---

    Oleg Nesterov spotted two interesting bugs with the current de_thread
    code. The simplest is a long standing double decrement of
    __get_cpu_var(process_counts) in __unhash_process. Caused by
    two processes exiting when only one was created.

    The other is that since we no longer detach from the thread_group list
    it is possible for do_each_thread when run under the tasklist_lock to
    see the same task_struct twice. Once on the task list as a
    thread_group_leader, and once on the thread list of another
    thread.

    The double appearance in do_each_thread can cause a double increment
    of mm_core_waiters in zap_threads resulting in problems later on in
    coredump_wait.

    To remedy those two problems this patch takes the simple approach
    of changing the old thread group leader into a child thread.
    The only routine in release_task that cares is __unhash_process,
    and it can be trivially seen that we handle cleaning up a
    thread group leader properly.

    Since de_thread doesn't change the pid of the exiting leader process
    and instead shares it with the new leader process. I change
    thread_group_leader to recognize group leadership based on the
    group_leader field and not based on pids. This should also be
    slightly cheaper then the existing thread_group_leader macro.

    I performed a quick audit and I couldn't see any user of
    thread_group_leader that cared how cared about the difference.

    I believe this is 2.6.17 material as the bug is present in
    2.6.17-rc1 and the fix is simple.

    Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>


    ---

    fs/exec.c | 7 ++++++-
    include/linux/sched.h | 3 ++-
    2 files changed, 8 insertions(+), 2 deletions(-)

    e621f800b1de6684beb995014190b3ccaad5c0b3
    diff --git a/include/linux/sched.h b/include/linux/sched.h
    index 541f482..a3e4f6b 100644
    --- a/include/linux/sched.h
    +++ b/include/linux/sched.h
    @@ -1203,7 +1203,8 @@ #define do_each_thread(g, t) \
    #define while_each_thread(g, t) \
    while ((t = next_thread(t)) != g)

    -#define thread_group_leader(p) (p->pid == p->tgid)
    +/* de_thread depends on thread_group_leader not being a pid based check */
    +#define thread_group_leader(p) (p == p->group_leader)

    static inline task_t *next_thread(task_t *p)
    {
    diff --git a/fs/exec.c b/fs/exec.c
    index 0291a68..4d38ad0 100644
    --- a/fs/exec.c
    +++ b/fs/exec.c
    @@ -723,7 +723,12 @@ static int de_thread(struct task_struct
    current->parent = current->real_parent = leader->real_parent;
    leader->parent = leader->real_parent = child_reaper;
    current->group_leader = current;
    - leader->group_leader = leader;
    + leader->group_leader = current;
    +
    + /* Reduce leader to a thread */
    + detach_pid(leader, PIDTYPE_PGID);
    + detach_pid(leader, PIDTYPE_SID);
    + list_del_init(&leader->tasks);

    add_parent(current);
    add_parent(leader);
    --
    1.3.0.rc3.g6b3a
    -
    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: 2006-04-11 01:20    [W:0.030 / U:91.228 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site