diff options
author | Eric W. Biederman <ebiederm@xmission.com> | 2006-04-10 19:16:49 -0400 |
---|---|---|
committer | Linus Torvalds <torvalds@g5.osdl.org> | 2006-04-10 19:36:50 -0400 |
commit | de12a7878c11f3b282d640888aa635e0711d0b5e (patch) | |
tree | 742b72a47cc36a7e591dba1883cd9af3c44290c4 | |
parent | 1b72373491a061be6d456d219a4e2d054ac2aaad (diff) |
[PATCH] de_thread: Don't confuse users do_each_thread.
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 about the difference.
Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>
-rw-r--r-- | fs/exec.c | 7 | ||||
-rw-r--r-- | include/linux/sched.h | 3 |
2 files changed, 8 insertions, 2 deletions
@@ -723,7 +723,12 @@ static int de_thread(struct task_struct *tsk) | |||
723 | current->parent = current->real_parent = leader->real_parent; | 723 | current->parent = current->real_parent = leader->real_parent; |
724 | leader->parent = leader->real_parent = child_reaper; | 724 | leader->parent = leader->real_parent = child_reaper; |
725 | current->group_leader = current; | 725 | current->group_leader = current; |
726 | leader->group_leader = leader; | 726 | leader->group_leader = current; |
727 | |||
728 | /* Reduce leader to a thread */ | ||
729 | detach_pid(leader, PIDTYPE_PGID); | ||
730 | detach_pid(leader, PIDTYPE_SID); | ||
731 | list_del_init(&leader->tasks); | ||
727 | 732 | ||
728 | add_parent(current); | 733 | add_parent(current); |
729 | add_parent(leader); | 734 | add_parent(leader); |
diff --git a/include/linux/sched.h b/include/linux/sched.h index 541f4828f5e7..a3e4f6b503a3 100644 --- a/include/linux/sched.h +++ b/include/linux/sched.h | |||
@@ -1203,7 +1203,8 @@ extern void wait_task_inactive(task_t * p); | |||
1203 | #define while_each_thread(g, t) \ | 1203 | #define while_each_thread(g, t) \ |
1204 | while ((t = next_thread(t)) != g) | 1204 | while ((t = next_thread(t)) != g) |
1205 | 1205 | ||
1206 | #define thread_group_leader(p) (p->pid == p->tgid) | 1206 | /* de_thread depends on thread_group_leader not being a pid based check */ |
1207 | #define thread_group_leader(p) (p == p->group_leader) | ||
1207 | 1208 | ||
1208 | static inline task_t *next_thread(task_t *p) | 1209 | static inline task_t *next_thread(task_t *p) |
1209 | { | 1210 | { |