diff options
author | Eric W. Biederman <ebiederm@xmission.com> | 2006-04-14 06:05:55 -0400 |
---|---|---|
committer | Linus Torvalds <torvalds@g5.osdl.org> | 2006-04-14 11:49:19 -0400 |
commit | c06511d12d720b23c8dffff23004f0a888698f20 (patch) | |
tree | 96916946cd8374821b10316c8f5fabc4917a38d6 | |
parent | 0e5e24bf64b755a034d8294303bc61d8f40ebeaf (diff) |
[PATCH] de_thread: Don't change our parents and ptrace flags.
This is two distinct changes.
- Not changing our real parents.
- Not changing our ptrace parents.
Not changing our real parents is trivially correct because both tasks
have the same real parents as they are part of a thread group. Now that
we demote the leader to a thread there is no longer any reason to change
it's parentage.
Not changing our ptrace parents is a user visible change if someone
looks hard enough. I don't think user space applications will care or
even notice.
In the practical and I think common case a debugger will have attached
to all of the threads using the same ptrace flags. From my quick skim
of strace and gdb that appears to be the case. Which if true means
debuggers will not notice a change.
Before this point we have already generated a ptrace event in do_exit
that reports the leaders pid has died so de_thread is visible to a
debugger. Which means attempting to hide this case by copying flags
around appears excessive.
By not doing anything it avoids all of the weird locking issues between
de_thread and ptrace attach, and removes one case from consideration for
fixing the ptrace locking.
This only addresses Oleg's first concern with ptrace_attach, that of the
problems caused by reparenting. Oleg's second concern is essentially a
race between ptrace_attach and release_task that causes an oops when we
get to force_sig_specific. There is nothing special about de_thread
with respect to that race.
Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>
-rw-r--r-- | fs/exec.c | 27 |
1 files changed, 0 insertions, 27 deletions
@@ -665,9 +665,7 @@ static int de_thread(struct task_struct *tsk) | |||
665 | * and to assume its PID: | 665 | * and to assume its PID: |
666 | */ | 666 | */ |
667 | if (!thread_group_leader(current)) { | 667 | if (!thread_group_leader(current)) { |
668 | struct task_struct *parent; | ||
669 | struct dentry *proc_dentry1, *proc_dentry2; | 668 | struct dentry *proc_dentry1, *proc_dentry2; |
670 | unsigned long ptrace; | ||
671 | 669 | ||
672 | /* | 670 | /* |
673 | * Wait for the thread group leader to be a zombie. | 671 | * Wait for the thread group leader to be a zombie. |
@@ -704,22 +702,6 @@ static int de_thread(struct task_struct *tsk) | |||
704 | * two threads with a switched PID, and release | 702 | * two threads with a switched PID, and release |
705 | * the former thread group leader: | 703 | * the former thread group leader: |
706 | */ | 704 | */ |
707 | ptrace = leader->ptrace; | ||
708 | parent = leader->parent; | ||
709 | if (unlikely(ptrace) && unlikely(parent == current)) { | ||
710 | /* | ||
711 | * Joker was ptracing his own group leader, | ||
712 | * and now he wants to be his own parent! | ||
713 | * We can't have that. | ||
714 | */ | ||
715 | ptrace = 0; | ||
716 | } | ||
717 | |||
718 | ptrace_unlink(current); | ||
719 | ptrace_unlink(leader); | ||
720 | remove_parent(current); | ||
721 | remove_parent(leader); | ||
722 | |||
723 | 705 | ||
724 | /* Become a process group leader with the old leader's pid. | 706 | /* Become a process group leader with the old leader's pid. |
725 | * Note: The old leader also uses thispid until release_task | 707 | * Note: The old leader also uses thispid until release_task |
@@ -732,8 +714,6 @@ static int de_thread(struct task_struct *tsk) | |||
732 | attach_pid(current, PIDTYPE_SID, current->signal->session); | 714 | attach_pid(current, PIDTYPE_SID, current->signal->session); |
733 | list_add_tail(¤t->tasks, &init_task.tasks); | 715 | list_add_tail(¤t->tasks, &init_task.tasks); |
734 | 716 | ||
735 | current->parent = current->real_parent = leader->real_parent; | ||
736 | leader->parent = leader->real_parent = child_reaper; | ||
737 | current->group_leader = current; | 717 | current->group_leader = current; |
738 | leader->group_leader = current; | 718 | leader->group_leader = current; |
739 | 719 | ||
@@ -742,13 +722,6 @@ static int de_thread(struct task_struct *tsk) | |||
742 | detach_pid(leader, PIDTYPE_SID); | 722 | detach_pid(leader, PIDTYPE_SID); |
743 | list_del_init(&leader->tasks); | 723 | list_del_init(&leader->tasks); |
744 | 724 | ||
745 | add_parent(current); | ||
746 | add_parent(leader); | ||
747 | if (ptrace) { | ||
748 | current->ptrace = ptrace; | ||
749 | __ptrace_link(current, parent); | ||
750 | } | ||
751 | |||
752 | current->exit_signal = SIGCHLD; | 725 | current->exit_signal = SIGCHLD; |
753 | 726 | ||
754 | BUG_ON(leader->exit_state != EXIT_ZOMBIE); | 727 | BUG_ON(leader->exit_state != EXIT_ZOMBIE); |