Re: [PATCH] cleanup ptrace stops and remove notify

public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed

* Re: [PATCH] cleanup ptrace stops and remove notify_parent
       [not found] <2z7vs-2F1-13@gated-at.bofh.it>
@ 2004-08-31 10:00 ` Andi Kleen
  2004-09-01  0:27   ` Roland McGrath
  0 siblings, 1 reply; 25+ messages in thread
From: Andi Kleen @ 2004-08-31 10:00 UTC (permalink / raw)
  To: Roland McGrath; +Cc: linux-kernel, torvalds, akpm

Roland McGrath <roland@redhat.com> writes:

> This patch is against Linus's current tree.
>
> This adds a new state TASK_TRACED that is used in place of TASK_STOPPED
> when a thread stops because it is ptraced.  Now ptrace operations are only
> permitted when the target is in TASK_TRACED state, not in TASK_STOPPED.
> This means that if a process is stopped normally by a job control signal
> and then you PTRACE_ATTACH to it, you will have to send it a SIGCONT before
> you can do any ptrace operations on it.  (The SIGCONT will be reported to
> ptrace and then you can discard it instead of passing it through when you
> call PTRACE_CONT et al.)

Are you sure such a user visible semantic change is a good idea? 

I at least have written (not very important, but existing) user space
code in the past that assumed it can stop and single step without
SIGCONT. I wouldn't be surprised if other debuggers ran into 
the same issue.

And the Linux debugging world is not gdb only anymore, there are
a lot of other users of ptrace around these days ...

I don't think it is very good to change such behaviour in 2.6.
Please don't do it.

-Andi

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] cleanup ptrace stops and remove notify_parent
  2004-08-31 10:00 ` [PATCH] cleanup ptrace stops and remove notify_parent Andi Kleen
@ 2004-09-01  0:27   ` Roland McGrath
  2004-09-01  1:56     ` Roland McGrath
  0 siblings, 1 reply; 25+ messages in thread
From: Roland McGrath @ 2004-09-01  0:27 UTC (permalink / raw)
  To: Andi Kleen; +Cc: linux-kernel, torvalds, akpm

> Are you sure such a user visible semantic change is a good idea? 

Not if it's not too hard to avoid.  I think I can make it not come up by
converting TASK_STOPPED into TASK_TRACED when you use ptrace.  It will just
take some synchronization for that case.

> I at least have written (not very important, but existing) user space
> code in the past that assumed it can stop and single step without
> SIGCONT. I wouldn't be surprised if other debuggers ran into 
> the same issue.

I assume you mean stop with SIGSTOP, then attach with PTRACE_ATTACH, then
step with PTRACE_SINGLESTEP.  If you are already attached, then any stop
will report to you first in TASK_TRACED and never end up TASK_STOPPED
unless you explicitly pass a stop signal through with ptrace.

Thanks,
Roland

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] cleanup ptrace stops and remove notify_parent
  2004-09-01  0:27   ` Roland McGrath
@ 2004-09-01  1:56     ` Roland McGrath
  2004-09-04 13:38       ` Andi Kleen
  0 siblings, 1 reply; 25+ messages in thread
From: Roland McGrath @ 2004-09-01  1:56 UTC (permalink / raw)
  To: Andi Kleen, linux-kernel, torvalds, akpm

I don't have a program like yours to test that it was broken with my patch
(now in 2.6.9-rc1-mm2).  But the following additional patch relative to
2.6.9-rc1-mm2 should do it.  I've tested that it doesn't create any new
problems.  I don't have something right handy that tests the case in
question, but you've said that you do.

This makes any ptrace operation that finds the target in TASK_STOPPED state
morph it into TASK_TRACED state before doing anything.  This necessitates
reverting the last_siginfo accesses to check instead of assume last_siginfo
is set, since it's no longer impossible to be in TASK_TRACED without being
stopped in ptrace_stop (though there are no associated races to worry
about).


Thanks,
Roland

--- linux-2.6.9-rc1-mm2/kernel/ptrace.c.~1~	2004-08-31 18:18:10.000000000 -0700
+++ linux-2.6.9-rc1-mm2/kernel/ptrace.c	2004-08-31 18:51:46.922556896 -0700
@@ -81,13 +81,20 @@ int ptrace_check_attach(struct task_stru
 	 * be changed by us so it's not changing right after this.
 	 */
 	read_lock(&tasklist_lock);
-	if ((child->ptrace & PT_PTRACED) && child->parent == current)
+	if ((child->ptrace & PT_PTRACED) && child->parent == current &&
+	    child->signal != NULL) {
 		ret = 0;
+		spin_lock_irq(&child->sighand->siglock);
+		if (child->state == TASK_STOPPED) {
+			child->state = TASK_TRACED;
+		} else if (child->state != TASK_TRACED && !kill) {
+			ret = -ESRCH;
+		}
+		spin_unlock_irq(&child->sighand->siglock);
+	}
 	read_unlock(&tasklist_lock);
 
 	if (!ret && !kill) {
-		if (child->state != TASK_TRACED)
-			return -ESRCH;
 		wait_task_inactive(child);
 	}
 
@@ -298,13 +305,15 @@ static int ptrace_setoptions(struct task
 
 static int ptrace_getsiginfo(struct task_struct *child, siginfo_t __user * data)
 {
-	BUG_ON(child->last_siginfo == NULL);
+	if (child->last_siginfo == NULL)
+		return -EINVAL;
 	return copy_siginfo_to_user(data, child->last_siginfo);
 }
 
 static int ptrace_setsiginfo(struct task_struct *child, siginfo_t __user * data)
 {
-	BUG_ON(child->last_siginfo == NULL);
+	if (child->last_siginfo == NULL)
+		return -EINVAL;
 	if (copy_from_user(child->last_siginfo, data, sizeof (siginfo_t)) != 0)
 		return -EFAULT;
 	return 0;

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] cleanup ptrace stops and remove notify_parent
  2004-09-01  1:56     ` Roland McGrath
@ 2004-09-04 13:38       ` Andi Kleen
  0 siblings, 0 replies; 25+ messages in thread
From: Andi Kleen @ 2004-09-04 13:38 UTC (permalink / raw)
  To: Roland McGrath; +Cc: linux-kernel, torvalds, akpm

On Tue, Aug 31, 2004 at 06:56:21PM -0700, Roland McGrath wrote:
> I don't have a program like yours to test that it was broken with my patch
> (now in 2.6.9-rc1-mm2).  But the following additional patch relative to
> 2.6.9-rc1-mm2 should do it.  I've tested that it doesn't create any new
> problems.  I don't have something right handy that tests the case in
> question, but you've said that you do.

I am travelling and can't reach the code right now, but I will
test later.

> 
> This makes any ptrace operation that finds the target in TASK_STOPPED state
> morph it into TASK_TRACED state before doing anything.  This necessitates
> reverting the last_siginfo accesses to check instead of assume last_siginfo
> is set, since it's no longer impossible to be in TASK_TRACED without being
> stopped in ptrace_stop (though there are no associated races to worry
> about).

Looks good, thanks.

-Andi

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] cleanup ptrace stops and remove notify_parent
@ 2004-08-31 15:59 Albert Cahalan
  2004-08-31 23:54 ` Roland McGrath
  0 siblings, 1 reply; 25+ messages in thread
From: Albert Cahalan @ 2004-08-31 15:59 UTC (permalink / raw)
  To: linux-kernel mailing list; +Cc: roland, Andrew Morton OSDL, Linus Torvalds

Roland McGrath writes:

> diff -rBzpu linux-2.6.9/fs/proc/array.c linux-2.6-ptracefix/fs/proc/array.c
> --- linux-2.6.9/fs/proc/array.c 2004-08-30 20:10:50.000000000 -0700
> +++ linux-2.6-ptracefix/fs/proc/array.c 2004-08-30 18:43:29.000000000 -0700
> @@ -130,8 +130,9 @@ static const char *task_state_array[] = 
>   "S (sleeping)",  /*  1 */
>   "D (disk sleep)", /*  2 */
>   "T (stopped)",  /*  4 */
> - "Z (zombie)",  /*  8 */
> - "X (dead)"  /* 16 */
> + "T (tracing stop)", /*  8 */
> + "Z (zombie)",  /* 16 */
> + "X (dead)"  /* 32 */
>  };

For TASK_DEAD and TASK_TRACED, might you be hiding some
underlying state? I want to display stuff like:

T    stopped (not traced)
TX   stopped, and being traced
RX   running or runnable, and being traced
ZE   zombie, and trying to exit
RXE  running, trying to exit, and traced

It's troubling the way TASK_DEAD and TASK_TRACED show up as
actual states and hide any other stuff. I'd much prefer them
to be flags of some sort.

> diff -rBzpu linux-2.6.9/kernel/power/process.c linux-2.6-ptracefix/kernel/power/process.c
> --- linux-2.6.9/kernel/power/process.c 2004-08-30 20:10:51.000000000 -0700
> +++ linux-2.6-ptracefix/kernel/power/process.c 2004-08-30 18:20:05.000000000 -0700
> @@ -25,7 +25,8 @@ static inline int freezeable(struct task
>       (p->flags & PF_NOFREEZE) ||
>       (p->state == TASK_ZOMBIE) ||
>       (p->state == TASK_DEAD) ||
> -     (p->state == TASK_STOPPED))
> +     (p->state == TASK_STOPPED) ||
> +     (p->state == TASK_TRACED))

(p->state & (TASK_ZOMBIE|TASK_DEAD|TASK_STOPPED|TASK_TRACED))

> @@ -70,6 +71,7 @@ int freeze_processes(void)
>     if (!freezeable(p))
>      continue;
>     if ((p->flags & PF_FROZEN) ||
> +       (p->state == TASK_TRACED) ||
>         (p->state == TASK_STOPPED))
>      continue;

(p->state & (TASK_TRACED|TASK_STOPPED))

> @@ -3670,7 +3670,7 @@ static void show_task(task_t * p)
>   task_t *relative;
>   unsigned state;
>   unsigned long free = 0;
> - static const char *stat_nam[] = { "R", "S", "D", "T", "Z", "W" };
> + static const char *stat_nam[] = { "R", "S", "D", "T", "t", "Z", "X" };

The 't' doesn't match what you do elsewhere. Try "TX" for that.
I think "ZX" would be right for an exiting process.




^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] cleanup ptrace stops and remove notify_parent
  2004-08-31 15:59 Albert Cahalan
@ 2004-08-31 23:54 ` Roland McGrath
  2004-09-01  0:27   ` Linus Torvalds
  0 siblings, 1 reply; 25+ messages in thread
From: Roland McGrath @ 2004-08-31 23:54 UTC (permalink / raw)
  To: Albert Cahalan
  Cc: linux-kernel mailing list, Andrew Morton OSDL, Linus Torvalds

> For TASK_DEAD and TASK_TRACED, might you be hiding some underlying state?

Not really.

> I want to display stuff like:
> 
> T    stopped (not traced)
> TX   stopped, and being traced
> RX   running or runnable, and being traced
> ZE   zombie, and trying to exit
> RXE  running, trying to exit, and traced

These distinctions don't exist in any data structures, except for the
PF_EXITING flag.  

> It's troubling the way TASK_DEAD and TASK_TRACED show up as
> actual states and hide any other stuff. I'd much prefer them
> to be flags of some sort.

That's a different implementation than what we've got now.  Feel free to
try to make it work.  Linus has talked before about treating the state as a
bitmask, but the code is not actually written that way now.  

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] cleanup ptrace stops and remove notify_parent
  2004-08-31 23:54 ` Roland McGrath
@ 2004-09-01  0:27   ` Linus Torvalds
  0 siblings, 0 replies; 25+ messages in thread
From: Linus Torvalds @ 2004-09-01  0:27 UTC (permalink / raw)
  To: Roland McGrath
  Cc: Albert Cahalan, linux-kernel mailing list, Andrew Morton OSDL



On Tue, 31 Aug 2004, Roland McGrath wrote:
> 
> That's a different implementation than what we've got now.  Feel free to
> try to make it work.  Linus has talked before about treating the state as a
> bitmask, but the code is not actually written that way now.  

The state itself shouldn't be a bitmask, but the _values_ get used for 
masks in quite a few places (ie "task->state & (TASK_DEAD | TASK_ZOMBIE)").

		Linus

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH] cleanup ptrace stops and remove notify_parent
@ 2004-08-31  3:25 Roland McGrath
  2004-08-31  3:43 ` Andrew Morton
                   ` (2 more replies)
  0 siblings, 3 replies; 25+ messages in thread
From: Roland McGrath @ 2004-08-31  3:25 UTC (permalink / raw)
  To: Linus Torvalds, Andrew Morton; +Cc: Linux Kernel Mailing List

This patch is against Linus's current tree.

This adds a new state TASK_TRACED that is used in place of TASK_STOPPED
when a thread stops because it is ptraced.  Now ptrace operations are only
permitted when the target is in TASK_TRACED state, not in TASK_STOPPED.
This means that if a process is stopped normally by a job control signal
and then you PTRACE_ATTACH to it, you will have to send it a SIGCONT before
you can do any ptrace operations on it.  (The SIGCONT will be reported to
ptrace and then you can discard it instead of passing it through when you
call PTRACE_CONT et al.)

If a traced child gets orphaned while in TASK_TRACED state, it morphs into
TASK_STOPPED state.  This makes it again possible to resume or destroy the
process with SIGCONT or SIGKILL.

All non-signal tracing stops should now be done via ptrace_notify.  I've
updated the syscall tracing code in several architectures to do this
instead of replicating the work by hand.  I also fixed several that were
unnecessarily repeating some of the checks in ptrace_check_attach.  Calling
ptrace_check_attach alone is sufficient, and the old checks repeated before
are now incorrect, not just superfluous.

I've closed a race in ptrace_check_attach.  With this, we should have a
robust guarantee that when ptrace starts operating, the task will be in
TASK_TRACED state and won't come out of it.  This is because the only way
to resume from TASK_TRACED is via ptrace operations, and only the one
parent thread attached as the tracer can do those.

This patch also cleans up the do_notify_parent and do_notify_parent_cldstop
code so that the dead and stopped cases are completely disjoint.  The
notify_parent function is gone.



Thanks,
Roland


Signed-off-by: Roland McGrath <roland@redhat.com>

diff -rBzpu linux-2.6.9/arch/arm/kernel/ptrace.c linux-2.6-ptracefix/arch/arm/kernel/ptrace.c
--- linux-2.6.9/arch/arm/kernel/ptrace.c	2004-08-30 20:10:49.000000000 -0700
+++ linux-2.6-ptracefix/arch/arm/kernel/ptrace.c	2004-08-30 18:20:03.000000000 -0700
@@ -792,11 +792,8 @@ asmlinkage void syscall_trace(int why, s
 
 	/* the 0x80 provides a way for the tracing parent to distinguish
 	   between a syscall stop and SIGTRAP delivery */
-	current->exit_code = SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
-					? 0x80 : 0);
-	current->state = TASK_STOPPED;
-	notify_parent(current, SIGCHLD);
-	schedule();
+	ptrace_notify(SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
+				 ? 0x80 : 0));
 	/*
 	 * this isn't the same as continuing with a signal, but it will do
 	 * for normal use.  strace only continues with a signal if the
diff -rBzpu linux-2.6.9/arch/arm26/kernel/ptrace.c linux-2.6-ptracefix/arch/arm26/kernel/ptrace.c
--- linux-2.6.9/arch/arm26/kernel/ptrace.c	2004-08-30 20:10:49.000000000 -0700
+++ linux-2.6-ptracefix/arch/arm26/kernel/ptrace.c	2004-08-30 18:20:03.000000000 -0700
@@ -729,11 +729,8 @@ asmlinkage void syscall_trace(int why, s
 
 	/* the 0x80 provides a way for the tracing parent to distinguish
 	   between a syscall stop and SIGTRAP delivery */
-	current->exit_code = SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
-					? 0x80 : 0);
-	current->state = TASK_STOPPED;
-	notify_parent(current, SIGCHLD);
-	schedule();
+	ptrace_notify(SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
+				 ? 0x80 : 0));
 	/*
 	 * this isn't the same as continuing with a signal, but it will do
 	 * for normal use.  strace only continues with a signal if the
diff -rBzpu linux-2.6.9/arch/cris/arch-v10/kernel/ptrace.c linux-2.6-ptracefix/arch/cris/arch-v10/kernel/ptrace.c
--- linux-2.6.9/arch/cris/arch-v10/kernel/ptrace.c	2004-08-30 20:10:49.000000000 -0700
+++ linux-2.6-ptracefix/arch/cris/arch-v10/kernel/ptrace.c	2004-08-30 18:20:03.000000000 -0700
@@ -85,17 +85,8 @@ sys_ptrace(long request, long pid, long 
 		goto out_tsk;
 	}
 	
-	ret = -ESRCH;
-	
-	if (!(child->ptrace & PT_PTRACED))
-		goto out_tsk;
-	
-	if (child->state != TASK_STOPPED) {
-		if (request != PTRACE_KILL)
-			goto out_tsk;
-	}
-	
-	if (child->parent != current)
+	ret = ptrace_check_attach(child, request == PTRACE_KILL);
+	if (ret < 0)
 		goto out_tsk;
 
 	switch (request) {
diff -rBzpu linux-2.6.9/arch/h8300/kernel/ptrace.c linux-2.6-ptracefix/arch/h8300/kernel/ptrace.c
--- linux-2.6.9/arch/h8300/kernel/ptrace.c	2004-08-30 20:10:49.000000000 -0700
+++ linux-2.6-ptracefix/arch/h8300/kernel/ptrace.c	2004-08-30 18:20:03.000000000 -0700
@@ -89,13 +89,6 @@ asmlinkage int sys_ptrace(long request, 
 		ret = ptrace_attach(child);
 		goto out_tsk;
 	}
-	ret = -ESRCH;
-	if (!(child->ptrace & PT_PTRACED))
-		goto out_tsk;
-	if (child->state != TASK_STOPPED) {
-		if (request != PTRACE_KILL)
-			goto out_tsk;
-	}
 	ret = ptrace_check_attach(child, request == PTRACE_KILL);
 	if (ret < 0)
 		goto out_tsk;
@@ -270,10 +263,8 @@ asmlinkage void syscall_trace(void)
 		return;
 	if (!(current->ptrace & PT_PTRACED))
 		return;
-	current->exit_code = SIGTRAP;
-	current->state = TASK_STOPPED;
-	notify_parent(current, SIGCHLD);
-	schedule();
+	ptrace_notify(SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
+				 ? 0x80 : 0));
 	/*
 	 * this isn't the same as continuing with a signal, but it will do
 	 * for normal use.  strace only continues with a signal if the
diff -rBzpu linux-2.6.9/arch/m68k/kernel/ptrace.c linux-2.6-ptracefix/arch/m68k/kernel/ptrace.c
--- linux-2.6.9/arch/m68k/kernel/ptrace.c	2004-08-30 20:10:49.000000000 -0700
+++ linux-2.6-ptracefix/arch/m68k/kernel/ptrace.c	2004-08-30 18:20:03.000000000 -0700
@@ -379,11 +379,8 @@ asmlinkage void syscall_trace(void)
 	if (!current->thread.work.delayed_trace &&
 	    !current->thread.work.syscall_trace)
 		return;
-	current->exit_code = SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
-					? 0x80 : 0);
-	current->state = TASK_STOPPED;
-	notify_parent(current, SIGCHLD);
-	schedule();
+	ptrace_notify(SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
+				 ? 0x80 : 0));
 	/*
 	 * this isn't the same as continuing with a signal, but it will do
 	 * for normal use.  strace only continues with a signal if the
diff -rBzpu linux-2.6.9/arch/m68knommu/kernel/ptrace.c linux-2.6-ptracefix/arch/m68knommu/kernel/ptrace.c
--- linux-2.6.9/arch/m68knommu/kernel/ptrace.c	2004-08-30 20:10:49.000000000 -0700
+++ linux-2.6-ptracefix/arch/m68knommu/kernel/ptrace.c	2004-08-30 18:20:03.000000000 -0700
@@ -133,13 +133,6 @@ asmlinkage int sys_ptrace(long request, 
 		ret = ptrace_attach(child);
 		goto out_tsk;
 	}
-	ret = -ESRCH;
-	if (!(child->ptrace & PT_PTRACED))
-		goto out_tsk;
-	if (child->state != TASK_STOPPED) {
-		if (request != PTRACE_KILL)
-			goto out_tsk;
-	}
 	ret = ptrace_check_attach(child, request == PTRACE_KILL);
 	if (ret < 0)
 		goto out_tsk;
@@ -376,10 +369,8 @@ asmlinkage void syscall_trace(void)
 		return;
 	if (!(current->ptrace & PT_PTRACED))
 		return;
-	current->exit_code = SIGTRAP;
-	current->state = TASK_STOPPED;
-	notify_parent(current, SIGCHLD);
-	schedule();
+	ptrace_notify(SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
+				 ? 0x80 : 0));
 	/*
 	 * this isn't the same as continuing with a signal, but it will do
 	 * for normal use.  strace only continues with a signal if the
diff -rBzpu linux-2.6.9/arch/parisc/kernel/ptrace.c linux-2.6-ptracefix/arch/parisc/kernel/ptrace.c
--- linux-2.6.9/arch/parisc/kernel/ptrace.c	2004-08-30 20:10:50.000000000 -0700
+++ linux-2.6-ptracefix/arch/parisc/kernel/ptrace.c	2004-08-30 18:20:03.000000000 -0700
@@ -404,11 +404,8 @@ void syscall_trace(void)
 		return;
 	if (!(current->ptrace & PT_PTRACED))
 		return;
-	current->exit_code = SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
-					? 0x80 : 0);
-	current->state = TASK_STOPPED;
-	notify_parent(current, SIGCHLD);
-	schedule();
+	ptrace_notify(SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
+				 ? 0x80 : 0));
 	/*
 	 * this isn't the same as continuing with a signal, but it will do
 	 * for normal use.  strace only continues with a signal if the
diff -rBzpu linux-2.6.9/arch/sh64/kernel/ptrace.c linux-2.6-ptracefix/arch/sh64/kernel/ptrace.c
--- linux-2.6.9/arch/sh64/kernel/ptrace.c	2004-08-30 20:10:50.000000000 -0700
+++ linux-2.6-ptracefix/arch/sh64/kernel/ptrace.c	2004-08-30 18:20:03.000000000 -0700
@@ -311,11 +311,8 @@ asmlinkage void syscall_trace(void)
 	if (!(tsk->ptrace & PT_PTRACED))
 		return;
 
-	tsk->exit_code = SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
-				    ? 0x80 : 0);
-	tsk->state = TASK_STOPPED;
-	notify_parent(tsk, SIGCHLD);
-	schedule();
+	ptrace_notify(SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
+				 ? 0x80 : 0));
 	/*
 	 * this isn't the same as continuing with a signal, but it will do
 	 * for normal use.  strace only continues with a signal if the
diff -rBzpu linux-2.6.9/arch/sparc/kernel/ptrace.c linux-2.6-ptracefix/arch/sparc/kernel/ptrace.c
--- linux-2.6.9/arch/sparc/kernel/ptrace.c	2004-08-30 20:10:50.000000000 -0700
+++ linux-2.6-ptracefix/arch/sparc/kernel/ptrace.c	2004-08-30 18:20:04.000000000 -0700
@@ -614,12 +614,9 @@ asmlinkage void syscall_trace(void)
 		return;
 	if (!(current->ptrace & PT_PTRACED))
 		return;
-	current->exit_code = SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
-					? 0x80 : 0);
-	current->state = TASK_STOPPED;
 	current->thread.flags ^= MAGIC_CONSTANT;
-	notify_parent(current, SIGCHLD);
-	schedule();
+	ptrace_notify(SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
+				 ? 0x80 : 0));
 	/*
 	 * this isn't the same as continuing with a signal, but it will do
 	 * for normal use.  strace only continues with a signal if the
diff -rBzpu linux-2.6.9/arch/sparc64/kernel/ptrace.c linux-2.6-ptracefix/arch/sparc64/kernel/ptrace.c
--- linux-2.6.9/arch/sparc64/kernel/ptrace.c	2004-08-30 20:10:50.000000000 -0700
+++ linux-2.6-ptracefix/arch/sparc64/kernel/ptrace.c	2004-08-30 18:20:04.000000000 -0700
@@ -627,11 +627,8 @@ asmlinkage void syscall_trace(void)
 		return;
 	if (!(current->ptrace & PT_PTRACED))
 		return;
-	current->exit_code = SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
-					? 0x80 : 0);
-	current->state = TASK_STOPPED;
-	notify_parent(current, SIGCHLD);
-	schedule();
+	ptrace_notify(SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
+				 ? 0x80 : 0));
 
 	/*
 	 * this isn't the same as continuing with a signal, but it will do
diff -rBzpu linux-2.6.9/arch/v850/kernel/ptrace.c linux-2.6-ptracefix/arch/v850/kernel/ptrace.c
--- linux-2.6.9/arch/v850/kernel/ptrace.c	2004-08-30 20:10:50.000000000 -0700
+++ linux-2.6-ptracefix/arch/v850/kernel/ptrace.c	2004-08-30 18:20:04.000000000 -0700
@@ -147,14 +147,8 @@ int sys_ptrace(long request, long pid, l
 		rval = ptrace_attach(child);
 		goto out_tsk;
 	}
-	rval = -ESRCH;
-	if (!(child->ptrace & PT_PTRACED))
-		goto out_tsk;
-	if (child->state != TASK_STOPPED) {
-		if (request != PTRACE_KILL)
-			goto out_tsk;
-	}
-	if (child->parent != current)
+	ret = ptrace_check_attach(child, request == PTRACE_KILL);
+	if (ret < 0)
 		goto out_tsk;
 
 	switch (request) {
@@ -269,11 +263,8 @@ asmlinkage void syscall_trace(void)
 		return;
 	/* The 0x80 provides a way for the tracing parent to distinguish
 	   between a syscall stop and SIGTRAP delivery */
-	current->exit_code = SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
-					? 0x80 : 0);
-	current->state = TASK_STOPPED;
-	notify_parent(current, SIGCHLD);
-	schedule();
+	ptrace_notify(SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
+				 ? 0x80 : 0));
 	/*
 	 * this isn't the same as continuing with a signal, but it will do
 	 * for normal use.  strace only continues with a signal if the
diff -rBzpu linux-2.6.9/fs/proc/array.c linux-2.6-ptracefix/fs/proc/array.c
--- linux-2.6.9/fs/proc/array.c	2004-08-30 20:10:50.000000000 -0700
+++ linux-2.6-ptracefix/fs/proc/array.c	2004-08-30 18:43:29.000000000 -0700
@@ -130,8 +130,9 @@ static const char *task_state_array[] = 
 	"S (sleeping)",		/*  1 */
 	"D (disk sleep)",	/*  2 */
 	"T (stopped)",		/*  4 */
-	"Z (zombie)",		/*  8 */
-	"X (dead)"		/* 16 */
+	"T (tracing stop)",	/*  8 */
+	"Z (zombie)",		/* 16 */
+	"X (dead)"		/* 32 */
 };
 
 static inline const char * get_task_state(struct task_struct *tsk)
@@ -141,7 +142,8 @@ static inline const char * get_task_stat
 					   TASK_UNINTERRUPTIBLE |
 					   TASK_ZOMBIE |
 					   TASK_DEAD |
-					   TASK_STOPPED);
+					   TASK_STOPPED |
+					   TASK_TRACED);
 	const char **p = &task_state_array[0];
 
 	while (state) {
diff -rBzpu linux-2.6.9/fs/proc/base.c linux-2.6-ptracefix/fs/proc/base.c
--- linux-2.6.9/fs/proc/base.c	2004-08-30 20:10:50.000000000 -0700
+++ linux-2.6-ptracefix/fs/proc/base.c	2004-08-30 18:20:05.000000000 -0700
@@ -287,7 +287,8 @@ static int proc_root_link(struct inode *
 #define MAY_PTRACE(task) \
 	(task == current || \
 	(task->parent == current && \
-	(task->ptrace & PT_PTRACED) &&  task->state == TASK_STOPPED && \
+	(task->ptrace & PT_PTRACED) && \
+	 (task->state == TASK_STOPPED || task->state == TASK_TRACED) && \
 	 security_ptrace(current,task) == 0))
 
 static int may_ptrace_attach(struct task_struct *task)
diff -rBzpu linux-2.6.9/include/linux/sched.h linux-2.6-ptracefix/include/linux/sched.h
--- linux-2.6.9/include/linux/sched.h	2004-08-30 20:10:50.000000000 -0700
+++ linux-2.6-ptracefix/include/linux/sched.h	2004-08-30 18:20:05.000000000 -0700
@@ -106,8 +106,9 @@ extern unsigned long nr_iowait(void);
 #define TASK_INTERRUPTIBLE	1
 #define TASK_UNINTERRUPTIBLE	2
 #define TASK_STOPPED		4
-#define TASK_ZOMBIE		8
-#define TASK_DEAD		16
+#define TASK_TRACED		8
+#define TASK_ZOMBIE		16
+#define TASK_DEAD		32
 
 #define __set_task_state(tsk, state_value)		\
 	do { (tsk)->state = (state_value); } while (0)
@@ -725,7 +726,6 @@ extern int __kill_pg_info(int sig, struc
 extern int kill_pg_info(int, struct siginfo *, pid_t);
 extern int kill_sl_info(int, struct siginfo *, pid_t);
 extern int kill_proc_info(int, struct siginfo *, pid_t);
-extern void notify_parent(struct task_struct *, int);
 extern void do_notify_parent(struct task_struct *, int);
 extern void force_sig(int, struct task_struct *);
 extern void force_sig_specific(int, struct task_struct *);
diff -rBzpu linux-2.6.9/kernel/power/process.c linux-2.6-ptracefix/kernel/power/process.c
--- linux-2.6.9/kernel/power/process.c	2004-08-30 20:10:51.000000000 -0700
+++ linux-2.6-ptracefix/kernel/power/process.c	2004-08-30 18:20:05.000000000 -0700
@@ -25,7 +25,8 @@ static inline int freezeable(struct task
 	    (p->flags & PF_NOFREEZE) ||
 	    (p->state == TASK_ZOMBIE) ||
 	    (p->state == TASK_DEAD) ||
-	    (p->state == TASK_STOPPED))
+	    (p->state == TASK_STOPPED) ||
+	    (p->state == TASK_TRACED))
 		return 0;
 	return 1;
 }
@@ -70,6 +71,7 @@ int freeze_processes(void)
 			if (!freezeable(p))
 				continue;
 			if ((p->flags & PF_FROZEN) ||
+			    (p->state == TASK_TRACED) ||
 			    (p->state == TASK_STOPPED))
 				continue;
 
diff -rBzpu linux-2.6.9/kernel/ptrace.c linux-2.6-ptracefix/kernel/ptrace.c
--- linux-2.6.9/kernel/ptrace.c	2004-08-30 20:10:51.000000000 -0700
+++ linux-2.6-ptracefix/kernel/ptrace.c	2004-08-30 19:35:34.000000000 -0700
@@ -55,6 +55,15 @@ void __ptrace_unlink(task_t *child)
 	REMOVE_LINKS(child);
 	child->parent = child->real_parent;
 	SET_LINKS(child);
+
+	if (child->state == TASK_TRACED) {
+		/*
+		 * Turn a tracing stop into a normal stop now,
+		 * since with no tracer there would be no way
+		 * to wake it up with SIGCONT or SIGKILL.
+		 */
+		child->state = TASK_STOPPED;
+	}
 }
 
 /*
@@ -62,20 +71,28 @@ void __ptrace_unlink(task_t *child)
  */
 int ptrace_check_attach(struct task_struct *child, int kill)
 {
-	if (!(child->ptrace & PT_PTRACED))
-		return -ESRCH;
+	int ret = -ESRCH;
 
-	if (child->parent != current)
-		return -ESRCH;
+	/*
+	 * We take the read lock around doing both checks to close a
+	 * possible race where someone else was tracing our child and
+	 * detached between these two checks.  After this locked check,
+	 * we are sure that this is our traced child and that can only
+	 * be changed by us so it's not changing right after this.
+	 */
+	read_lock(&tasklist_lock);
+	if ((child->ptrace & PT_PTRACED) && child->parent == current)
+		ret = 0;
+	read_unlock(&tasklist_lock);
 
-	if (!kill) {
-		if (child->state != TASK_STOPPED)
+	if (!ret && !kill) {
+		if (child->state != TASK_TRACED)
 			return -ESRCH;
 		wait_task_inactive(child);
 	}
 
 	/* All systems go.. */
-	return 0;
+	return ret;
 }
 
 int ptrace_attach(struct task_struct *task)
@@ -281,15 +298,13 @@ static int ptrace_setoptions(struct task
 
 static int ptrace_getsiginfo(struct task_struct *child, siginfo_t __user * data)
 {
-	if (child->last_siginfo == NULL)
-		return -EINVAL;
+	BUG_ON(child->last_siginfo == NULL);
 	return copy_siginfo_to_user(data, child->last_siginfo);
 }
 
 static int ptrace_setsiginfo(struct task_struct *child, siginfo_t __user * data)
 {
-	if (child->last_siginfo == NULL)
-		return -EINVAL;
+	BUG_ON(child->last_siginfo == NULL);
 	if (copy_from_user(child->last_siginfo, data, sizeof (siginfo_t)) != 0)
 		return -EFAULT;
 	return 0;
@@ -322,24 +337,3 @@ int ptrace_request(struct task_struct *c
 
 	return ret;
 }
-
-void ptrace_notify(int exit_code)
-{
-	BUG_ON (!(current->ptrace & PT_PTRACED));
-
-	/* Let the debugger run.  */
-	current->exit_code = exit_code;
-	set_current_state(TASK_STOPPED);
-	notify_parent(current, SIGCHLD);
-	schedule();
-
-	/*
-	 * Signals sent while we were stopped might set TIF_SIGPENDING.
-	 */
-
-	spin_lock_irq(&current->sighand->siglock);
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
-}
-
-EXPORT_SYMBOL(ptrace_notify);
diff -rBzpu linux-2.6.9/kernel/sched.c linux-2.6-ptracefix/kernel/sched.c
--- linux-2.6.9/kernel/sched.c	2004-08-30 20:10:51.000000000 -0700
+++ linux-2.6-ptracefix/kernel/sched.c	2004-08-30 18:58:34.000000000 -0700
@@ -1258,7 +1258,7 @@ out:
 
 int fastcall wake_up_process(task_t * p)
 {
-	return try_to_wake_up(p, TASK_STOPPED |
+	return try_to_wake_up(p, TASK_STOPPED | TASK_TRACED |
 		       		 TASK_INTERRUPTIBLE | TASK_UNINTERRUPTIBLE, 0);
 }
 
@@ -3670,7 +3670,7 @@ static void show_task(task_t * p)
 	task_t *relative;
 	unsigned state;
 	unsigned long free = 0;
-	static const char *stat_nam[] = { "R", "S", "D", "T", "Z", "W" };
+	static const char *stat_nam[] = { "R", "S", "D", "T", "t", "Z", "X" };
 
 	printk("%-13.13s ", p->comm);
 	state = p->state ? __ffs(p->state) + 1 : 0;
diff -rBzpu linux-2.6.9/kernel/signal.c linux-2.6-ptracefix/kernel/signal.c
--- linux-2.6.9/kernel/signal.c	2004-08-30 20:10:51.000000000 -0700
+++ linux-2.6-ptracefix/kernel/signal.c	2004-08-30 20:04:45.000000000 -0700
@@ -618,7 +618,8 @@ static int check_kill_permission(int sig
 
 /* forward decl */
 static void do_notify_parent_cldstop(struct task_struct *tsk,
-				     struct task_struct *parent);
+				     struct task_struct *parent,
+				     int why);
 
 /*
  * Handle magic process-wide effects of stop/continue signals.
@@ -661,11 +662,13 @@ static void handle_stop_signal(int sig, 
 			 */
 			p->signal->group_stop_count = 0;
 			if (p->ptrace & PT_PTRACED)
-				do_notify_parent_cldstop(p, p->parent);
+				do_notify_parent_cldstop(p, p->parent,
+							 CLD_STOPPED);
 			else
 				do_notify_parent_cldstop(
 					p->group_leader,
-					p->group_leader->real_parent);
+					p->group_leader->real_parent,
+					CLD_STOPPED);
 		}
 		rm_from_queue(SIG_KERNEL_STOP_MASK, &p->signal->shared_pending);
 		t = p;
@@ -861,11 +864,20 @@ force_sig_specific(int sig, struct task_
 
 
 static void
-__group_complete_signal(int sig, struct task_struct *p, unsigned int mask)
+__group_complete_signal(int sig, struct task_struct *p)
 {
+	unsigned int mask;
 	struct task_struct *t;
 
 	/*
+	 * Don't bother zombies and stopped tasks (but
+	 * SIGKILL will punch through stopped state)
+	 */
+	mask = TASK_DEAD | TASK_ZOMBIE | TASK_TRACED;
+	if (sig != SIGKILL)
+		mask |= TASK_STOPPED;
+
+	/*
 	 * Now find a thread we can wake up to take the signal off the queue.
 	 *
 	 * If the main thread wants the signal, it gets first crack.
@@ -966,7 +978,6 @@ __group_complete_signal(int sig, struct 
 static int
 __group_send_sig_info(int sig, struct siginfo *info, struct task_struct *p)
 {
-	unsigned int mask;
 	int ret = 0;
 
 #ifdef CONFIG_SMP
@@ -990,14 +1001,6 @@ __group_send_sig_info(int sig, struct si
 		return ret;
 
 	/*
-	 * Don't bother zombies and stopped tasks (but
-	 * SIGKILL will punch through stopped state)
-	 */
-	mask = TASK_DEAD | TASK_ZOMBIE;
-	if (sig != SIGKILL)
-		mask |= TASK_STOPPED;
-
-	/*
 	 * Put this signal on the shared-pending queue, or fail with EAGAIN.
 	 * We always use the shared queue for process-wide signals,
 	 * to avoid several races.
@@ -1006,7 +1009,7 @@ __group_send_sig_info(int sig, struct si
 	if (unlikely(ret))
 		return ret;
 
-	__group_complete_signal(sig, p, mask);
+	__group_complete_signal(sig, p);
 	return 0;
 }
 
@@ -1367,7 +1370,6 @@ int
 send_group_sigqueue(int sig, struct sigqueue *q, struct task_struct *p)
 {
 	unsigned long flags;
-	unsigned int mask;
 	int ret = 0;
 
 	BUG_ON(!(q->flags & SIGQUEUE_PREALLOC));
@@ -1392,13 +1394,6 @@ send_group_sigqueue(int sig, struct sigq
 		q->info.si_overrun++;
 		goto out;
 	} 
-	/*
-	 * Don't bother zombies and stopped tasks (but
-	 * SIGKILL will punch through stopped state)
-	 */
-	mask = TASK_DEAD | TASK_ZOMBIE;
-	if (sig != SIGKILL)
-		mask |= TASK_STOPPED;
 
 	/*
 	 * Put this signal on the shared-pending queue.
@@ -1409,7 +1404,7 @@ send_group_sigqueue(int sig, struct sigq
 	list_add_tail(&q->list, &p->signal->shared_pending.list);
 	sigaddset(&p->signal->shared_pending.signal, sig);
 
-	__group_complete_signal(sig, p, mask);
+	__group_complete_signal(sig, p);
 out:
 	spin_unlock_irqrestore(&p->sighand->siglock, flags);
 	read_unlock(&tasklist_lock);
@@ -1442,21 +1437,24 @@ static void __wake_up_parent(struct task
 }
 
 /*
- * Let a parent know about a status change of a child.
+ * Let a parent know about the death of a child.
+ * For a stopped/continued status change, use do_notify_parent_cldstop instead.
  */
 
 void do_notify_parent(struct task_struct *tsk, int sig)
 {
 	struct siginfo info;
 	unsigned long flags;
-	int why, status;
 	struct sighand_struct *psig;
 
 	if (sig == -1)
 		BUG();
 
-	BUG_ON(tsk->group_leader != tsk && tsk->group_leader->state != TASK_ZOMBIE && !tsk->ptrace);
-	BUG_ON(tsk->group_leader == tsk && !thread_group_empty(tsk) && !tsk->ptrace);
+	/* do_notify_parent_cldstop should have been called instead.  */
+	BUG_ON(tsk->state & (TASK_STOPPED|TASK_TRACED));
+
+	BUG_ON(!tsk->ptrace &&
+	       (tsk->group_leader != tsk || !thread_group_empty(tsk)));
 
 	info.si_signo = sig;
 	info.si_errno = 0;
@@ -1467,34 +1465,19 @@ void do_notify_parent(struct task_struct
 	info.si_utime = tsk->utime;
 	info.si_stime = tsk->stime;
 
-	status = tsk->exit_code & 0x7f;
-	why = SI_KERNEL;	/* shouldn't happen */
-	switch (tsk->state) {
-	case TASK_STOPPED:
-		/* FIXME -- can we deduce CLD_TRAPPED or CLD_CONTINUED? */
-		if (tsk->ptrace & PT_PTRACED)
-			why = CLD_TRAPPED;
-		else
-			why = CLD_STOPPED;
-		break;
-
-	default:
+	info.si_status = tsk->exit_code & 0x7f;
 		if (tsk->exit_code & 0x80)
-			why = CLD_DUMPED;
+		info.si_code = CLD_DUMPED;
 		else if (tsk->exit_code & 0x7f)
-			why = CLD_KILLED;
+		info.si_code = CLD_KILLED;
 		else {
-			why = CLD_EXITED;
-			status = tsk->exit_code >> 8;
-		}
-		break;
+		info.si_code = CLD_EXITED;
+		info.si_status = tsk->exit_code >> 8;
 	}
-	info.si_code = why;
-	info.si_status = status;
 
 	psig = tsk->parent->sighand;
 	spin_lock_irqsave(&psig->siglock, flags);
-	if (sig == SIGCHLD && tsk->state != TASK_STOPPED &&
+	if (sig == SIGCHLD &&
 	    (psig->action[SIGCHLD-1].sa.sa_handler == SIG_IGN ||
 	     (psig->action[SIGCHLD-1].sa.sa_flags & SA_NOCLDWAIT))) {
 		/*
@@ -1522,26 +1505,9 @@ void do_notify_parent(struct task_struct
 	spin_unlock_irqrestore(&psig->siglock, flags);
 }
 
-
-/*
- * We need the tasklist lock because it's the only
- * thing that protects out "parent" pointer.
- *
- * exit.c calls "do_notify_parent()" directly, because
- * it already has the tasklist lock.
- */
-void
-notify_parent(struct task_struct *tsk, int sig)
-{
-	if (sig != -1) {
-		read_lock(&tasklist_lock);
-		do_notify_parent(tsk, sig);
-		read_unlock(&tasklist_lock);
-	}
-}
-
 static void
-do_notify_parent_cldstop(struct task_struct *tsk, struct task_struct *parent)
+do_notify_parent_cldstop(struct task_struct *tsk, struct task_struct *parent,
+			 int why)
 {
 	struct siginfo info;
 	unsigned long flags;
@@ -1556,8 +1522,20 @@ do_notify_parent_cldstop(struct task_str
 	info.si_utime = tsk->utime;
 	info.si_stime = tsk->stime;
 
-	info.si_status = tsk->exit_code & 0x7f;
-	info.si_code = CLD_STOPPED;
+	info.si_code = why;
+	switch (why) {
+	case CLD_CONTINUED:
+		info.si_status = SIGCONT;
+		break;
+	case CLD_STOPPED:
+		info.si_status = tsk->signal->group_exit_code & 0x7f;
+		break;
+	case CLD_TRAPPED:
+		info.si_status = tsk->exit_code & 0x7f;
+		break;
+	default:
+		BUG();
+	}
 
 	sighand = parent->sighand;
 	spin_lock_irqsave(&sighand->siglock, flags);
@@ -1571,6 +1549,68 @@ do_notify_parent_cldstop(struct task_str
 	spin_unlock_irqrestore(&sighand->siglock, flags);
 }
 
+/*
+ * This must be called with current->sighand->siglock held.
+ *
+ * This should be the path for all ptrace stops.
+ * We always set current->last_siginfo while stopped here.
+ * That makes it a way to test a stopped process for
+ * being ptrace-stopped vs being job-control-stopped.
+ */
+static void ptrace_stop(int exit_code, siginfo_t *info)
+{
+	BUG_ON(!(current->ptrace & PT_PTRACED));
+
+	/*
+	 * If there is a group stop in progress,
+	 * we must participate in the bookkeeping.
+	 */
+	if (current->signal->group_stop_count > 0)
+		--current->signal->group_stop_count;
+
+	current->last_siginfo = info;
+	current->exit_code = exit_code;
+
+	/* Let the debugger run.  */
+	set_current_state(TASK_TRACED);
+	spin_unlock_irq(&current->sighand->siglock);
+	read_lock(&tasklist_lock);
+	do_notify_parent_cldstop(current, current->parent, CLD_TRAPPED);
+	read_unlock(&tasklist_lock);
+	schedule();
+
+	/*
+	 * We are back.  Now reacquire the siglock before touching
+	 * last_siginfo, so that we are sure to have synchronized with
+	 * any signal-sending on another CPU that wants to examine it.
+	 */
+	spin_lock_irq(&current->sighand->siglock);
+	current->last_siginfo = NULL;
+
+	/*
+	 * Queued signals ignored us while we were stopped for tracing.
+	 * So check for any that we should take before resuming user mode.
+	 */
+	recalc_sigpending();
+}
+
+void ptrace_notify(int exit_code)
+{
+	siginfo_t info;
+
+	BUG_ON((exit_code & (0x7f | ~0xffff)) != SIGTRAP);
+
+	memset(&info, 0, sizeof info);
+	info.si_signo = SIGTRAP;
+	info.si_code = exit_code;
+	info.si_pid = current->pid;
+	info.si_uid = current->uid;
+
+	/* Let the debugger run.  */
+	spin_lock_irq(&current->sighand->siglock);
+	ptrace_stop(exit_code, &info);
+	spin_unlock_irq(&current->sighand->siglock);
+}
 
 #ifndef HAVE_ARCH_GET_SIGNAL_TO_DELIVER
 
@@ -1584,13 +1624,15 @@ finish_stop(int stop_count)
 	 */
 	if (stop_count < 0 || (current->ptrace & PT_PTRACED)) {
 		read_lock(&tasklist_lock);
-		do_notify_parent_cldstop(current, current->parent);
+		do_notify_parent_cldstop(current, current->parent,
+					 CLD_STOPPED);
 		read_unlock(&tasklist_lock);
 	}
 	else if (stop_count == 0) {
 		read_lock(&tasklist_lock);
 		do_notify_parent_cldstop(current->group_leader,
-					 current->group_leader->real_parent);
+					 current->group_leader->real_parent,
+					 CLD_STOPPED);
 		read_unlock(&tasklist_lock);
 	}
 
@@ -1629,7 +1671,7 @@ do_signal_stop(int signr)
 		/*
 		 * Lock must be held through transition to stopped state.
 		 */
-		current->exit_code = signr;
+		current->exit_code = current->signal->group_exit_code = signr;
 		set_current_state(TASK_STOPPED);
 		spin_unlock_irq(&sighand->siglock);
 	}
@@ -1766,25 +1808,10 @@ relock:
 		if ((current->ptrace & PT_PTRACED) && signr != SIGKILL) {
 			ptrace_signal_deliver(regs, cookie);
 
-			/*
-			 * If there is a group stop in progress,
-			 * we must participate in the bookkeeping.
-			 */
-			if (current->signal->group_stop_count > 0)
-				--current->signal->group_stop_count;
-
 			/* Let the debugger run.  */
-			current->exit_code = signr;
-			current->last_siginfo = info;
-			set_current_state(TASK_STOPPED);
-			spin_unlock_irq(&current->sighand->siglock);
-			notify_parent(current, SIGCHLD);
-			schedule();
-
-			current->last_siginfo = NULL;
+			ptrace_stop(signr, info);
 
 			/* We're back.  Did the debugger cancel the sig?  */
-			spin_lock_irq(&current->sighand->siglock);
 			signr = current->exit_code;
 			if (signr == 0)
 				continue;
@@ -1915,7 +1942,7 @@ EXPORT_SYMBOL(kill_proc);
 EXPORT_SYMBOL(kill_proc_info);
 EXPORT_SYMBOL(kill_sl);
 EXPORT_SYMBOL(kill_sl_info);
-EXPORT_SYMBOL(notify_parent);
+EXPORT_SYMBOL(ptrace_notify);
 EXPORT_SYMBOL(send_sig);
 EXPORT_SYMBOL(send_sig_info);
 EXPORT_SYMBOL(send_group_sig_info);


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] cleanup ptrace stops and remove notify_parent
  2004-08-31  3:25 Roland McGrath
@ 2004-08-31  3:43 ` Andrew Morton
  2004-08-31  4:11   ` Roland McGrath
  2004-08-31  3:59 ` Roland McGrath
  2004-08-31  4:22 ` Daniel Jacobowitz
  2 siblings, 1 reply; 25+ messages in thread
From: Andrew Morton @ 2004-08-31  3:43 UTC (permalink / raw)
  To: Roland McGrath; +Cc: torvalds, linux-kernel

Roland McGrath <roland@redhat.com> wrote:
>
> This patch is against Linus's current tree.

It clashes significantly with your waitid syscall patch.  Which one do you
believe has priority?

> +	info.si_status = tsk->exit_code & 0x7f;
>  		if (tsk->exit_code & 0x80)
> -			why = CLD_DUMPED;
> +		info.si_code = CLD_DUMPED;
>  		else if (tsk->exit_code & 0x7f)
> -			why = CLD_KILLED;
> +		info.si_code = CLD_KILLED;
>  		else {
> -			why = CLD_EXITED;
> -			status = tsk->exit_code >> 8;
> -		}
> -		break;
> +		info.si_code = CLD_EXITED;
> +		info.si_status = tsk->exit_code >> 8;

Indenting catastrophe in do_notify_parent() needs fixing please.


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] cleanup ptrace stops and remove notify_parent
  2004-08-31  3:43 ` Andrew Morton
@ 2004-08-31  4:11   ` Roland McGrath
  2004-08-31  4:26     ` Andrew Morton
                       ` (2 more replies)
  0 siblings, 3 replies; 25+ messages in thread
From: Roland McGrath @ 2004-08-31  4:11 UTC (permalink / raw)
  To: Andrew Morton; +Cc: torvalds, linux-kernel

> It clashes significantly with your waitid syscall patch.  Which one do you
> believe has priority?

I of course already have a tree with both reconciled.  I did this ptrace
patch independently against Linus's tree since it was based on suggestions
from Linus and I thought he might like to try it and perhaps merge it
before my other patches in -mm go in.

I think it makes sense for this patch to be applied before the waitid and
rusage patches that probably conflict with it a lot.  It's simpler than those.
But you know more about patch order issues in your tree than I do.

I can give you whatever merged flavors you would like.  I can easily give
you a version of this patch against 2.6.9-mm1.  Or, I can take 2.6.9-mm1,
revert the waitid and rusage patches, apply this patch, and give you
replacement patches for those two intended for this patch to be applied
first.  Let me know what is most convenient.

> Indenting catastrophe in do_notify_parent() needs fixing please.

Oops!  Fallout from the various steps in moving changes between trees.
Sorry about that.  Here is yet another version of that patch against
Linus's tree.


Thanks,
Roland

---


This patch is against Linus's current tree.

This adds a new state TASK_TRACED that is used in place of TASK_STOPPED
when a thread stops because it is ptraced.  Now ptrace operations are only
permitted when the target is in TASK_TRACED state, not in TASK_STOPPED.
This means that if a process is stopped normally by a job control signal
and then you PTRACE_ATTACH to it, you will have to send it a SIGCONT before
you can do any ptrace operations on it.  (The SIGCONT will be reported to
ptrace and then you can discard it instead of passing it through when you
call PTRACE_CONT et al.)

If a traced child gets orphaned while in TASK_TRACED state, it morphs into
TASK_STOPPED state.  This makes it again possible to resume or destroy the
process with SIGCONT or SIGKILL.

All non-signal tracing stops should now be done via ptrace_notify.  I've
updated the syscall tracing code in several architectures to do this
instead of replicating the work by hand.  I also fixed several that were
unnecessarily repeating some of the checks in ptrace_check_attach.  Calling
ptrace_check_attach alone is sufficient, and the old checks repeated before
are now incorrect, not just superfluous.

I've closed a race in ptrace_check_attach.  With this, we should have a
robust guarantee that when ptrace starts operating, the task will be in
TASK_TRACED state and won't come out of it.  This is because the only way
to resume from TASK_TRACED is via ptrace operations, and only the one
parent thread attached as the tracer can do those.

This patch also cleans up the do_notify_parent and do_notify_parent_cldstop
code so that the dead and stopped cases are completely disjoint.  The
notify_parent function is gone.



Thanks,
Roland


Signed-off-by: Roland McGrath <roland@redhat.com>

diff -rBzpu linux-2.6.9/arch/arm/kernel/ptrace.c linux-2.6-ptracefix/arch/arm/kernel/ptrace.c
--- linux-2.6.9/arch/arm/kernel/ptrace.c	2004-08-30 20:10:49.000000000 -0700
+++ linux-2.6-ptracefix/arch/arm/kernel/ptrace.c	2004-08-30 18:20:03.000000000 -0700
@@ -792,11 +792,8 @@ asmlinkage void syscall_trace(int why, s
 
 	/* the 0x80 provides a way for the tracing parent to distinguish
 	   between a syscall stop and SIGTRAP delivery */
-	current->exit_code = SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
-					? 0x80 : 0);
-	current->state = TASK_STOPPED;
-	notify_parent(current, SIGCHLD);
-	schedule();
+	ptrace_notify(SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
+				 ? 0x80 : 0));
 	/*
 	 * this isn't the same as continuing with a signal, but it will do
 	 * for normal use.  strace only continues with a signal if the
diff -rBzpu linux-2.6.9/arch/arm26/kernel/ptrace.c linux-2.6-ptracefix/arch/arm26/kernel/ptrace.c
--- linux-2.6.9/arch/arm26/kernel/ptrace.c	2004-08-30 20:10:49.000000000 -0700
+++ linux-2.6-ptracefix/arch/arm26/kernel/ptrace.c	2004-08-30 18:20:03.000000000 -0700
@@ -729,11 +729,8 @@ asmlinkage void syscall_trace(int why, s
 
 	/* the 0x80 provides a way for the tracing parent to distinguish
 	   between a syscall stop and SIGTRAP delivery */
-	current->exit_code = SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
-					? 0x80 : 0);
-	current->state = TASK_STOPPED;
-	notify_parent(current, SIGCHLD);
-	schedule();
+	ptrace_notify(SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
+				 ? 0x80 : 0));
 	/*
 	 * this isn't the same as continuing with a signal, but it will do
 	 * for normal use.  strace only continues with a signal if the
diff -rBzpu linux-2.6.9/arch/cris/arch-v10/kernel/ptrace.c linux-2.6-ptracefix/arch/cris/arch-v10/kernel/ptrace.c
--- linux-2.6.9/arch/cris/arch-v10/kernel/ptrace.c	2004-08-30 20:10:49.000000000 -0700
+++ linux-2.6-ptracefix/arch/cris/arch-v10/kernel/ptrace.c	2004-08-30 18:20:03.000000000 -0700
@@ -85,17 +85,8 @@ sys_ptrace(long request, long pid, long 
 		goto out_tsk;
 	}
 	
-	ret = -ESRCH;
-	
-	if (!(child->ptrace & PT_PTRACED))
-		goto out_tsk;
-	
-	if (child->state != TASK_STOPPED) {
-		if (request != PTRACE_KILL)
-			goto out_tsk;
-	}
-	
-	if (child->parent != current)
+	ret = ptrace_check_attach(child, request == PTRACE_KILL);
+	if (ret < 0)
 		goto out_tsk;
 
 	switch (request) {
diff -rBzpu linux-2.6.9/arch/h8300/kernel/ptrace.c linux-2.6-ptracefix/arch/h8300/kernel/ptrace.c
--- linux-2.6.9/arch/h8300/kernel/ptrace.c	2004-08-30 20:10:49.000000000 -0700
+++ linux-2.6-ptracefix/arch/h8300/kernel/ptrace.c	2004-08-30 18:20:03.000000000 -0700
@@ -89,13 +89,6 @@ asmlinkage int sys_ptrace(long request, 
 		ret = ptrace_attach(child);
 		goto out_tsk;
 	}
-	ret = -ESRCH;
-	if (!(child->ptrace & PT_PTRACED))
-		goto out_tsk;
-	if (child->state != TASK_STOPPED) {
-		if (request != PTRACE_KILL)
-			goto out_tsk;
-	}
 	ret = ptrace_check_attach(child, request == PTRACE_KILL);
 	if (ret < 0)
 		goto out_tsk;
@@ -270,10 +263,8 @@ asmlinkage void syscall_trace(void)
 		return;
 	if (!(current->ptrace & PT_PTRACED))
 		return;
-	current->exit_code = SIGTRAP;
-	current->state = TASK_STOPPED;
-	notify_parent(current, SIGCHLD);
-	schedule();
+	ptrace_notify(SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
+				 ? 0x80 : 0));
 	/*
 	 * this isn't the same as continuing with a signal, but it will do
 	 * for normal use.  strace only continues with a signal if the
diff -rBzpu linux-2.6.9/arch/m68k/kernel/ptrace.c linux-2.6-ptracefix/arch/m68k/kernel/ptrace.c
--- linux-2.6.9/arch/m68k/kernel/ptrace.c	2004-08-30 20:10:49.000000000 -0700
+++ linux-2.6-ptracefix/arch/m68k/kernel/ptrace.c	2004-08-30 18:20:03.000000000 -0700
@@ -379,11 +379,8 @@ asmlinkage void syscall_trace(void)
 	if (!current->thread.work.delayed_trace &&
 	    !current->thread.work.syscall_trace)
 		return;
-	current->exit_code = SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
-					? 0x80 : 0);
-	current->state = TASK_STOPPED;
-	notify_parent(current, SIGCHLD);
-	schedule();
+	ptrace_notify(SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
+				 ? 0x80 : 0));
 	/*
 	 * this isn't the same as continuing with a signal, but it will do
 	 * for normal use.  strace only continues with a signal if the
diff -rBzpu linux-2.6.9/arch/m68knommu/kernel/ptrace.c linux-2.6-ptracefix/arch/m68knommu/kernel/ptrace.c
--- linux-2.6.9/arch/m68knommu/kernel/ptrace.c	2004-08-30 20:10:49.000000000 -0700
+++ linux-2.6-ptracefix/arch/m68knommu/kernel/ptrace.c	2004-08-30 18:20:03.000000000 -0700
@@ -133,13 +133,6 @@ asmlinkage int sys_ptrace(long request, 
 		ret = ptrace_attach(child);
 		goto out_tsk;
 	}
-	ret = -ESRCH;
-	if (!(child->ptrace & PT_PTRACED))
-		goto out_tsk;
-	if (child->state != TASK_STOPPED) {
-		if (request != PTRACE_KILL)
-			goto out_tsk;
-	}
 	ret = ptrace_check_attach(child, request == PTRACE_KILL);
 	if (ret < 0)
 		goto out_tsk;
@@ -376,10 +369,8 @@ asmlinkage void syscall_trace(void)
 		return;
 	if (!(current->ptrace & PT_PTRACED))
 		return;
-	current->exit_code = SIGTRAP;
-	current->state = TASK_STOPPED;
-	notify_parent(current, SIGCHLD);
-	schedule();
+	ptrace_notify(SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
+				 ? 0x80 : 0));
 	/*
 	 * this isn't the same as continuing with a signal, but it will do
 	 * for normal use.  strace only continues with a signal if the
diff -rBzpu linux-2.6.9/arch/parisc/kernel/ptrace.c linux-2.6-ptracefix/arch/parisc/kernel/ptrace.c
--- linux-2.6.9/arch/parisc/kernel/ptrace.c	2004-08-30 20:10:50.000000000 -0700
+++ linux-2.6-ptracefix/arch/parisc/kernel/ptrace.c	2004-08-30 18:20:03.000000000 -0700
@@ -404,11 +404,8 @@ void syscall_trace(void)
 		return;
 	if (!(current->ptrace & PT_PTRACED))
 		return;
-	current->exit_code = SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
-					? 0x80 : 0);
-	current->state = TASK_STOPPED;
-	notify_parent(current, SIGCHLD);
-	schedule();
+	ptrace_notify(SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
+				 ? 0x80 : 0));
 	/*
 	 * this isn't the same as continuing with a signal, but it will do
 	 * for normal use.  strace only continues with a signal if the
diff -rBzpu linux-2.6.9/arch/sh64/kernel/ptrace.c linux-2.6-ptracefix/arch/sh64/kernel/ptrace.c
--- linux-2.6.9/arch/sh64/kernel/ptrace.c	2004-08-30 20:10:50.000000000 -0700
+++ linux-2.6-ptracefix/arch/sh64/kernel/ptrace.c	2004-08-30 18:20:03.000000000 -0700
@@ -311,11 +311,8 @@ asmlinkage void syscall_trace(void)
 	if (!(tsk->ptrace & PT_PTRACED))
 		return;
 
-	tsk->exit_code = SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
-				    ? 0x80 : 0);
-	tsk->state = TASK_STOPPED;
-	notify_parent(tsk, SIGCHLD);
-	schedule();
+	ptrace_notify(SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
+				 ? 0x80 : 0));
 	/*
 	 * this isn't the same as continuing with a signal, but it will do
 	 * for normal use.  strace only continues with a signal if the
diff -rBzpu linux-2.6.9/arch/sparc/kernel/ptrace.c linux-2.6-ptracefix/arch/sparc/kernel/ptrace.c
--- linux-2.6.9/arch/sparc/kernel/ptrace.c	2004-08-30 20:10:50.000000000 -0700
+++ linux-2.6-ptracefix/arch/sparc/kernel/ptrace.c	2004-08-30 18:20:04.000000000 -0700
@@ -614,12 +614,9 @@ asmlinkage void syscall_trace(void)
 		return;
 	if (!(current->ptrace & PT_PTRACED))
 		return;
-	current->exit_code = SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
-					? 0x80 : 0);
-	current->state = TASK_STOPPED;
 	current->thread.flags ^= MAGIC_CONSTANT;
-	notify_parent(current, SIGCHLD);
-	schedule();
+	ptrace_notify(SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
+				 ? 0x80 : 0));
 	/*
 	 * this isn't the same as continuing with a signal, but it will do
 	 * for normal use.  strace only continues with a signal if the
diff -rBzpu linux-2.6.9/arch/sparc64/kernel/ptrace.c linux-2.6-ptracefix/arch/sparc64/kernel/ptrace.c
--- linux-2.6.9/arch/sparc64/kernel/ptrace.c	2004-08-30 20:10:50.000000000 -0700
+++ linux-2.6-ptracefix/arch/sparc64/kernel/ptrace.c	2004-08-30 18:20:04.000000000 -0700
@@ -627,11 +627,8 @@ asmlinkage void syscall_trace(void)
 		return;
 	if (!(current->ptrace & PT_PTRACED))
 		return;
-	current->exit_code = SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
-					? 0x80 : 0);
-	current->state = TASK_STOPPED;
-	notify_parent(current, SIGCHLD);
-	schedule();
+	ptrace_notify(SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
+				 ? 0x80 : 0));
 
 	/*
 	 * this isn't the same as continuing with a signal, but it will do
diff -rBzpu linux-2.6.9/arch/v850/kernel/ptrace.c linux-2.6-ptracefix/arch/v850/kernel/ptrace.c
--- linux-2.6.9/arch/v850/kernel/ptrace.c	2004-08-30 20:10:50.000000000 -0700
+++ linux-2.6-ptracefix/arch/v850/kernel/ptrace.c	2004-08-30 18:20:04.000000000 -0700
@@ -147,14 +147,8 @@ int sys_ptrace(long request, long pid, l
 		rval = ptrace_attach(child);
 		goto out_tsk;
 	}
-	rval = -ESRCH;
-	if (!(child->ptrace & PT_PTRACED))
-		goto out_tsk;
-	if (child->state != TASK_STOPPED) {
-		if (request != PTRACE_KILL)
-			goto out_tsk;
-	}
-	if (child->parent != current)
+	ret = ptrace_check_attach(child, request == PTRACE_KILL);
+	if (ret < 0)
 		goto out_tsk;
 
 	switch (request) {
@@ -269,11 +263,8 @@ asmlinkage void syscall_trace(void)
 		return;
 	/* The 0x80 provides a way for the tracing parent to distinguish
 	   between a syscall stop and SIGTRAP delivery */
-	current->exit_code = SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
-					? 0x80 : 0);
-	current->state = TASK_STOPPED;
-	notify_parent(current, SIGCHLD);
-	schedule();
+	ptrace_notify(SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
+				 ? 0x80 : 0));
 	/*
 	 * this isn't the same as continuing with a signal, but it will do
 	 * for normal use.  strace only continues with a signal if the
diff -rBzpu linux-2.6.9/fs/proc/array.c linux-2.6-ptracefix/fs/proc/array.c
--- linux-2.6.9/fs/proc/array.c	2004-08-30 20:10:50.000000000 -0700
+++ linux-2.6-ptracefix/fs/proc/array.c	2004-08-30 18:43:29.000000000 -0700
@@ -130,8 +130,9 @@ static const char *task_state_array[] = 
 	"S (sleeping)",		/*  1 */
 	"D (disk sleep)",	/*  2 */
 	"T (stopped)",		/*  4 */
-	"Z (zombie)",		/*  8 */
-	"X (dead)"		/* 16 */
+	"T (tracing stop)",	/*  8 */
+	"Z (zombie)",		/* 16 */
+	"X (dead)"		/* 32 */
 };
 
 static inline const char * get_task_state(struct task_struct *tsk)
@@ -141,7 +142,8 @@ static inline const char * get_task_stat
 					   TASK_UNINTERRUPTIBLE |
 					   TASK_ZOMBIE |
 					   TASK_DEAD |
-					   TASK_STOPPED);
+					   TASK_STOPPED |
+					   TASK_TRACED);
 	const char **p = &task_state_array[0];
 
 	while (state) {
diff -rBzpu linux-2.6.9/fs/proc/base.c linux-2.6-ptracefix/fs/proc/base.c
--- linux-2.6.9/fs/proc/base.c	2004-08-30 20:10:50.000000000 -0700
+++ linux-2.6-ptracefix/fs/proc/base.c	2004-08-30 18:20:05.000000000 -0700
@@ -287,7 +287,8 @@ static int proc_root_link(struct inode *
 #define MAY_PTRACE(task) \
 	(task == current || \
 	(task->parent == current && \
-	(task->ptrace & PT_PTRACED) &&  task->state == TASK_STOPPED && \
+	(task->ptrace & PT_PTRACED) && \
+	 (task->state == TASK_STOPPED || task->state == TASK_TRACED) && \
 	 security_ptrace(current,task) == 0))
 
 static int may_ptrace_attach(struct task_struct *task)
diff -rBzpu linux-2.6.9/include/linux/sched.h linux-2.6-ptracefix/include/linux/sched.h
--- linux-2.6.9/include/linux/sched.h	2004-08-30 20:10:50.000000000 -0700
+++ linux-2.6-ptracefix/include/linux/sched.h	2004-08-30 18:20:05.000000000 -0700
@@ -106,8 +106,9 @@ extern unsigned long nr_iowait(void);
 #define TASK_INTERRUPTIBLE	1
 #define TASK_UNINTERRUPTIBLE	2
 #define TASK_STOPPED		4
-#define TASK_ZOMBIE		8
-#define TASK_DEAD		16
+#define TASK_TRACED		8
+#define TASK_ZOMBIE		16
+#define TASK_DEAD		32
 
 #define __set_task_state(tsk, state_value)		\
 	do { (tsk)->state = (state_value); } while (0)
@@ -725,7 +726,6 @@ extern int __kill_pg_info(int sig, struc
 extern int kill_pg_info(int, struct siginfo *, pid_t);
 extern int kill_sl_info(int, struct siginfo *, pid_t);
 extern int kill_proc_info(int, struct siginfo *, pid_t);
-extern void notify_parent(struct task_struct *, int);
 extern void do_notify_parent(struct task_struct *, int);
 extern void force_sig(int, struct task_struct *);
 extern void force_sig_specific(int, struct task_struct *);
diff -rBzpu linux-2.6.9/kernel/exit.c linux-2.6-ptracefix/kernel/exit.c
--- linux-2.6.9/kernel/exit.c	2004-08-30 20:57:39.220151522 -0700
+++ linux-2.6-ptracefix/kernel/exit.c	2004-08-30 19:50:16.000000000 -0700
@@ -566,6 +566,14 @@ static inline void reparent_thread(task_
 		if (p->state == TASK_ZOMBIE && p->exit_signal != -1 &&
 		    thread_group_empty(p))
 			do_notify_parent(p, p->exit_signal);
+		else if (p->state == TASK_TRACED) {
+			/*
+			 * If it was at a trace stop, turn it into
+			 * a normal stop since it's no longer being
+			 * traced.
+			 */
+			p->state = TASK_STOPPED;
+		}
 	}
 
 	/*
@@ -1065,7 +1073,7 @@ static int wait_task_stopped(task_t *p, 
 	 * race with the TASK_ZOMBIE case.
 	 */
 	exit_code = xchg(&p->exit_code, 0);
-	if (unlikely(p->state > TASK_STOPPED)) {
+	if (unlikely(p->state >= TASK_ZOMBIE)) {
 		/*
 		 * The task resumed and then died.  Let the next iteration
 		 * catch it in TASK_ZOMBIE.  Note that exit_code might
@@ -1133,6 +1141,10 @@ repeat:
 			flag = 1;
 
 			switch (p->state) {
+			case TASK_TRACED:
+				if (!(p->ptrace & PT_PTRACED))
+					continue;
+				/*FALLTHROUGH*/
 			case TASK_STOPPED:
 				if (!(options & WUNTRACED) &&
 				    !(p->ptrace & PT_PTRACED))
diff -rBzpu linux-2.6.9/kernel/power/process.c linux-2.6-ptracefix/kernel/power/process.c
--- linux-2.6.9/kernel/power/process.c	2004-08-30 20:10:51.000000000 -0700
+++ linux-2.6-ptracefix/kernel/power/process.c	2004-08-30 18:20:05.000000000 -0700
@@ -25,7 +25,8 @@ static inline int freezeable(struct task
 	    (p->flags & PF_NOFREEZE) ||
 	    (p->state == TASK_ZOMBIE) ||
 	    (p->state == TASK_DEAD) ||
-	    (p->state == TASK_STOPPED))
+	    (p->state == TASK_STOPPED) ||
+	    (p->state == TASK_TRACED))
 		return 0;
 	return 1;
 }
@@ -70,6 +71,7 @@ int freeze_processes(void)
 			if (!freezeable(p))
 				continue;
 			if ((p->flags & PF_FROZEN) ||
+			    (p->state == TASK_TRACED) ||
 			    (p->state == TASK_STOPPED))
 				continue;
 
diff -rBzpu linux-2.6.9/kernel/ptrace.c linux-2.6-ptracefix/kernel/ptrace.c
--- linux-2.6.9/kernel/ptrace.c	2004-08-30 20:10:51.000000000 -0700
+++ linux-2.6-ptracefix/kernel/ptrace.c	2004-08-30 19:35:34.000000000 -0700
@@ -55,6 +55,15 @@ void __ptrace_unlink(task_t *child)
 	REMOVE_LINKS(child);
 	child->parent = child->real_parent;
 	SET_LINKS(child);
+
+	if (child->state == TASK_TRACED) {
+		/*
+		 * Turn a tracing stop into a normal stop now,
+		 * since with no tracer there would be no way
+		 * to wake it up with SIGCONT or SIGKILL.
+		 */
+		child->state = TASK_STOPPED;
+	}
 }
 
 /*
@@ -62,20 +71,28 @@ void __ptrace_unlink(task_t *child)
  */
 int ptrace_check_attach(struct task_struct *child, int kill)
 {
-	if (!(child->ptrace & PT_PTRACED))
-		return -ESRCH;
+	int ret = -ESRCH;
 
-	if (child->parent != current)
-		return -ESRCH;
+	/*
+	 * We take the read lock around doing both checks to close a
+	 * possible race where someone else was tracing our child and
+	 * detached between these two checks.  After this locked check,
+	 * we are sure that this is our traced child and that can only
+	 * be changed by us so it's not changing right after this.
+	 */
+	read_lock(&tasklist_lock);
+	if ((child->ptrace & PT_PTRACED) && child->parent == current)
+		ret = 0;
+	read_unlock(&tasklist_lock);
 
-	if (!kill) {
-		if (child->state != TASK_STOPPED)
+	if (!ret && !kill) {
+		if (child->state != TASK_TRACED)
 			return -ESRCH;
 		wait_task_inactive(child);
 	}
 
 	/* All systems go.. */
-	return 0;
+	return ret;
 }
 
 int ptrace_attach(struct task_struct *task)
@@ -281,15 +298,13 @@ static int ptrace_setoptions(struct task
 
 static int ptrace_getsiginfo(struct task_struct *child, siginfo_t __user * data)
 {
-	if (child->last_siginfo == NULL)
-		return -EINVAL;
+	BUG_ON(child->last_siginfo == NULL);
 	return copy_siginfo_to_user(data, child->last_siginfo);
 }
 
 static int ptrace_setsiginfo(struct task_struct *child, siginfo_t __user * data)
 {
-	if (child->last_siginfo == NULL)
-		return -EINVAL;
+	BUG_ON(child->last_siginfo == NULL);
 	if (copy_from_user(child->last_siginfo, data, sizeof (siginfo_t)) != 0)
 		return -EFAULT;
 	return 0;
@@ -322,24 +337,3 @@ int ptrace_request(struct task_struct *c
 
 	return ret;
 }
-
-void ptrace_notify(int exit_code)
-{
-	BUG_ON (!(current->ptrace & PT_PTRACED));
-
-	/* Let the debugger run.  */
-	current->exit_code = exit_code;
-	set_current_state(TASK_STOPPED);
-	notify_parent(current, SIGCHLD);
-	schedule();
-
-	/*
-	 * Signals sent while we were stopped might set TIF_SIGPENDING.
-	 */
-
-	spin_lock_irq(&current->sighand->siglock);
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
-}
-
-EXPORT_SYMBOL(ptrace_notify);
diff -rBzpu linux-2.6.9/kernel/sched.c linux-2.6-ptracefix/kernel/sched.c
--- linux-2.6.9/kernel/sched.c	2004-08-30 20:10:51.000000000 -0700
+++ linux-2.6-ptracefix/kernel/sched.c	2004-08-30 18:58:34.000000000 -0700
@@ -1258,7 +1258,7 @@ out:
 
 int fastcall wake_up_process(task_t * p)
 {
-	return try_to_wake_up(p, TASK_STOPPED |
+	return try_to_wake_up(p, TASK_STOPPED | TASK_TRACED |
 		       		 TASK_INTERRUPTIBLE | TASK_UNINTERRUPTIBLE, 0);
 }
 
@@ -3670,7 +3670,7 @@ static void show_task(task_t * p)
 	task_t *relative;
 	unsigned state;
 	unsigned long free = 0;
-	static const char *stat_nam[] = { "R", "S", "D", "T", "Z", "W" };
+	static const char *stat_nam[] = { "R", "S", "D", "T", "t", "Z", "X" };
 
 	printk("%-13.13s ", p->comm);
 	state = p->state ? __ffs(p->state) + 1 : 0;
diff -rBzpu linux-2.6.9/kernel/signal.c linux-2.6-ptracefix/kernel/signal.c
--- linux-2.6.9/kernel/signal.c	2004-08-30 20:10:51.000000000 -0700
+++ linux-2.6-ptracefix/kernel/signal.c	2004-08-30 21:07:40.918136488 -0700
@@ -618,7 +618,8 @@ static int check_kill_permission(int sig
 
 /* forward decl */
 static void do_notify_parent_cldstop(struct task_struct *tsk,
-				     struct task_struct *parent);
+				     struct task_struct *parent,
+				     int why);
 
 /*
  * Handle magic process-wide effects of stop/continue signals.
@@ -661,11 +662,13 @@ static void handle_stop_signal(int sig, 
 			 */
 			p->signal->group_stop_count = 0;
 			if (p->ptrace & PT_PTRACED)
-				do_notify_parent_cldstop(p, p->parent);
+				do_notify_parent_cldstop(p, p->parent,
+							 CLD_STOPPED);
 			else
 				do_notify_parent_cldstop(
 					p->group_leader,
-					p->group_leader->real_parent);
+					p->group_leader->real_parent,
+					CLD_STOPPED);
 		}
 		rm_from_queue(SIG_KERNEL_STOP_MASK, &p->signal->shared_pending);
 		t = p;
@@ -861,11 +864,20 @@ force_sig_specific(int sig, struct task_
 
 
 static void
-__group_complete_signal(int sig, struct task_struct *p, unsigned int mask)
+__group_complete_signal(int sig, struct task_struct *p)
 {
+	unsigned int mask;
 	struct task_struct *t;
 
 	/*
+	 * Don't bother zombies and stopped tasks (but
+	 * SIGKILL will punch through stopped state)
+	 */
+	mask = TASK_DEAD | TASK_ZOMBIE | TASK_TRACED;
+	if (sig != SIGKILL)
+		mask |= TASK_STOPPED;
+
+	/*
 	 * Now find a thread we can wake up to take the signal off the queue.
 	 *
 	 * If the main thread wants the signal, it gets first crack.
@@ -966,7 +978,6 @@ __group_complete_signal(int sig, struct 
 static int
 __group_send_sig_info(int sig, struct siginfo *info, struct task_struct *p)
 {
-	unsigned int mask;
 	int ret = 0;
 
 #ifdef CONFIG_SMP
@@ -990,14 +1001,6 @@ __group_send_sig_info(int sig, struct si
 		return ret;
 
 	/*
-	 * Don't bother zombies and stopped tasks (but
-	 * SIGKILL will punch through stopped state)
-	 */
-	mask = TASK_DEAD | TASK_ZOMBIE;
-	if (sig != SIGKILL)
-		mask |= TASK_STOPPED;
-
-	/*
 	 * Put this signal on the shared-pending queue, or fail with EAGAIN.
 	 * We always use the shared queue for process-wide signals,
 	 * to avoid several races.
@@ -1006,7 +1009,7 @@ __group_send_sig_info(int sig, struct si
 	if (unlikely(ret))
 		return ret;
 
-	__group_complete_signal(sig, p, mask);
+	__group_complete_signal(sig, p);
 	return 0;
 }
 
@@ -1367,7 +1370,6 @@ int
 send_group_sigqueue(int sig, struct sigqueue *q, struct task_struct *p)
 {
 	unsigned long flags;
-	unsigned int mask;
 	int ret = 0;
 
 	BUG_ON(!(q->flags & SIGQUEUE_PREALLOC));
@@ -1392,13 +1394,6 @@ send_group_sigqueue(int sig, struct sigq
 		q->info.si_overrun++;
 		goto out;
 	} 
-	/*
-	 * Don't bother zombies and stopped tasks (but
-	 * SIGKILL will punch through stopped state)
-	 */
-	mask = TASK_DEAD | TASK_ZOMBIE;
-	if (sig != SIGKILL)
-		mask |= TASK_STOPPED;
 
 	/*
 	 * Put this signal on the shared-pending queue.
@@ -1409,7 +1404,7 @@ send_group_sigqueue(int sig, struct sigq
 	list_add_tail(&q->list, &p->signal->shared_pending.list);
 	sigaddset(&p->signal->shared_pending.signal, sig);
 
-	__group_complete_signal(sig, p, mask);
+	__group_complete_signal(sig, p);
 out:
 	spin_unlock_irqrestore(&p->sighand->siglock, flags);
 	read_unlock(&tasklist_lock);
@@ -1442,21 +1437,24 @@ static void __wake_up_parent(struct task
 }
 
 /*
- * Let a parent know about a status change of a child.
+ * Let a parent know about the death of a child.
+ * For a stopped/continued status change, use do_notify_parent_cldstop instead.
  */
 
 void do_notify_parent(struct task_struct *tsk, int sig)
 {
 	struct siginfo info;
 	unsigned long flags;
-	int why, status;
 	struct sighand_struct *psig;
 
 	if (sig == -1)
 		BUG();
 
-	BUG_ON(tsk->group_leader != tsk && tsk->group_leader->state != TASK_ZOMBIE && !tsk->ptrace);
-	BUG_ON(tsk->group_leader == tsk && !thread_group_empty(tsk) && !tsk->ptrace);
+	/* do_notify_parent_cldstop should have been called instead.  */
+	BUG_ON(tsk->state & (TASK_STOPPED|TASK_TRACED));
+
+	BUG_ON(!tsk->ptrace &&
+	       (tsk->group_leader != tsk || !thread_group_empty(tsk)));
 
 	info.si_signo = sig;
 	info.si_errno = 0;
@@ -1467,34 +1465,19 @@ void do_notify_parent(struct task_struct
 	info.si_utime = tsk->utime;
 	info.si_stime = tsk->stime;
 
-	status = tsk->exit_code & 0x7f;
-	why = SI_KERNEL;	/* shouldn't happen */
-	switch (tsk->state) {
-	case TASK_STOPPED:
-		/* FIXME -- can we deduce CLD_TRAPPED or CLD_CONTINUED? */
-		if (tsk->ptrace & PT_PTRACED)
-			why = CLD_TRAPPED;
-		else
-			why = CLD_STOPPED;
-		break;
-
-	default:
-		if (tsk->exit_code & 0x80)
-			why = CLD_DUMPED;
-		else if (tsk->exit_code & 0x7f)
-			why = CLD_KILLED;
-		else {
-			why = CLD_EXITED;
-			status = tsk->exit_code >> 8;
-		}
-		break;
+	info.si_status = tsk->exit_code & 0x7f;
+	if (tsk->exit_code & 0x80)
+		info.si_code = CLD_DUMPED;
+	else if (tsk->exit_code & 0x7f)
+		info.si_code = CLD_KILLED;
+	else {
+		info.si_code = CLD_EXITED;
+		info.si_status = tsk->exit_code >> 8;
 	}
-	info.si_code = why;
-	info.si_status = status;
 
 	psig = tsk->parent->sighand;
 	spin_lock_irqsave(&psig->siglock, flags);
-	if (sig == SIGCHLD && tsk->state != TASK_STOPPED &&
+	if (sig == SIGCHLD &&
 	    (psig->action[SIGCHLD-1].sa.sa_handler == SIG_IGN ||
 	     (psig->action[SIGCHLD-1].sa.sa_flags & SA_NOCLDWAIT))) {
 		/*
@@ -1522,26 +1505,9 @@ void do_notify_parent(struct task_struct
 	spin_unlock_irqrestore(&psig->siglock, flags);
 }
 
-
-/*
- * We need the tasklist lock because it's the only
- * thing that protects out "parent" pointer.
- *
- * exit.c calls "do_notify_parent()" directly, because
- * it already has the tasklist lock.
- */
-void
-notify_parent(struct task_struct *tsk, int sig)
-{
-	if (sig != -1) {
-		read_lock(&tasklist_lock);
-		do_notify_parent(tsk, sig);
-		read_unlock(&tasklist_lock);
-	}
-}
-
 static void
-do_notify_parent_cldstop(struct task_struct *tsk, struct task_struct *parent)
+do_notify_parent_cldstop(struct task_struct *tsk, struct task_struct *parent,
+			 int why)
 {
 	struct siginfo info;
 	unsigned long flags;
@@ -1556,8 +1522,20 @@ do_notify_parent_cldstop(struct task_str
 	info.si_utime = tsk->utime;
 	info.si_stime = tsk->stime;
 
-	info.si_status = tsk->exit_code & 0x7f;
-	info.si_code = CLD_STOPPED;
+	info.si_code = why;
+	switch (why) {
+	case CLD_CONTINUED:
+		info.si_status = SIGCONT;
+		break;
+	case CLD_STOPPED:
+		info.si_status = tsk->signal->group_exit_code & 0x7f;
+		break;
+	case CLD_TRAPPED:
+		info.si_status = tsk->exit_code & 0x7f;
+		break;
+	default:
+		BUG();
+	}
 
 	sighand = parent->sighand;
 	spin_lock_irqsave(&sighand->siglock, flags);
@@ -1571,6 +1549,68 @@ do_notify_parent_cldstop(struct task_str
 	spin_unlock_irqrestore(&sighand->siglock, flags);
 }
 
+/*
+ * This must be called with current->sighand->siglock held.
+ *
+ * This should be the path for all ptrace stops.
+ * We always set current->last_siginfo while stopped here.
+ * That makes it a way to test a stopped process for
+ * being ptrace-stopped vs being job-control-stopped.
+ */
+static void ptrace_stop(int exit_code, siginfo_t *info)
+{
+	BUG_ON(!(current->ptrace & PT_PTRACED));
+
+	/*
+	 * If there is a group stop in progress,
+	 * we must participate in the bookkeeping.
+	 */
+	if (current->signal->group_stop_count > 0)
+		--current->signal->group_stop_count;
+
+	current->last_siginfo = info;
+	current->exit_code = exit_code;
+
+	/* Let the debugger run.  */
+	set_current_state(TASK_TRACED);
+	spin_unlock_irq(&current->sighand->siglock);
+	read_lock(&tasklist_lock);
+	do_notify_parent_cldstop(current, current->parent, CLD_TRAPPED);
+	read_unlock(&tasklist_lock);
+	schedule();
+
+	/*
+	 * We are back.  Now reacquire the siglock before touching
+	 * last_siginfo, so that we are sure to have synchronized with
+	 * any signal-sending on another CPU that wants to examine it.
+	 */
+	spin_lock_irq(&current->sighand->siglock);
+	current->last_siginfo = NULL;
+
+	/*
+	 * Queued signals ignored us while we were stopped for tracing.
+	 * So check for any that we should take before resuming user mode.
+	 */
+	recalc_sigpending();
+}
+
+void ptrace_notify(int exit_code)
+{
+	siginfo_t info;
+
+	BUG_ON((exit_code & (0x7f | ~0xffff)) != SIGTRAP);
+
+	memset(&info, 0, sizeof info);
+	info.si_signo = SIGTRAP;
+	info.si_code = exit_code;
+	info.si_pid = current->pid;
+	info.si_uid = current->uid;
+
+	/* Let the debugger run.  */
+	spin_lock_irq(&current->sighand->siglock);
+	ptrace_stop(exit_code, &info);
+	spin_unlock_irq(&current->sighand->siglock);
+}
 
 #ifndef HAVE_ARCH_GET_SIGNAL_TO_DELIVER
 
@@ -1584,13 +1624,15 @@ finish_stop(int stop_count)
 	 */
 	if (stop_count < 0 || (current->ptrace & PT_PTRACED)) {
 		read_lock(&tasklist_lock);
-		do_notify_parent_cldstop(current, current->parent);
+		do_notify_parent_cldstop(current, current->parent,
+					 CLD_STOPPED);
 		read_unlock(&tasklist_lock);
 	}
 	else if (stop_count == 0) {
 		read_lock(&tasklist_lock);
 		do_notify_parent_cldstop(current->group_leader,
-					 current->group_leader->real_parent);
+					 current->group_leader->real_parent,
+					 CLD_STOPPED);
 		read_unlock(&tasklist_lock);
 	}
 
@@ -1629,7 +1671,7 @@ do_signal_stop(int signr)
 		/*
 		 * Lock must be held through transition to stopped state.
 		 */
-		current->exit_code = signr;
+		current->exit_code = current->signal->group_exit_code = signr;
 		set_current_state(TASK_STOPPED);
 		spin_unlock_irq(&sighand->siglock);
 	}
@@ -1766,25 +1808,10 @@ relock:
 		if ((current->ptrace & PT_PTRACED) && signr != SIGKILL) {
 			ptrace_signal_deliver(regs, cookie);
 
-			/*
-			 * If there is a group stop in progress,
-			 * we must participate in the bookkeeping.
-			 */
-			if (current->signal->group_stop_count > 0)
-				--current->signal->group_stop_count;
-
 			/* Let the debugger run.  */
-			current->exit_code = signr;
-			current->last_siginfo = info;
-			set_current_state(TASK_STOPPED);
-			spin_unlock_irq(&current->sighand->siglock);
-			notify_parent(current, SIGCHLD);
-			schedule();
-
-			current->last_siginfo = NULL;
+			ptrace_stop(signr, info);
 
 			/* We're back.  Did the debugger cancel the sig?  */
-			spin_lock_irq(&current->sighand->siglock);
 			signr = current->exit_code;
 			if (signr == 0)
 				continue;
@@ -1915,7 +1942,7 @@ EXPORT_SYMBOL(kill_proc);
 EXPORT_SYMBOL(kill_proc_info);
 EXPORT_SYMBOL(kill_sl);
 EXPORT_SYMBOL(kill_sl_info);
-EXPORT_SYMBOL(notify_parent);
+EXPORT_SYMBOL(ptrace_notify);
 EXPORT_SYMBOL(send_sig);
 EXPORT_SYMBOL(send_sig_info);
 EXPORT_SYMBOL(send_group_sig_info);

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] cleanup ptrace stops and remove notify_parent
  2004-08-31  4:11   ` Roland McGrath
@ 2004-08-31  4:26     ` Andrew Morton
  2004-08-31  5:43       ` Roland McGrath
  2004-08-31  4:27     ` Linus Torvalds
  2004-08-31  9:29     ` Christoph Hellwig
  2 siblings, 1 reply; 25+ messages in thread
From: Andrew Morton @ 2004-08-31  4:26 UTC (permalink / raw)
  To: Roland McGrath; +Cc: torvalds, linux-kernel

Roland McGrath <roland@redhat.com> wrote:
>
> > It clashes significantly with your waitid syscall patch.  Which one do you
>  > believe has priority?
> 
>  I of course already have a tree with both reconciled.  I did this ptrace
>  patch independently against Linus's tree since it was based on suggestions
>  from Linus and I thought he might like to try it and perhaps merge it
>  before my other patches in -mm go in.

Well given that waitid ha baked for a bit longer I guess we'd best make
this patch come afterwards.  So if you could redo this patch against
rc1-mm1, please?


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] cleanup ptrace stops and remove notify_parent
  2004-08-31  4:26     ` Andrew Morton
@ 2004-08-31  5:43       ` Roland McGrath
  0 siblings, 0 replies; 25+ messages in thread
From: Roland McGrath @ 2004-08-31  5:43 UTC (permalink / raw)
  To: Andrew Morton; +Cc: torvalds, linux-kernel

> Well given that waitid ha baked for a bit longer I guess we'd best make
> this patch come afterwards.  So if you could redo this patch against
> rc1-mm1, please?

Here is the TASK_TRACED patch against 2.6.9-rc1-mm1.


Thanks,
Roland



diff -rzBpu --exclude='.*' --exclude='*.o' linux-2.6.9-rc1-mm1/arch/arm/kernel/ptrace.c linux-2.6.9-rc1-mm1+TASK_TRACED/arch/arm/kernel/ptrace.c
--- linux-2.6.9-rc1-mm1/arch/arm/kernel/ptrace.c	2004-08-14 03:54:49.000000000 -0700
+++ linux-2.6.9-rc1-mm1+TASK_TRACED/arch/arm/kernel/ptrace.c	2004-08-30 22:02:55.000000000 -0700
@@ -792,11 +792,8 @@ asmlinkage void syscall_trace(int why, s
 
 	/* the 0x80 provides a way for the tracing parent to distinguish
 	   between a syscall stop and SIGTRAP delivery */
-	current->exit_code = SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
-					? 0x80 : 0);
-	current->state = TASK_STOPPED;
-	notify_parent(current, SIGCHLD);
-	schedule();
+	ptrace_notify(SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
+				 ? 0x80 : 0));
 	/*
 	 * this isn't the same as continuing with a signal, but it will do
 	 * for normal use.  strace only continues with a signal if the
diff -rzBpu --exclude='.*' --exclude='*.o' linux-2.6.9-rc1-mm1/arch/arm26/kernel/ptrace.c linux-2.6.9-rc1-mm1+TASK_TRACED/arch/arm26/kernel/ptrace.c
--- linux-2.6.9-rc1-mm1/arch/arm26/kernel/ptrace.c	2004-08-14 03:55:32.000000000 -0700
+++ linux-2.6.9-rc1-mm1+TASK_TRACED/arch/arm26/kernel/ptrace.c	2004-08-30 22:02:56.000000000 -0700
@@ -729,11 +729,8 @@ asmlinkage void syscall_trace(int why, s
 
 	/* the 0x80 provides a way for the tracing parent to distinguish
 	   between a syscall stop and SIGTRAP delivery */
-	current->exit_code = SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
-					? 0x80 : 0);
-	current->state = TASK_STOPPED;
-	notify_parent(current, SIGCHLD);
-	schedule();
+	ptrace_notify(SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
+				 ? 0x80 : 0));
 	/*
 	 * this isn't the same as continuing with a signal, but it will do
 	 * for normal use.  strace only continues with a signal if the
diff -rzBpu --exclude='.*' --exclude='*.o' linux-2.6.9-rc1-mm1/arch/cris/arch-v10/kernel/ptrace.c linux-2.6.9-rc1-mm1+TASK_TRACED/arch/cris/arch-v10/kernel/ptrace.c
--- linux-2.6.9-rc1-mm1/arch/cris/arch-v10/kernel/ptrace.c	2004-08-14 03:56:23.000000000 -0700
+++ linux-2.6.9-rc1-mm1+TASK_TRACED/arch/cris/arch-v10/kernel/ptrace.c	2004-08-30 22:02:56.000000000 -0700
@@ -85,17 +85,8 @@ sys_ptrace(long request, long pid, long 
 		goto out_tsk;
 	}
 	
-	ret = -ESRCH;
-	
-	if (!(child->ptrace & PT_PTRACED))
-		goto out_tsk;
-	
-	if (child->state != TASK_STOPPED) {
-		if (request != PTRACE_KILL)
-			goto out_tsk;
-	}
-	
-	if (child->parent != current)
+	ret = ptrace_check_attach(child, request == PTRACE_KILL);
+	if (ret < 0)
 		goto out_tsk;
 
 	switch (request) {
diff -rzBpu --exclude='.*' --exclude='*.o' linux-2.6.9-rc1-mm1/arch/h8300/kernel/ptrace.c linux-2.6.9-rc1-mm1+TASK_TRACED/arch/h8300/kernel/ptrace.c
--- linux-2.6.9-rc1-mm1/arch/h8300/kernel/ptrace.c	2004-08-14 03:55:10.000000000 -0700
+++ linux-2.6.9-rc1-mm1+TASK_TRACED/arch/h8300/kernel/ptrace.c	2004-08-30 22:02:56.000000000 -0700
@@ -89,13 +89,6 @@ asmlinkage int sys_ptrace(long request, 
 		ret = ptrace_attach(child);
 		goto out_tsk;
 	}
-	ret = -ESRCH;
-	if (!(child->ptrace & PT_PTRACED))
-		goto out_tsk;
-	if (child->state != TASK_STOPPED) {
-		if (request != PTRACE_KILL)
-			goto out_tsk;
-	}
 	ret = ptrace_check_attach(child, request == PTRACE_KILL);
 	if (ret < 0)
 		goto out_tsk;
@@ -270,10 +263,8 @@ asmlinkage void syscall_trace(void)
 		return;
 	if (!(current->ptrace & PT_PTRACED))
 		return;
-	current->exit_code = SIGTRAP;
-	current->state = TASK_STOPPED;
-	notify_parent(current, SIGCHLD);
-	schedule();
+	ptrace_notify(SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
+				 ? 0x80 : 0));
 	/*
 	 * this isn't the same as continuing with a signal, but it will do
 	 * for normal use.  strace only continues with a signal if the
diff -rzBpu --exclude='.*' --exclude='*.o' linux-2.6.9-rc1-mm1/arch/m68k/kernel/ptrace.c linux-2.6.9-rc1-mm1+TASK_TRACED/arch/m68k/kernel/ptrace.c
--- linux-2.6.9-rc1-mm1/arch/m68k/kernel/ptrace.c	2004-08-14 03:55:09.000000000 -0700
+++ linux-2.6.9-rc1-mm1+TASK_TRACED/arch/m68k/kernel/ptrace.c	2004-08-30 22:02:56.000000000 -0700
@@ -379,11 +379,8 @@ asmlinkage void syscall_trace(void)
 	if (!current->thread.work.delayed_trace &&
 	    !current->thread.work.syscall_trace)
 		return;
-	current->exit_code = SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
-					? 0x80 : 0);
-	current->state = TASK_STOPPED;
-	notify_parent(current, SIGCHLD);
-	schedule();
+	ptrace_notify(SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
+				 ? 0x80 : 0));
 	/*
 	 * this isn't the same as continuing with a signal, but it will do
 	 * for normal use.  strace only continues with a signal if the
diff -rzBpu --exclude='.*' --exclude='*.o' linux-2.6.9-rc1-mm1/arch/m68knommu/kernel/ptrace.c linux-2.6.9-rc1-mm1+TASK_TRACED/arch/m68knommu/kernel/ptrace.c
--- linux-2.6.9-rc1-mm1/arch/m68knommu/kernel/ptrace.c	2004-08-14 03:55:33.000000000 -0700
+++ linux-2.6.9-rc1-mm1+TASK_TRACED/arch/m68knommu/kernel/ptrace.c	2004-08-30 22:02:56.000000000 -0700
@@ -133,13 +133,6 @@ asmlinkage int sys_ptrace(long request, 
 		ret = ptrace_attach(child);
 		goto out_tsk;
 	}
-	ret = -ESRCH;
-	if (!(child->ptrace & PT_PTRACED))
-		goto out_tsk;
-	if (child->state != TASK_STOPPED) {
-		if (request != PTRACE_KILL)
-			goto out_tsk;
-	}
 	ret = ptrace_check_attach(child, request == PTRACE_KILL);
 	if (ret < 0)
 		goto out_tsk;
@@ -376,10 +369,8 @@ asmlinkage void syscall_trace(void)
 		return;
 	if (!(current->ptrace & PT_PTRACED))
 		return;
-	current->exit_code = SIGTRAP;
-	current->state = TASK_STOPPED;
-	notify_parent(current, SIGCHLD);
-	schedule();
+	ptrace_notify(SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
+				 ? 0x80 : 0));
 	/*
 	 * this isn't the same as continuing with a signal, but it will do
 	 * for normal use.  strace only continues with a signal if the
diff -rzBpu --exclude='.*' --exclude='*.o' linux-2.6.9-rc1-mm1/arch/parisc/kernel/ptrace.c linux-2.6.9-rc1-mm1+TASK_TRACED/arch/parisc/kernel/ptrace.c
--- linux-2.6.9-rc1-mm1/arch/parisc/kernel/ptrace.c	2004-08-14 03:54:50.000000000 -0700
+++ linux-2.6.9-rc1-mm1+TASK_TRACED/arch/parisc/kernel/ptrace.c	2004-08-30 22:02:56.000000000 -0700
@@ -404,11 +404,8 @@ void syscall_trace(void)
 		return;
 	if (!(current->ptrace & PT_PTRACED))
 		return;
-	current->exit_code = SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
-					? 0x80 : 0);
-	current->state = TASK_STOPPED;
-	notify_parent(current, SIGCHLD);
-	schedule();
+	ptrace_notify(SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
+				 ? 0x80 : 0));
 	/*
 	 * this isn't the same as continuing with a signal, but it will do
 	 * for normal use.  strace only continues with a signal if the
diff -rzBpu --exclude='.*' --exclude='*.o' linux-2.6.9-rc1-mm1/arch/sh64/kernel/ptrace.c linux-2.6.9-rc1-mm1+TASK_TRACED/arch/sh64/kernel/ptrace.c
--- linux-2.6.9-rc1-mm1/arch/sh64/kernel/ptrace.c	2004-08-14 03:55:47.000000000 -0700
+++ linux-2.6.9-rc1-mm1+TASK_TRACED/arch/sh64/kernel/ptrace.c	2004-08-30 22:02:56.000000000 -0700
@@ -311,11 +311,8 @@ asmlinkage void syscall_trace(void)
 	if (!(tsk->ptrace & PT_PTRACED))
 		return;
 
-	tsk->exit_code = SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
-				    ? 0x80 : 0);
-	tsk->state = TASK_STOPPED;
-	notify_parent(tsk, SIGCHLD);
-	schedule();
+	ptrace_notify(SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
+				 ? 0x80 : 0));
 	/*
 	 * this isn't the same as continuing with a signal, but it will do
 	 * for normal use.  strace only continues with a signal if the
diff -rzBpu --exclude='.*' --exclude='*.o' linux-2.6.9-rc1-mm1/arch/sparc/kernel/ptrace.c linux-2.6.9-rc1-mm1+TASK_TRACED/arch/sparc/kernel/ptrace.c
--- linux-2.6.9-rc1-mm1/arch/sparc/kernel/ptrace.c	2004-08-14 03:56:23.000000000 -0700
+++ linux-2.6.9-rc1-mm1+TASK_TRACED/arch/sparc/kernel/ptrace.c	2004-08-30 22:02:56.000000000 -0700
@@ -614,12 +614,9 @@ asmlinkage void syscall_trace(void)
 		return;
 	if (!(current->ptrace & PT_PTRACED))
 		return;
-	current->exit_code = SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
-					? 0x80 : 0);
-	current->state = TASK_STOPPED;
 	current->thread.flags ^= MAGIC_CONSTANT;
-	notify_parent(current, SIGCHLD);
-	schedule();
+	ptrace_notify(SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
+				 ? 0x80 : 0));
 	/*
 	 * this isn't the same as continuing with a signal, but it will do
 	 * for normal use.  strace only continues with a signal if the
diff -rzBpu --exclude='.*' --exclude='*.o' linux-2.6.9-rc1-mm1/arch/sparc64/kernel/ptrace.c linux-2.6.9-rc1-mm1+TASK_TRACED/arch/sparc64/kernel/ptrace.c
--- linux-2.6.9-rc1-mm1/arch/sparc64/kernel/ptrace.c	2004-08-14 03:56:23.000000000 -0700
+++ linux-2.6.9-rc1-mm1+TASK_TRACED/arch/sparc64/kernel/ptrace.c	2004-08-30 22:02:56.000000000 -0700
@@ -627,11 +627,8 @@ asmlinkage void syscall_trace(void)
 		return;
 	if (!(current->ptrace & PT_PTRACED))
 		return;
-	current->exit_code = SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
-					? 0x80 : 0);
-	current->state = TASK_STOPPED;
-	notify_parent(current, SIGCHLD);
-	schedule();
+	ptrace_notify(SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
+				 ? 0x80 : 0));
 
 	/*
 	 * this isn't the same as continuing with a signal, but it will do
diff -rzBpu --exclude='.*' --exclude='*.o' linux-2.6.9-rc1-mm1/arch/v850/kernel/ptrace.c linux-2.6.9-rc1-mm1+TASK_TRACED/arch/v850/kernel/ptrace.c
--- linux-2.6.9-rc1-mm1/arch/v850/kernel/ptrace.c	2004-08-14 03:55:32.000000000 -0700
+++ linux-2.6.9-rc1-mm1+TASK_TRACED/arch/v850/kernel/ptrace.c	2004-08-30 22:02:56.000000000 -0700
@@ -147,14 +147,8 @@ int sys_ptrace(long request, long pid, l
 		rval = ptrace_attach(child);
 		goto out_tsk;
 	}
-	rval = -ESRCH;
-	if (!(child->ptrace & PT_PTRACED))
-		goto out_tsk;
-	if (child->state != TASK_STOPPED) {
-		if (request != PTRACE_KILL)
-			goto out_tsk;
-	}
-	if (child->parent != current)
+	ret = ptrace_check_attach(child, request == PTRACE_KILL);
+	if (ret < 0)
 		goto out_tsk;
 
 	switch (request) {
@@ -269,11 +263,8 @@ asmlinkage void syscall_trace(void)
 		return;
 	/* The 0x80 provides a way for the tracing parent to distinguish
 	   between a syscall stop and SIGTRAP delivery */
-	current->exit_code = SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
-					? 0x80 : 0);
-	current->state = TASK_STOPPED;
-	notify_parent(current, SIGCHLD);
-	schedule();
+	ptrace_notify(SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
+				 ? 0x80 : 0));
 	/*
 	 * this isn't the same as continuing with a signal, but it will do
 	 * for normal use.  strace only continues with a signal if the
diff -rzBpu --exclude='.*' --exclude='*.o' linux-2.6.9-rc1-mm1/fs/proc/array.c linux-2.6.9-rc1-mm1+TASK_TRACED/fs/proc/array.c
--- linux-2.6.9-rc1-mm1/fs/proc/array.c	2004-08-27 13:46:35.000000000 -0700
+++ linux-2.6.9-rc1-mm1+TASK_TRACED/fs/proc/array.c	2004-08-30 22:02:56.000000000 -0700
@@ -130,8 +130,9 @@ static const char *task_state_array[] = 
 	"S (sleeping)",		/*  1 */
 	"D (disk sleep)",	/*  2 */
 	"T (stopped)",		/*  4 */
-	"Z (zombie)",		/*  8 */
-	"X (dead)"		/* 16 */
+	"T (tracing stop)",	/*  8 */
+	"Z (zombie)",		/* 16 */
+	"X (dead)"		/* 32 */
 };
 
 static inline const char * get_task_state(struct task_struct *tsk)
@@ -141,7 +142,8 @@ static inline const char * get_task_stat
 					   TASK_UNINTERRUPTIBLE |
 					   TASK_ZOMBIE |
 					   TASK_DEAD |
-					   TASK_STOPPED);
+					   TASK_STOPPED |
+					   TASK_TRACED);
 	const char **p = &task_state_array[0];
 
 	while (state) {
diff -rzBpu --exclude='.*' --exclude='*.o' linux-2.6.9-rc1-mm1/fs/proc/base.c linux-2.6.9-rc1-mm1+TASK_TRACED/fs/proc/base.c
--- linux-2.6.9-rc1-mm1/fs/proc/base.c	2004-08-27 13:46:35.000000000 -0700
+++ linux-2.6.9-rc1-mm1+TASK_TRACED/fs/proc/base.c	2004-08-30 22:02:56.000000000 -0700
@@ -300,7 +300,8 @@ static int proc_root_link(struct inode *
 #define MAY_PTRACE(task) \
 	(task == current || \
 	(task->parent == current && \
-	(task->ptrace & PT_PTRACED) &&  task->state == TASK_STOPPED && \
+	(task->ptrace & PT_PTRACED) && \
+	 (task->state == TASK_STOPPED || task->state == TASK_TRACED) && \
 	 security_ptrace(current,task) == 0))
 
 static int may_ptrace_attach(struct task_struct *task)
diff -rzBpu --exclude='.*' --exclude='*.o' linux-2.6.9-rc1-mm1/include/linux/sched.h linux-2.6.9-rc1-mm1+TASK_TRACED/include/linux/sched.h
--- linux-2.6.9-rc1-mm1/include/linux/sched.h	2004-08-27 13:46:37.000000000 -0700
+++ linux-2.6.9-rc1-mm1+TASK_TRACED/include/linux/sched.h	2004-08-30 22:02:56.657738337 -0700
@@ -107,8 +107,9 @@ extern unsigned long nr_iowait(void);
 #define TASK_INTERRUPTIBLE	1
 #define TASK_UNINTERRUPTIBLE	2
 #define TASK_STOPPED		4
-#define TASK_ZOMBIE		8
-#define TASK_DEAD		16
+#define TASK_TRACED		8
+#define TASK_ZOMBIE		16
+#define TASK_DEAD		32
 
 #define __set_task_state(tsk, state_value)		\
 	do { (tsk)->state = (state_value); } while (0)
@@ -758,7 +759,6 @@ extern int __kill_pg_info(int sig, struc
 extern int kill_pg_info(int, struct siginfo *, pid_t);
 extern int kill_sl_info(int, struct siginfo *, pid_t);
 extern int kill_proc_info(int, struct siginfo *, pid_t);
-extern void notify_parent(struct task_struct *, int);
 extern void do_notify_parent(struct task_struct *, int);
 extern void force_sig(int, struct task_struct *);
 extern void force_sig_specific(int, struct task_struct *);
diff -rzBpu --exclude='.*' --exclude='*.o' linux-2.6.9-rc1-mm1/kernel/exit.c linux-2.6.9-rc1-mm1+TASK_TRACED/kernel/exit.c
--- linux-2.6.9-rc1-mm1/kernel/exit.c	2004-08-27 13:46:37.000000000 -0700
+++ linux-2.6.9-rc1-mm1+TASK_TRACED/kernel/exit.c	2004-08-30 22:02:56.000000000 -0700
@@ -564,6 +564,14 @@ static inline void reparent_thread(task_
 		if (p->state == TASK_ZOMBIE && p->exit_signal != -1 &&
 		    thread_group_empty(p))
 			do_notify_parent(p, p->exit_signal);
+		else if (p->state == TASK_TRACED) {
+			/*
+			 * If it was at a trace stop, turn it into
+			 * a normal stop since it's no longer being
+			 * traced.
+			 */
+			p->state = TASK_STOPPED;
+		}
 	}
 
 	/*
@@ -1201,7 +1209,7 @@ static int wait_task_stopped(task_t *p, 
 	 * race with the TASK_ZOMBIE case.
 	 */
 	exit_code = xchg(&p->exit_code, 0);
-	if (unlikely(p->state > TASK_STOPPED)) {
+	if (unlikely(p->state >= TASK_ZOMBIE)) {
 		/*
 		 * The task resumed and then died.  Let the next iteration
 		 * catch it in TASK_ZOMBIE.  Note that exit_code might
@@ -1282,6 +1290,10 @@ repeat:
 			flag = 1;
 
 			switch (p->state) {
+			case TASK_TRACED:
+				if (!(p->ptrace & PT_PTRACED))
+					continue;
+				/*FALLTHROUGH*/
 			case TASK_STOPPED:
 				if (!(options & WUNTRACED) &&
 				    !(p->ptrace & PT_PTRACED))
diff -rzBpu --exclude='.*' --exclude='*.o' linux-2.6.9-rc1-mm1/kernel/power/process.c linux-2.6.9-rc1-mm1+TASK_TRACED/kernel/power/process.c
--- linux-2.6.9-rc1-mm1/kernel/power/process.c	2004-08-14 03:54:47.000000000 -0700
+++ linux-2.6.9-rc1-mm1+TASK_TRACED/kernel/power/process.c	2004-08-30 22:02:56.000000000 -0700
@@ -25,7 +25,8 @@ static inline int freezeable(struct task
 	    (p->flags & PF_NOFREEZE) ||
 	    (p->state == TASK_ZOMBIE) ||
 	    (p->state == TASK_DEAD) ||
-	    (p->state == TASK_STOPPED))
+	    (p->state == TASK_STOPPED) ||
+	    (p->state == TASK_TRACED))
 		return 0;
 	return 1;
 }
@@ -70,6 +71,7 @@ int freeze_processes(void)
 			if (!freezeable(p))
 				continue;
 			if ((p->flags & PF_FROZEN) ||
+			    (p->state == TASK_TRACED) ||
 			    (p->state == TASK_STOPPED))
 				continue;
 
diff -rzBpu --exclude='.*' --exclude='*.o' linux-2.6.9-rc1-mm1/kernel/ptrace.c linux-2.6.9-rc1-mm1+TASK_TRACED/kernel/ptrace.c
--- linux-2.6.9-rc1-mm1/kernel/ptrace.c	2004-08-14 03:56:24.000000000 -0700
+++ linux-2.6.9-rc1-mm1+TASK_TRACED/kernel/ptrace.c	2004-08-30 22:02:56.000000000 -0700
@@ -55,6 +55,15 @@ void __ptrace_unlink(task_t *child)
 	REMOVE_LINKS(child);
 	child->parent = child->real_parent;
 	SET_LINKS(child);
+
+	if (child->state == TASK_TRACED) {
+		/*
+		 * Turn a tracing stop into a normal stop now,
+		 * since with no tracer there would be no way
+		 * to wake it up with SIGCONT or SIGKILL.
+		 */
+		child->state = TASK_STOPPED;
+	}
 }
 
 /*
@@ -62,20 +71,28 @@ void __ptrace_unlink(task_t *child)
  */
 int ptrace_check_attach(struct task_struct *child, int kill)
 {
-	if (!(child->ptrace & PT_PTRACED))
-		return -ESRCH;
+	int ret = -ESRCH;
 
-	if (child->parent != current)
-		return -ESRCH;
+	/*
+	 * We take the read lock around doing both checks to close a
+	 * possible race where someone else was tracing our child and
+	 * detached between these two checks.  After this locked check,
+	 * we are sure that this is our traced child and that can only
+	 * be changed by us so it's not changing right after this.
+	 */
+	read_lock(&tasklist_lock);
+	if ((child->ptrace & PT_PTRACED) && child->parent == current)
+		ret = 0;
+	read_unlock(&tasklist_lock);
 
-	if (!kill) {
-		if (child->state != TASK_STOPPED)
+	if (!ret && !kill) {
+		if (child->state != TASK_TRACED)
 			return -ESRCH;
 		wait_task_inactive(child);
 	}
 
 	/* All systems go.. */
-	return 0;
+	return ret;
 }
 
 int ptrace_attach(struct task_struct *task)
@@ -281,15 +298,13 @@ static int ptrace_setoptions(struct task
 
 static int ptrace_getsiginfo(struct task_struct *child, siginfo_t __user * data)
 {
-	if (child->last_siginfo == NULL)
-		return -EINVAL;
+	BUG_ON(child->last_siginfo == NULL);
 	return copy_siginfo_to_user(data, child->last_siginfo);
 }
 
 static int ptrace_setsiginfo(struct task_struct *child, siginfo_t __user * data)
 {
-	if (child->last_siginfo == NULL)
-		return -EINVAL;
+	BUG_ON(child->last_siginfo == NULL);
 	if (copy_from_user(child->last_siginfo, data, sizeof (siginfo_t)) != 0)
 		return -EFAULT;
 	return 0;
@@ -322,24 +337,3 @@ int ptrace_request(struct task_struct *c
 
 	return ret;
 }
-
-void ptrace_notify(int exit_code)
-{
-	BUG_ON (!(current->ptrace & PT_PTRACED));
-
-	/* Let the debugger run.  */
-	current->exit_code = exit_code;
-	set_current_state(TASK_STOPPED);
-	notify_parent(current, SIGCHLD);
-	schedule();
-
-	/*
-	 * Signals sent while we were stopped might set TIF_SIGPENDING.
-	 */
-
-	spin_lock_irq(&current->sighand->siglock);
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
-}
-
-EXPORT_SYMBOL(ptrace_notify);
diff -rzBpu --exclude='.*' --exclude='*.o' linux-2.6.9-rc1-mm1/kernel/sched.c linux-2.6.9-rc1-mm1+TASK_TRACED/kernel/sched.c
--- linux-2.6.9-rc1-mm1/kernel/sched.c	2004-08-27 13:46:37.000000000 -0700
+++ linux-2.6.9-rc1-mm1+TASK_TRACED/kernel/sched.c	2004-08-30 22:03:27.000000000 -0700
@@ -1174,7 +1174,7 @@ out:
 
 int fastcall wake_up_process(task_t * p)
 {
-	return try_to_wake_up(p, TASK_STOPPED |
+	return try_to_wake_up(p, TASK_STOPPED | TASK_TRACED |
 				TASK_INTERRUPTIBLE | TASK_UNINTERRUPTIBLE, 0);
 }
 
@@ -3493,7 +3493,7 @@ static void show_task(task_t * p)
 	task_t *relative;
 	unsigned state;
 	unsigned long free = 0;
-	static const char *stat_nam[] = { "R", "S", "D", "T", "Z", "W" };
+	static const char *stat_nam[] = { "R", "S", "D", "T", "t", "Z", "X" };
 
 	printk("%-13.13s ", p->comm);
 	state = p->state ? __ffs(p->state) + 1 : 0;
diff -rzBpu --exclude='.*' --exclude='*.o' linux-2.6.9-rc1-mm1/kernel/signal.c linux-2.6.9-rc1-mm1+TASK_TRACED/kernel/signal.c
--- linux-2.6.9-rc1-mm1/kernel/signal.c	2004-08-27 13:46:37.000000000 -0700
+++ linux-2.6.9-rc1-mm1+TASK_TRACED/kernel/signal.c	2004-08-30 22:08:42.000000000 -0700
@@ -636,7 +636,8 @@ static int check_kill_permission(int sig
 
 /* forward decl */
 static void do_notify_parent_cldstop(struct task_struct *tsk,
-				     struct task_struct *parent);
+				     struct task_struct *parent,
+				     int why);
 
 /*
  * Handle magic process-wide effects of stop/continue signals.
@@ -681,11 +682,13 @@ static void handle_stop_signal(int sig, 
 			p->signal->stop_state = 1;
 			spin_unlock(&p->sighand->siglock);
 			if (p->ptrace & PT_PTRACED)
-				do_notify_parent_cldstop(p, p->parent);
+				do_notify_parent_cldstop(p, p->parent,
+							 CLD_STOPPED);
 			else
 				do_notify_parent_cldstop(
 					p->group_leader,
-					p->group_leader->real_parent);
+					p->group_leader->real_parent,
+							 CLD_STOPPED);
 			spin_lock(&p->sighand->siglock);
 		}
 		rm_from_queue(SIG_KERNEL_STOP_MASK, &p->signal->shared_pending);
@@ -727,11 +730,13 @@ static void handle_stop_signal(int sig, 
 			p->signal->group_exit_code = 0;
 			spin_unlock(&p->sighand->siglock);
 			if (p->ptrace & PT_PTRACED)
-				do_notify_parent_cldstop(p, p->parent);
+				do_notify_parent_cldstop(p, p->parent,
+							 CLD_CONTINUED);
 			else
 				do_notify_parent_cldstop(
 					p->group_leader,
-					p->group_leader->real_parent);
+					p->group_leader->real_parent,
+							 CLD_CONTINUED);
 			spin_lock(&p->sighand->siglock);
 		}
 	}
@@ -899,11 +904,20 @@ force_sig_specific(int sig, struct task_
 
 
 static void
-__group_complete_signal(int sig, struct task_struct *p, unsigned int mask)
+__group_complete_signal(int sig, struct task_struct *p)
 {
+	unsigned int mask;
 	struct task_struct *t;
 
 	/*
+	 * Don't bother zombies and stopped tasks (but
+	 * SIGKILL will punch through stopped state)
+	 */
+	mask = TASK_DEAD | TASK_ZOMBIE | TASK_TRACED;
+	if (sig != SIGKILL)
+		mask |= TASK_STOPPED;
+
+	/*
 	 * Now find a thread we can wake up to take the signal off the queue.
 	 *
 	 * If the main thread wants the signal, it gets first crack.
@@ -1004,7 +1018,6 @@ __group_complete_signal(int sig, struct 
 static int
 __group_send_sig_info(int sig, struct siginfo *info, struct task_struct *p)
 {
-	unsigned int mask;
 	int ret = 0;
 
 #ifdef CONFIG_SMP
@@ -1028,14 +1041,6 @@ __group_send_sig_info(int sig, struct si
 		return ret;
 
 	/*
-	 * Don't bother zombies and stopped tasks (but
-	 * SIGKILL will punch through stopped state)
-	 */
-	mask = TASK_DEAD | TASK_ZOMBIE;
-	if (sig != SIGKILL)
-		mask |= TASK_STOPPED;
-
-	/*
 	 * Put this signal on the shared-pending queue, or fail with EAGAIN.
 	 * We always use the shared queue for process-wide signals,
 	 * to avoid several races.
@@ -1044,7 +1049,7 @@ __group_send_sig_info(int sig, struct si
 	if (unlikely(ret))
 		return ret;
 
-	__group_complete_signal(sig, p, mask);
+	__group_complete_signal(sig, p);
 	return 0;
 }
 
@@ -1405,7 +1410,6 @@ int
 send_group_sigqueue(int sig, struct sigqueue *q, struct task_struct *p)
 {
 	unsigned long flags;
-	unsigned int mask;
 	int ret = 0;
 
 	BUG_ON(!(q->flags & SIGQUEUE_PREALLOC));
@@ -1430,13 +1434,6 @@ send_group_sigqueue(int sig, struct sigq
 		q->info.si_overrun++;
 		goto out;
 	} 
-	/*
-	 * Don't bother zombies and stopped tasks (but
-	 * SIGKILL will punch through stopped state)
-	 */
-	mask = TASK_DEAD | TASK_ZOMBIE;
-	if (sig != SIGKILL)
-		mask |= TASK_STOPPED;
 
 	/*
 	 * Put this signal on the shared-pending queue.
@@ -1447,7 +1444,7 @@ send_group_sigqueue(int sig, struct sigq
 	list_add_tail(&q->list, &p->signal->shared_pending.list);
 	sigaddset(&p->signal->shared_pending.signal, sig);
 
-	__group_complete_signal(sig, p, mask);
+	__group_complete_signal(sig, p);
 out:
 	spin_unlock_irqrestore(&p->sighand->siglock, flags);
 	read_unlock(&tasklist_lock);
@@ -1480,19 +1477,22 @@ static void __wake_up_parent(struct task
 }
 
 /*
- * Let a parent know about a status change of a child.
+ * Let a parent know about the death of a child.
+ * For a stopped/continued status change, use do_notify_parent_cldstop instead.
  */
 
 void do_notify_parent(struct task_struct *tsk, int sig)
 {
 	struct siginfo info;
 	unsigned long flags;
-	int why, status;
 	struct sighand_struct *psig;
 
 	if (sig == -1)
 		BUG();
 
+ 	/* do_notify_parent_cldstop should have been called instead.  */
+ 	BUG_ON(tsk->state & (TASK_STOPPED|TASK_TRACED));
+
 	BUG_ON(!tsk->ptrace &&
 	       (tsk->group_leader != tsk || !thread_group_empty(tsk)));
 
@@ -1506,34 +1506,19 @@ void do_notify_parent(struct task_struct
 	info.si_stime = tsk->stime + tsk->signal->stime;
 	k_getrusage(tsk, RUSAGE_BOTH, &info.si_rusage);
 
-	status = tsk->exit_code & 0x7f;
-	why = SI_KERNEL;	/* shouldn't happen */
-	switch (tsk->state) {
-	case TASK_STOPPED:
-		/* FIXME -- can we deduce CLD_TRAPPED or CLD_CONTINUED? */
-		if (tsk->ptrace & PT_PTRACED)
-			why = CLD_TRAPPED;
-		else
-			why = CLD_STOPPED;
-		break;
-
-	default:
-		if (tsk->exit_code & 0x80)
-			why = CLD_DUMPED;
-		else if (tsk->exit_code & 0x7f)
-			why = CLD_KILLED;
-		else {
-			why = CLD_EXITED;
-			status = tsk->exit_code >> 8;
-		}
-		break;
+	info.si_status = tsk->exit_code & 0x7f;
+	if (tsk->exit_code & 0x80)
+		info.si_code = CLD_DUMPED;
+	else if (tsk->exit_code & 0x7f)
+		info.si_code = CLD_KILLED;
+	else {
+		info.si_code = CLD_EXITED;
+		info.si_status = tsk->exit_code >> 8;
 	}
-	info.si_code = why;
-	info.si_status = status;
 
 	psig = tsk->parent->sighand;
 	spin_lock_irqsave(&psig->siglock, flags);
-	if (sig == SIGCHLD && tsk->state != TASK_STOPPED &&
+	if (sig == SIGCHLD &&
 	    (psig->action[SIGCHLD-1].sa.sa_handler == SIG_IGN ||
 	     (psig->action[SIGCHLD-1].sa.sa_flags & SA_NOCLDWAIT))) {
 		/*
@@ -1561,26 +1546,9 @@ void do_notify_parent(struct task_struct
 	spin_unlock_irqrestore(&psig->siglock, flags);
 }
 
-
-/*
- * We need the tasklist lock because it's the only
- * thing that protects out "parent" pointer.
- *
- * exit.c calls "do_notify_parent()" directly, because
- * it already has the tasklist lock.
- */
-void
-notify_parent(struct task_struct *tsk, int sig)
-{
-	if (sig != -1) {
-		read_lock(&tasklist_lock);
-		do_notify_parent(tsk, sig);
-		read_unlock(&tasklist_lock);
-	}
-}
-
 static void
-do_notify_parent_cldstop(struct task_struct *tsk, struct task_struct *parent)
+do_notify_parent_cldstop(struct task_struct *tsk, struct task_struct *parent,
+			 int why)
 {
 	struct siginfo info;
 	unsigned long flags;
@@ -1596,14 +1564,20 @@ do_notify_parent_cldstop(struct task_str
 	info.si_stime = tsk->stime;
 	k_getrusage(tsk, RUSAGE_BOTH, &info.si_rusage);
 
-	info.si_status = (tsk->signal ? tsk->signal->group_exit_code :
-			  tsk->exit_code) & 0x7f;
-	if (info.si_status == 0) {
-		info.si_status = SIGCONT;
-		info.si_code = CLD_CONTINUED;
-	} else {
-		info.si_code = CLD_STOPPED;
-	}
+ 	info.si_code = why;
+ 	switch (why) {
+ 	case CLD_CONTINUED:
+ 		info.si_status = SIGCONT;
+ 		break;
+ 	case CLD_STOPPED:
+ 		info.si_status = tsk->signal->group_exit_code & 0x7f;
+ 		break;
+ 	case CLD_TRAPPED:
+ 		info.si_status = tsk->exit_code & 0x7f;
+ 		break;
+ 	default:
+ 		BUG();
+ 	}
 
 	sighand = parent->sighand;
 	spin_lock_irqsave(&sighand->siglock, flags);
@@ -1617,6 +1591,68 @@ do_notify_parent_cldstop(struct task_str
 	spin_unlock_irqrestore(&sighand->siglock, flags);
 }
 
+/*
+ * This must be called with current->sighand->siglock held.
+ *
+ * This should be the path for all ptrace stops.
+ * We always set current->last_siginfo while stopped here.
+ * That makes it a way to test a stopped process for
+ * being ptrace-stopped vs being job-control-stopped.
+ */
+static void ptrace_stop(int exit_code, siginfo_t *info)
+{
+	BUG_ON(!(current->ptrace & PT_PTRACED));
+
+	/*
+	 * If there is a group stop in progress,
+	 * we must participate in the bookkeeping.
+	 */
+	if (current->signal->group_stop_count > 0)
+		--current->signal->group_stop_count;
+
+	current->last_siginfo = info;
+	current->exit_code = exit_code;
+
+	/* Let the debugger run.  */
+	set_current_state(TASK_TRACED);
+	spin_unlock_irq(&current->sighand->siglock);
+	read_lock(&tasklist_lock);
+	do_notify_parent_cldstop(current, current->parent, CLD_TRAPPED);
+	read_unlock(&tasklist_lock);
+	schedule();
+
+	/*
+	 * We are back.  Now reacquire the siglock before touching
+	 * last_siginfo, so that we are sure to have synchronized with
+	 * any signal-sending on another CPU that wants to examine it.
+	 */
+	spin_lock_irq(&current->sighand->siglock);
+	current->last_siginfo = NULL;
+
+	/*
+	 * Queued signals ignored us while we were stopped for tracing.
+	 * So check for any that we should take before resuming user mode.
+	 */
+	recalc_sigpending();
+}
+
+void ptrace_notify(int exit_code)
+{
+	siginfo_t info;
+
+	BUG_ON((exit_code & (0x7f | ~0xffff)) != SIGTRAP);
+
+	memset(&info, 0, sizeof info);
+	info.si_signo = SIGTRAP;
+	info.si_code = exit_code;
+	info.si_pid = current->pid;
+	info.si_uid = current->uid;
+
+	/* Let the debugger run.  */
+	spin_lock_irq(&current->sighand->siglock);
+	ptrace_stop(exit_code, &info);
+	spin_unlock_irq(&current->sighand->siglock);
+}
 
 #ifndef HAVE_ARCH_GET_SIGNAL_TO_DELIVER
 
@@ -1630,13 +1666,15 @@ finish_stop(int stop_count)
 	 */
 	if (stop_count < 0 || (current->ptrace & PT_PTRACED)) {
 		read_lock(&tasklist_lock);
-		do_notify_parent_cldstop(current, current->parent);
+		do_notify_parent_cldstop(current, current->parent,
+					 CLD_STOPPED);
 		read_unlock(&tasklist_lock);
 	}
 	else if (stop_count == 0) {
 		read_lock(&tasklist_lock);
 		do_notify_parent_cldstop(current->group_leader,
-					 current->group_leader->real_parent);
+					 current->group_leader->real_parent,
+					 CLD_STOPPED);
 		read_unlock(&tasklist_lock);
 	}
 
@@ -1819,25 +1857,10 @@ relock:
 		if ((current->ptrace & PT_PTRACED) && signr != SIGKILL) {
 			ptrace_signal_deliver(regs, cookie);
 
-			/*
-			 * If there is a group stop in progress,
-			 * we must participate in the bookkeeping.
-			 */
-			if (current->signal->group_stop_count > 0)
-				--current->signal->group_stop_count;
-
 			/* Let the debugger run.  */
-			current->exit_code = signr;
-			current->last_siginfo = info;
-			set_current_state(TASK_STOPPED);
-			spin_unlock_irq(&current->sighand->siglock);
-			notify_parent(current, SIGCHLD);
-			schedule();
-
-			current->last_siginfo = NULL;
+			ptrace_stop(signr, info);
 
 			/* We're back.  Did the debugger cancel the sig?  */
-			spin_lock_irq(&current->sighand->siglock);
 			signr = current->exit_code;
 			if (signr == 0)
 				continue;
@@ -1968,7 +1991,7 @@ EXPORT_SYMBOL(kill_proc);
 EXPORT_SYMBOL(kill_proc_info);
 EXPORT_SYMBOL(kill_sl);
 EXPORT_SYMBOL(kill_sl_info);
-EXPORT_SYMBOL(notify_parent);
+EXPORT_SYMBOL(ptrace_notify);
 EXPORT_SYMBOL(send_sig);
 EXPORT_SYMBOL(send_sig_info);
 EXPORT_SYMBOL(send_group_sig_info);

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] cleanup ptrace stops and remove notify_parent
  2004-08-31  4:11   ` Roland McGrath
  2004-08-31  4:26     ` Andrew Morton
@ 2004-08-31  4:27     ` Linus Torvalds
  2004-08-31  4:50       ` Roland McGrath
  2004-08-31 13:19       ` OGAWA Hirofumi
  2004-08-31  9:29     ` Christoph Hellwig
  2 siblings, 2 replies; 25+ messages in thread
From: Linus Torvalds @ 2004-08-31  4:27 UTC (permalink / raw)
  To: Roland McGrath; +Cc: Andrew Morton, Kernel Mailing List



On Mon, 30 Aug 2004, Roland McGrath wrote:
> 
> This adds a new state TASK_TRACED that is used in place of TASK_STOPPED
> when a thread stops because it is ptraced.  Now ptrace operations are only
> permitted when the target is in TASK_TRACED state, not in TASK_STOPPED.
> This means that if a process is stopped normally by a job control signal
> and then you PTRACE_ATTACH to it, you will have to send it a SIGCONT before
> you can do any ptrace operations on it.  (The SIGCONT will be reported to
> ptrace and then you can discard it instead of passing it through when you
> call PTRACE_CONT et al.)

Ok, I definitely agree with the approach (I've not become schizofrenic
yet, but as others can attest, I obviously change my mind occasionally, so 
sometimes I disagree even with my own suggestions ;)

Looks pretty clean as an implementation. The question is whether we should 
aim for 2.6.9 or 2.6.10 - if the first, then I should probably take it 
now, otherwise it should go into -mm first and be merged early after 2.6.9 
has been released, for the first -rc.

I _looks_ pretty safe, and it's hopefully much less likely to have subtle
bugs and races than our old approach had, but I have a hard time judging. 
I assume you ran all your gdb tests on the result? What's your gut feel?

		Linus

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] cleanup ptrace stops and remove notify_parent
  2004-08-31  4:27     ` Linus Torvalds
@ 2004-08-31  4:50       ` Roland McGrath
  2004-08-31 13:19       ` OGAWA Hirofumi
  1 sibling, 0 replies; 25+ messages in thread
From: Roland McGrath @ 2004-08-31  4:50 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Andrew Morton, Kernel Mailing List

> I _looks_ pretty safe, and it's hopefully much less likely to have subtle
> bugs and races than our old approach had, but I have a hard time judging. 
> I assume you ran all your gdb tests on the result? What's your gut feel?

My gut feel is that it's pretty safe.  I did run all the LTP ptrace
programs, and the gdb test suite.  (The gdb test suite has failures because
of gdb's own issues and perhaps the state of my gdb build, so I can't be
totally certain that it has zero new complaints, but I am pretty sure it
didn't change the results.)

Thanks,
Roland

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] cleanup ptrace stops and remove notify_parent
  2004-08-31  4:27     ` Linus Torvalds
  2004-08-31  4:50       ` Roland McGrath
@ 2004-08-31 13:19       ` OGAWA Hirofumi
  2004-08-31 13:56         ` Daniel Jacobowitz
  2004-09-01  0:11         ` Roland McGrath
  1 sibling, 2 replies; 25+ messages in thread
From: OGAWA Hirofumi @ 2004-08-31 13:19 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Roland McGrath, Andrew Morton, Kernel Mailing List

Linus Torvalds <torvalds@osdl.org> writes:

> Ok, I definitely agree with the approach

I agree with that approach.

> Looks pretty clean as an implementation. The question is whether we should 
> aim for 2.6.9 or 2.6.10 - if the first, then I should probably take it 
> now, otherwise it should go into -mm first and be merged early after 2.6.9 
> has been released, for the first -rc.
> 
> I _looks_ pretty safe, and it's hopefully much less likely to have subtle
> bugs and races than our old approach had, but I have a hard time judging. 

Ptrace has several ugly things. And I'm thinking those needs
user-visible change more or less to improve, like this.
(->parent/wait4/child_list, PTRACE_SYSCALL/PTRACE_SINGLESTEP ...)

Should we also clean up and improve those with user-visible change?
Those should be thought as separate issue?

I think we should be improved with new interface... (after it, we
can deprecate ptrace)
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] cleanup ptrace stops and remove notify_parent
  2004-08-31 13:19       ` OGAWA Hirofumi
@ 2004-08-31 13:56         ` Daniel Jacobowitz
  2004-09-01  0:11         ` Roland McGrath
  1 sibling, 0 replies; 25+ messages in thread
From: Daniel Jacobowitz @ 2004-08-31 13:56 UTC (permalink / raw)
  To: OGAWA Hirofumi
  Cc: Linus Torvalds, Roland McGrath, Andrew Morton,
	Kernel Mailing List

On Tue, Aug 31, 2004 at 10:19:04PM +0900, OGAWA Hirofumi wrote:
> Linus Torvalds <torvalds@osdl.org> writes:
> 
> > Ok, I definitely agree with the approach
> 
> I agree with that approach.
> 
> > Looks pretty clean as an implementation. The question is whether we should 
> > aim for 2.6.9 or 2.6.10 - if the first, then I should probably take it 
> > now, otherwise it should go into -mm first and be merged early after 2.6.9 
> > has been released, for the first -rc.
> > 
> > I _looks_ pretty safe, and it's hopefully much less likely to have subtle
> > bugs and races than our old approach had, but I have a hard time judging. 
> 
> Ptrace has several ugly things. And I'm thinking those needs
> user-visible change more or less to improve, like this.
> (->parent/wait4/child_list, PTRACE_SYSCALL/PTRACE_SINGLESTEP ...)
> 
> Should we also clean up and improve those with user-visible change?
> Those should be thought as separate issue?
> 
> I think we should be improved with new interface... (after it, we
> can deprecate ptrace)

I recommend the same thing I recommend every time this comes up: make
sure to take a look at how Solaris does this through /proc.  It seems
to be much nicer.

-- 
Daniel Jacobowitz

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] cleanup ptrace stops and remove notify_parent
  2004-08-31 13:19       ` OGAWA Hirofumi
  2004-08-31 13:56         ` Daniel Jacobowitz
@ 2004-09-01  0:11         ` Roland McGrath
  2004-09-01  4:25           ` OGAWA Hirofumi
  1 sibling, 1 reply; 25+ messages in thread
From: Roland McGrath @ 2004-09-01  0:11 UTC (permalink / raw)
  To: OGAWA Hirofumi; +Cc: Linus Torvalds, Andrew Morton, Kernel Mailing List

> Should we also clean up and improve those with user-visible change?
> Those should be thought as separate issue?

The first answer to give is to the second question: yes, this is a
separate subject from the implementation changes we are discussing right
now.  I don't really want to get into discussing a different interface at
the moment, because I have more than enough to do this week already.

It is certainly something I have considered.  Cleaning up ptrace for use
on multi-threaded processes was no fun at all.  Dan Jacobowitz and I have
talked about this before, and my tendency is the same he mentioned, to
start along the lines of the Solaris /proc interface.  (However, I would
not make Solaris compatibility a priority over implementation sanity.)
But like I said, not for this week!

An important step to a sane and clean implementation of whatever new
interface will be to clean up the machine-specific parts of the
implementation that are required.  If I am ever going to work on this
code again and not turn homicidal, it needs to take the form of a general
framework for special state examination/mutation requests machines
require uses common code to present these in whatever new interface and
compatibly for the ptrace requests that now exist.  More for not this week.

I'd be happy to discuss details of all this stuff with anyone interested
in helping make it happen.  But, you know, later.

Thanks,
Roland

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] cleanup ptrace stops and remove notify_parent
  2004-09-01  0:11         ` Roland McGrath
@ 2004-09-01  4:25           ` OGAWA Hirofumi
  0 siblings, 0 replies; 25+ messages in thread
From: OGAWA Hirofumi @ 2004-09-01  4:25 UTC (permalink / raw)
  To: Roland McGrath; +Cc: Linus Torvalds, Andrew Morton, Kernel Mailing List

Roland McGrath <roland@redhat.com> writes:

> > Should we also clean up and improve those with user-visible change?
> > Those should be thought as separate issue?
> 
> The first answer to give is to the second question: yes, this is a
> separate subject from the implementation changes we are discussing right
> now.  I don't really want to get into discussing a different interface at
> the moment, because I have more than enough to do this week already.

No. You are missing my point. This is user-visible change, not
implementation only.

Is it sane way to do user-visible change for preparation?
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] cleanup ptrace stops and remove notify_parent
  2004-08-31  4:11   ` Roland McGrath
  2004-08-31  4:26     ` Andrew Morton
  2004-08-31  4:27     ` Linus Torvalds
@ 2004-08-31  9:29     ` Christoph Hellwig
  2004-08-31  9:43       ` Anton Blanchard
  2 siblings, 1 reply; 25+ messages in thread
From: Christoph Hellwig @ 2004-08-31  9:29 UTC (permalink / raw)
  To: Roland McGrath; +Cc: Andrew Morton, torvalds, linux-kernel


Just about every architecture hasa line

+	ptrace_notify(SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)  ? 0x80 : 0));

you probably want a one-liner inline wrapper with a descriptive name
around this


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] cleanup ptrace stops and remove notify_parent
  2004-08-31  9:29     ` Christoph Hellwig
@ 2004-08-31  9:43       ` Anton Blanchard
  2004-09-01  0:00         ` Roland McGrath
  0 siblings, 1 reply; 25+ messages in thread
From: Anton Blanchard @ 2004-08-31  9:43 UTC (permalink / raw)
  To: Christoph Hellwig, Roland McGrath, Andrew Morton, torvalds,
	linux-kernel


> Just about every architecture hasa line
> 
> +	ptrace_notify(SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)  ? 0x80 : 0));
> 
> you probably want a one-liner inline wrapper with a descriptive name
> around this

Yep, and it looks like only some architectures got this recent change:

        ptrace_notify(SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD) &&
                                 !test_thread_flag(TIF_SINGLESTEP) ?  0x80 : 0));

Is there a reason this shouldnt be propogated to all architectures?

Anton

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] cleanup ptrace stops and remove notify_parent
  2004-08-31  9:43       ` Anton Blanchard
@ 2004-09-01  0:00         ` Roland McGrath
  0 siblings, 0 replies; 25+ messages in thread
From: Roland McGrath @ 2004-09-01  0:00 UTC (permalink / raw)
  To: Anton Blanchard; +Cc: Christoph Hellwig, Andrew Morton, torvalds, linux-kernel

> 
> > Just about every architecture hasa line
> > 
> > +	ptrace_notify(SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)  ? 0x80 : 0));
> > 
> > you probably want a one-liner inline wrapper with a descriptive name
> > around this
> 
> Yep, and it looks like only some architectures got this recent change:
> 
>         ptrace_notify(SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD) &&
>                                  !test_thread_flag(TIF_SINGLESTEP) ?  0x80 : 0));
> 
> Is there a reason this shouldnt be propogated to all architectures?

It ought to produce the the same result on all, certainly.  TIF_SINGLESTEP
is an implementation detail specific to i386 and x86_64.  The new test here
makes sure that PTRACE_SINGLESTEP never produces an 0x80|SIGTRAP
notification, only PTRACE_SYSCALL, to be consistent with previous behavior.
This i386/x86_64 code path can now be entered for the single-step case, due
to changes that just went in to make single-stepping the system call
instruction work nicely.  If other machines require similar fixes and the
implementation overloads their syscall_trace functions in the same way,
then they will have a change similar to this one to make.

Thanks,
Roland

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] cleanup ptrace stops and remove notify_parent
  2004-08-31  3:25 Roland McGrath
  2004-08-31  3:43 ` Andrew Morton
@ 2004-08-31  3:59 ` Roland McGrath
  2004-08-31  4:22 ` Daniel Jacobowitz
  2 siblings, 0 replies; 25+ messages in thread
From: Roland McGrath @ 2004-08-31  3:59 UTC (permalink / raw)
  To: Linus Torvalds, Andrew Morton, Linux Kernel Mailing List

D'oh!  That patch omitted the changes to kernel/exit.c, which are necessary.
Take this message as a complete replacement for the last one.



This patch is against Linus's current tree.

This adds a new state TASK_TRACED that is used in place of TASK_STOPPED
when a thread stops because it is ptraced.  Now ptrace operations are only
permitted when the target is in TASK_TRACED state, not in TASK_STOPPED.
This means that if a process is stopped normally by a job control signal
and then you PTRACE_ATTACH to it, you will have to send it a SIGCONT before
you can do any ptrace operations on it.  (The SIGCONT will be reported to
ptrace and then you can discard it instead of passing it through when you
call PTRACE_CONT et al.)

If a traced child gets orphaned while in TASK_TRACED state, it morphs into
TASK_STOPPED state.  This makes it again possible to resume or destroy the
process with SIGCONT or SIGKILL.

All non-signal tracing stops should now be done via ptrace_notify.  I've
updated the syscall tracing code in several architectures to do this
instead of replicating the work by hand.  I also fixed several that were
unnecessarily repeating some of the checks in ptrace_check_attach.  Calling
ptrace_check_attach alone is sufficient, and the old checks repeated before
are now incorrect, not just superfluous.

I've closed a race in ptrace_check_attach.  With this, we should have a
robust guarantee that when ptrace starts operating, the task will be in
TASK_TRACED state and won't come out of it.  This is because the only way
to resume from TASK_TRACED is via ptrace operations, and only the one
parent thread attached as the tracer can do those.

This patch also cleans up the do_notify_parent and do_notify_parent_cldstop
code so that the dead and stopped cases are completely disjoint.  The
notify_parent function is gone.



Thanks,
Roland


Signed-off-by: Roland McGrath <roland@redhat.com>

diff -rBzpu linux-2.6.9/arch/arm/kernel/ptrace.c linux-2.6-ptracefix/arch/arm/kernel/ptrace.c
--- linux-2.6.9/arch/arm/kernel/ptrace.c	2004-08-30 20:10:49.000000000 -0700
+++ linux-2.6-ptracefix/arch/arm/kernel/ptrace.c	2004-08-30 18:20:03.000000000 -0700
@@ -792,11 +792,8 @@ asmlinkage void syscall_trace(int why, s
 
 	/* the 0x80 provides a way for the tracing parent to distinguish
 	   between a syscall stop and SIGTRAP delivery */
-	current->exit_code = SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
-					? 0x80 : 0);
-	current->state = TASK_STOPPED;
-	notify_parent(current, SIGCHLD);
-	schedule();
+	ptrace_notify(SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
+				 ? 0x80 : 0));
 	/*
 	 * this isn't the same as continuing with a signal, but it will do
 	 * for normal use.  strace only continues with a signal if the
diff -rBzpu linux-2.6.9/arch/arm26/kernel/ptrace.c linux-2.6-ptracefix/arch/arm26/kernel/ptrace.c
--- linux-2.6.9/arch/arm26/kernel/ptrace.c	2004-08-30 20:10:49.000000000 -0700
+++ linux-2.6-ptracefix/arch/arm26/kernel/ptrace.c	2004-08-30 18:20:03.000000000 -0700
@@ -729,11 +729,8 @@ asmlinkage void syscall_trace(int why, s
 
 	/* the 0x80 provides a way for the tracing parent to distinguish
 	   between a syscall stop and SIGTRAP delivery */
-	current->exit_code = SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
-					? 0x80 : 0);
-	current->state = TASK_STOPPED;
-	notify_parent(current, SIGCHLD);
-	schedule();
+	ptrace_notify(SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
+				 ? 0x80 : 0));
 	/*
 	 * this isn't the same as continuing with a signal, but it will do
 	 * for normal use.  strace only continues with a signal if the
diff -rBzpu linux-2.6.9/arch/cris/arch-v10/kernel/ptrace.c linux-2.6-ptracefix/arch/cris/arch-v10/kernel/ptrace.c
--- linux-2.6.9/arch/cris/arch-v10/kernel/ptrace.c	2004-08-30 20:10:49.000000000 -0700
+++ linux-2.6-ptracefix/arch/cris/arch-v10/kernel/ptrace.c	2004-08-30 18:20:03.000000000 -0700
@@ -85,17 +85,8 @@ sys_ptrace(long request, long pid, long 
 		goto out_tsk;
 	}
 	
-	ret = -ESRCH;
-	
-	if (!(child->ptrace & PT_PTRACED))
-		goto out_tsk;
-	
-	if (child->state != TASK_STOPPED) {
-		if (request != PTRACE_KILL)
-			goto out_tsk;
-	}
-	
-	if (child->parent != current)
+	ret = ptrace_check_attach(child, request == PTRACE_KILL);
+	if (ret < 0)
 		goto out_tsk;
 
 	switch (request) {
diff -rBzpu linux-2.6.9/arch/h8300/kernel/ptrace.c linux-2.6-ptracefix/arch/h8300/kernel/ptrace.c
--- linux-2.6.9/arch/h8300/kernel/ptrace.c	2004-08-30 20:10:49.000000000 -0700
+++ linux-2.6-ptracefix/arch/h8300/kernel/ptrace.c	2004-08-30 18:20:03.000000000 -0700
@@ -89,13 +89,6 @@ asmlinkage int sys_ptrace(long request, 
 		ret = ptrace_attach(child);
 		goto out_tsk;
 	}
-	ret = -ESRCH;
-	if (!(child->ptrace & PT_PTRACED))
-		goto out_tsk;
-	if (child->state != TASK_STOPPED) {
-		if (request != PTRACE_KILL)
-			goto out_tsk;
-	}
 	ret = ptrace_check_attach(child, request == PTRACE_KILL);
 	if (ret < 0)
 		goto out_tsk;
@@ -270,10 +263,8 @@ asmlinkage void syscall_trace(void)
 		return;
 	if (!(current->ptrace & PT_PTRACED))
 		return;
-	current->exit_code = SIGTRAP;
-	current->state = TASK_STOPPED;
-	notify_parent(current, SIGCHLD);
-	schedule();
+	ptrace_notify(SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
+				 ? 0x80 : 0));
 	/*
 	 * this isn't the same as continuing with a signal, but it will do
 	 * for normal use.  strace only continues with a signal if the
diff -rBzpu linux-2.6.9/arch/m68k/kernel/ptrace.c linux-2.6-ptracefix/arch/m68k/kernel/ptrace.c
--- linux-2.6.9/arch/m68k/kernel/ptrace.c	2004-08-30 20:10:49.000000000 -0700
+++ linux-2.6-ptracefix/arch/m68k/kernel/ptrace.c	2004-08-30 18:20:03.000000000 -0700
@@ -379,11 +379,8 @@ asmlinkage void syscall_trace(void)
 	if (!current->thread.work.delayed_trace &&
 	    !current->thread.work.syscall_trace)
 		return;
-	current->exit_code = SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
-					? 0x80 : 0);
-	current->state = TASK_STOPPED;
-	notify_parent(current, SIGCHLD);
-	schedule();
+	ptrace_notify(SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
+				 ? 0x80 : 0));
 	/*
 	 * this isn't the same as continuing with a signal, but it will do
 	 * for normal use.  strace only continues with a signal if the
diff -rBzpu linux-2.6.9/arch/m68knommu/kernel/ptrace.c linux-2.6-ptracefix/arch/m68knommu/kernel/ptrace.c
--- linux-2.6.9/arch/m68knommu/kernel/ptrace.c	2004-08-30 20:10:49.000000000 -0700
+++ linux-2.6-ptracefix/arch/m68knommu/kernel/ptrace.c	2004-08-30 18:20:03.000000000 -0700
@@ -133,13 +133,6 @@ asmlinkage int sys_ptrace(long request, 
 		ret = ptrace_attach(child);
 		goto out_tsk;
 	}
-	ret = -ESRCH;
-	if (!(child->ptrace & PT_PTRACED))
-		goto out_tsk;
-	if (child->state != TASK_STOPPED) {
-		if (request != PTRACE_KILL)
-			goto out_tsk;
-	}
 	ret = ptrace_check_attach(child, request == PTRACE_KILL);
 	if (ret < 0)
 		goto out_tsk;
@@ -376,10 +369,8 @@ asmlinkage void syscall_trace(void)
 		return;
 	if (!(current->ptrace & PT_PTRACED))
 		return;
-	current->exit_code = SIGTRAP;
-	current->state = TASK_STOPPED;
-	notify_parent(current, SIGCHLD);
-	schedule();
+	ptrace_notify(SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
+				 ? 0x80 : 0));
 	/*
 	 * this isn't the same as continuing with a signal, but it will do
 	 * for normal use.  strace only continues with a signal if the
diff -rBzpu linux-2.6.9/arch/parisc/kernel/ptrace.c linux-2.6-ptracefix/arch/parisc/kernel/ptrace.c
--- linux-2.6.9/arch/parisc/kernel/ptrace.c	2004-08-30 20:10:50.000000000 -0700
+++ linux-2.6-ptracefix/arch/parisc/kernel/ptrace.c	2004-08-30 18:20:03.000000000 -0700
@@ -404,11 +404,8 @@ void syscall_trace(void)
 		return;
 	if (!(current->ptrace & PT_PTRACED))
 		return;
-	current->exit_code = SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
-					? 0x80 : 0);
-	current->state = TASK_STOPPED;
-	notify_parent(current, SIGCHLD);
-	schedule();
+	ptrace_notify(SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
+				 ? 0x80 : 0));
 	/*
 	 * this isn't the same as continuing with a signal, but it will do
 	 * for normal use.  strace only continues with a signal if the
diff -rBzpu linux-2.6.9/arch/sh64/kernel/ptrace.c linux-2.6-ptracefix/arch/sh64/kernel/ptrace.c
--- linux-2.6.9/arch/sh64/kernel/ptrace.c	2004-08-30 20:10:50.000000000 -0700
+++ linux-2.6-ptracefix/arch/sh64/kernel/ptrace.c	2004-08-30 18:20:03.000000000 -0700
@@ -311,11 +311,8 @@ asmlinkage void syscall_trace(void)
 	if (!(tsk->ptrace & PT_PTRACED))
 		return;
 
-	tsk->exit_code = SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
-				    ? 0x80 : 0);
-	tsk->state = TASK_STOPPED;
-	notify_parent(tsk, SIGCHLD);
-	schedule();
+	ptrace_notify(SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
+				 ? 0x80 : 0));
 	/*
 	 * this isn't the same as continuing with a signal, but it will do
 	 * for normal use.  strace only continues with a signal if the
diff -rBzpu linux-2.6.9/arch/sparc/kernel/ptrace.c linux-2.6-ptracefix/arch/sparc/kernel/ptrace.c
--- linux-2.6.9/arch/sparc/kernel/ptrace.c	2004-08-30 20:10:50.000000000 -0700
+++ linux-2.6-ptracefix/arch/sparc/kernel/ptrace.c	2004-08-30 18:20:04.000000000 -0700
@@ -614,12 +614,9 @@ asmlinkage void syscall_trace(void)
 		return;
 	if (!(current->ptrace & PT_PTRACED))
 		return;
-	current->exit_code = SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
-					? 0x80 : 0);
-	current->state = TASK_STOPPED;
 	current->thread.flags ^= MAGIC_CONSTANT;
-	notify_parent(current, SIGCHLD);
-	schedule();
+	ptrace_notify(SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
+				 ? 0x80 : 0));
 	/*
 	 * this isn't the same as continuing with a signal, but it will do
 	 * for normal use.  strace only continues with a signal if the
diff -rBzpu linux-2.6.9/arch/sparc64/kernel/ptrace.c linux-2.6-ptracefix/arch/sparc64/kernel/ptrace.c
--- linux-2.6.9/arch/sparc64/kernel/ptrace.c	2004-08-30 20:10:50.000000000 -0700
+++ linux-2.6-ptracefix/arch/sparc64/kernel/ptrace.c	2004-08-30 18:20:04.000000000 -0700
@@ -627,11 +627,8 @@ asmlinkage void syscall_trace(void)
 		return;
 	if (!(current->ptrace & PT_PTRACED))
 		return;
-	current->exit_code = SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
-					? 0x80 : 0);
-	current->state = TASK_STOPPED;
-	notify_parent(current, SIGCHLD);
-	schedule();
+	ptrace_notify(SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
+				 ? 0x80 : 0));
 
 	/*
 	 * this isn't the same as continuing with a signal, but it will do
diff -rBzpu linux-2.6.9/arch/v850/kernel/ptrace.c linux-2.6-ptracefix/arch/v850/kernel/ptrace.c
--- linux-2.6.9/arch/v850/kernel/ptrace.c	2004-08-30 20:10:50.000000000 -0700
+++ linux-2.6-ptracefix/arch/v850/kernel/ptrace.c	2004-08-30 18:20:04.000000000 -0700
@@ -147,14 +147,8 @@ int sys_ptrace(long request, long pid, l
 		rval = ptrace_attach(child);
 		goto out_tsk;
 	}
-	rval = -ESRCH;
-	if (!(child->ptrace & PT_PTRACED))
-		goto out_tsk;
-	if (child->state != TASK_STOPPED) {
-		if (request != PTRACE_KILL)
-			goto out_tsk;
-	}
-	if (child->parent != current)
+	ret = ptrace_check_attach(child, request == PTRACE_KILL);
+	if (ret < 0)
 		goto out_tsk;
 
 	switch (request) {
@@ -269,11 +263,8 @@ asmlinkage void syscall_trace(void)
 		return;
 	/* The 0x80 provides a way for the tracing parent to distinguish
 	   between a syscall stop and SIGTRAP delivery */
-	current->exit_code = SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
-					? 0x80 : 0);
-	current->state = TASK_STOPPED;
-	notify_parent(current, SIGCHLD);
-	schedule();
+	ptrace_notify(SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
+				 ? 0x80 : 0));
 	/*
 	 * this isn't the same as continuing with a signal, but it will do
 	 * for normal use.  strace only continues with a signal if the
diff -rBzpu linux-2.6.9/fs/proc/array.c linux-2.6-ptracefix/fs/proc/array.c
--- linux-2.6.9/fs/proc/array.c	2004-08-30 20:10:50.000000000 -0700
+++ linux-2.6-ptracefix/fs/proc/array.c	2004-08-30 18:43:29.000000000 -0700
@@ -130,8 +130,9 @@ static const char *task_state_array[] = 
 	"S (sleeping)",		/*  1 */
 	"D (disk sleep)",	/*  2 */
 	"T (stopped)",		/*  4 */
-	"Z (zombie)",		/*  8 */
-	"X (dead)"		/* 16 */
+	"T (tracing stop)",	/*  8 */
+	"Z (zombie)",		/* 16 */
+	"X (dead)"		/* 32 */
 };
 
 static inline const char * get_task_state(struct task_struct *tsk)
@@ -141,7 +142,8 @@ static inline const char * get_task_stat
 					   TASK_UNINTERRUPTIBLE |
 					   TASK_ZOMBIE |
 					   TASK_DEAD |
-					   TASK_STOPPED);
+					   TASK_STOPPED |
+					   TASK_TRACED);
 	const char **p = &task_state_array[0];
 
 	while (state) {
diff -rBzpu linux-2.6.9/fs/proc/base.c linux-2.6-ptracefix/fs/proc/base.c
--- linux-2.6.9/fs/proc/base.c	2004-08-30 20:10:50.000000000 -0700
+++ linux-2.6-ptracefix/fs/proc/base.c	2004-08-30 18:20:05.000000000 -0700
@@ -287,7 +287,8 @@ static int proc_root_link(struct inode *
 #define MAY_PTRACE(task) \
 	(task == current || \
 	(task->parent == current && \
-	(task->ptrace & PT_PTRACED) &&  task->state == TASK_STOPPED && \
+	(task->ptrace & PT_PTRACED) && \
+	 (task->state == TASK_STOPPED || task->state == TASK_TRACED) && \
 	 security_ptrace(current,task) == 0))
 
 static int may_ptrace_attach(struct task_struct *task)
diff -rBzpu linux-2.6.9/include/linux/sched.h linux-2.6-ptracefix/include/linux/sched.h
--- linux-2.6.9/include/linux/sched.h	2004-08-30 20:10:50.000000000 -0700
+++ linux-2.6-ptracefix/include/linux/sched.h	2004-08-30 18:20:05.000000000 -0700
@@ -106,8 +106,9 @@ extern unsigned long nr_iowait(void);
 #define TASK_INTERRUPTIBLE	1
 #define TASK_UNINTERRUPTIBLE	2
 #define TASK_STOPPED		4
-#define TASK_ZOMBIE		8
-#define TASK_DEAD		16
+#define TASK_TRACED		8
+#define TASK_ZOMBIE		16
+#define TASK_DEAD		32
 
 #define __set_task_state(tsk, state_value)		\
 	do { (tsk)->state = (state_value); } while (0)
@@ -725,7 +726,6 @@ extern int __kill_pg_info(int sig, struc
 extern int kill_pg_info(int, struct siginfo *, pid_t);
 extern int kill_sl_info(int, struct siginfo *, pid_t);
 extern int kill_proc_info(int, struct siginfo *, pid_t);
-extern void notify_parent(struct task_struct *, int);
 extern void do_notify_parent(struct task_struct *, int);
 extern void force_sig(int, struct task_struct *);
 extern void force_sig_specific(int, struct task_struct *);
diff -rBzpu linux-2.6.9/kernel/exit.c linux-2.6-ptracefix/kernel/exit.c
--- linux-2.6.9/kernel/exit.c	2004-08-30 20:57:39.220151522 -0700
+++ linux-2.6-ptracefix/kernel/exit.c	2004-08-30 19:50:16.000000000 -0700
@@ -566,6 +566,14 @@ static inline void reparent_thread(task_
 		if (p->state == TASK_ZOMBIE && p->exit_signal != -1 &&
 		    thread_group_empty(p))
 			do_notify_parent(p, p->exit_signal);
+		else if (p->state == TASK_TRACED) {
+			/*
+			 * If it was at a trace stop, turn it into
+			 * a normal stop since it's no longer being
+			 * traced.
+			 */
+			p->state = TASK_STOPPED;
+		}
 	}
 
 	/*
@@ -1065,7 +1073,7 @@ static int wait_task_stopped(task_t *p, 
 	 * race with the TASK_ZOMBIE case.
 	 */
 	exit_code = xchg(&p->exit_code, 0);
-	if (unlikely(p->state > TASK_STOPPED)) {
+	if (unlikely(p->state >= TASK_ZOMBIE)) {
 		/*
 		 * The task resumed and then died.  Let the next iteration
 		 * catch it in TASK_ZOMBIE.  Note that exit_code might
@@ -1133,6 +1141,10 @@ repeat:
 			flag = 1;
 
 			switch (p->state) {
+			case TASK_TRACED:
+				if (!(p->ptrace & PT_PTRACED))
+					continue;
+				/*FALLTHROUGH*/
 			case TASK_STOPPED:
 				if (!(options & WUNTRACED) &&
 				    !(p->ptrace & PT_PTRACED))
diff -rBzpu linux-2.6.9/kernel/power/process.c linux-2.6-ptracefix/kernel/power/process.c
--- linux-2.6.9/kernel/power/process.c	2004-08-30 20:10:51.000000000 -0700
+++ linux-2.6-ptracefix/kernel/power/process.c	2004-08-30 18:20:05.000000000 -0700
@@ -25,7 +25,8 @@ static inline int freezeable(struct task
 	    (p->flags & PF_NOFREEZE) ||
 	    (p->state == TASK_ZOMBIE) ||
 	    (p->state == TASK_DEAD) ||
-	    (p->state == TASK_STOPPED))
+	    (p->state == TASK_STOPPED) ||
+	    (p->state == TASK_TRACED))
 		return 0;
 	return 1;
 }
@@ -70,6 +71,7 @@ int freeze_processes(void)
 			if (!freezeable(p))
 				continue;
 			if ((p->flags & PF_FROZEN) ||
+			    (p->state == TASK_TRACED) ||
 			    (p->state == TASK_STOPPED))
 				continue;
 
diff -rBzpu linux-2.6.9/kernel/ptrace.c linux-2.6-ptracefix/kernel/ptrace.c
--- linux-2.6.9/kernel/ptrace.c	2004-08-30 20:10:51.000000000 -0700
+++ linux-2.6-ptracefix/kernel/ptrace.c	2004-08-30 19:35:34.000000000 -0700
@@ -55,6 +55,15 @@ void __ptrace_unlink(task_t *child)
 	REMOVE_LINKS(child);
 	child->parent = child->real_parent;
 	SET_LINKS(child);
+
+	if (child->state == TASK_TRACED) {
+		/*
+		 * Turn a tracing stop into a normal stop now,
+		 * since with no tracer there would be no way
+		 * to wake it up with SIGCONT or SIGKILL.
+		 */
+		child->state = TASK_STOPPED;
+	}
 }
 
 /*
@@ -62,20 +71,28 @@ void __ptrace_unlink(task_t *child)
  */
 int ptrace_check_attach(struct task_struct *child, int kill)
 {
-	if (!(child->ptrace & PT_PTRACED))
-		return -ESRCH;
+	int ret = -ESRCH;
 
-	if (child->parent != current)
-		return -ESRCH;
+	/*
+	 * We take the read lock around doing both checks to close a
+	 * possible race where someone else was tracing our child and
+	 * detached between these two checks.  After this locked check,
+	 * we are sure that this is our traced child and that can only
+	 * be changed by us so it's not changing right after this.
+	 */
+	read_lock(&tasklist_lock);
+	if ((child->ptrace & PT_PTRACED) && child->parent == current)
+		ret = 0;
+	read_unlock(&tasklist_lock);
 
-	if (!kill) {
-		if (child->state != TASK_STOPPED)
+	if (!ret && !kill) {
+		if (child->state != TASK_TRACED)
 			return -ESRCH;
 		wait_task_inactive(child);
 	}
 
 	/* All systems go.. */
-	return 0;
+	return ret;
 }
 
 int ptrace_attach(struct task_struct *task)
@@ -281,15 +298,13 @@ static int ptrace_setoptions(struct task
 
 static int ptrace_getsiginfo(struct task_struct *child, siginfo_t __user * data)
 {
-	if (child->last_siginfo == NULL)
-		return -EINVAL;
+	BUG_ON(child->last_siginfo == NULL);
 	return copy_siginfo_to_user(data, child->last_siginfo);
 }
 
 static int ptrace_setsiginfo(struct task_struct *child, siginfo_t __user * data)
 {
-	if (child->last_siginfo == NULL)
-		return -EINVAL;
+	BUG_ON(child->last_siginfo == NULL);
 	if (copy_from_user(child->last_siginfo, data, sizeof (siginfo_t)) != 0)
 		return -EFAULT;
 	return 0;
@@ -322,24 +337,3 @@ int ptrace_request(struct task_struct *c
 
 	return ret;
 }
-
-void ptrace_notify(int exit_code)
-{
-	BUG_ON (!(current->ptrace & PT_PTRACED));
-
-	/* Let the debugger run.  */
-	current->exit_code = exit_code;
-	set_current_state(TASK_STOPPED);
-	notify_parent(current, SIGCHLD);
-	schedule();
-
-	/*
-	 * Signals sent while we were stopped might set TIF_SIGPENDING.
-	 */
-
-	spin_lock_irq(&current->sighand->siglock);
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
-}
-
-EXPORT_SYMBOL(ptrace_notify);
diff -rBzpu linux-2.6.9/kernel/sched.c linux-2.6-ptracefix/kernel/sched.c
--- linux-2.6.9/kernel/sched.c	2004-08-30 20:10:51.000000000 -0700
+++ linux-2.6-ptracefix/kernel/sched.c	2004-08-30 18:58:34.000000000 -0700
@@ -1258,7 +1258,7 @@ out:
 
 int fastcall wake_up_process(task_t * p)
 {
-	return try_to_wake_up(p, TASK_STOPPED |
+	return try_to_wake_up(p, TASK_STOPPED | TASK_TRACED |
 		       		 TASK_INTERRUPTIBLE | TASK_UNINTERRUPTIBLE, 0);
 }
 
@@ -3670,7 +3670,7 @@ static void show_task(task_t * p)
 	task_t *relative;
 	unsigned state;
 	unsigned long free = 0;
-	static const char *stat_nam[] = { "R", "S", "D", "T", "Z", "W" };
+	static const char *stat_nam[] = { "R", "S", "D", "T", "t", "Z", "X" };
 
 	printk("%-13.13s ", p->comm);
 	state = p->state ? __ffs(p->state) + 1 : 0;
diff -rBzpu linux-2.6.9/kernel/signal.c linux-2.6-ptracefix/kernel/signal.c
--- linux-2.6.9/kernel/signal.c	2004-08-30 20:10:51.000000000 -0700
+++ linux-2.6-ptracefix/kernel/signal.c	2004-08-30 20:04:45.000000000 -0700
@@ -618,7 +618,8 @@ static int check_kill_permission(int sig
 
 /* forward decl */
 static void do_notify_parent_cldstop(struct task_struct *tsk,
-				     struct task_struct *parent);
+				     struct task_struct *parent,
+				     int why);
 
 /*
  * Handle magic process-wide effects of stop/continue signals.
@@ -661,11 +662,13 @@ static void handle_stop_signal(int sig, 
 			 */
 			p->signal->group_stop_count = 0;
 			if (p->ptrace & PT_PTRACED)
-				do_notify_parent_cldstop(p, p->parent);
+				do_notify_parent_cldstop(p, p->parent,
+							 CLD_STOPPED);
 			else
 				do_notify_parent_cldstop(
 					p->group_leader,
-					p->group_leader->real_parent);
+					p->group_leader->real_parent,
+					CLD_STOPPED);
 		}
 		rm_from_queue(SIG_KERNEL_STOP_MASK, &p->signal->shared_pending);
 		t = p;
@@ -861,11 +864,20 @@ force_sig_specific(int sig, struct task_
 
 
 static void
-__group_complete_signal(int sig, struct task_struct *p, unsigned int mask)
+__group_complete_signal(int sig, struct task_struct *p)
 {
+	unsigned int mask;
 	struct task_struct *t;
 
 	/*
+	 * Don't bother zombies and stopped tasks (but
+	 * SIGKILL will punch through stopped state)
+	 */
+	mask = TASK_DEAD | TASK_ZOMBIE | TASK_TRACED;
+	if (sig != SIGKILL)
+		mask |= TASK_STOPPED;
+
+	/*
 	 * Now find a thread we can wake up to take the signal off the queue.
 	 *
 	 * If the main thread wants the signal, it gets first crack.
@@ -966,7 +978,6 @@ __group_complete_signal(int sig, struct 
 static int
 __group_send_sig_info(int sig, struct siginfo *info, struct task_struct *p)
 {
-	unsigned int mask;
 	int ret = 0;
 
 #ifdef CONFIG_SMP
@@ -990,14 +1001,6 @@ __group_send_sig_info(int sig, struct si
 		return ret;
 
 	/*
-	 * Don't bother zombies and stopped tasks (but
-	 * SIGKILL will punch through stopped state)
-	 */
-	mask = TASK_DEAD | TASK_ZOMBIE;
-	if (sig != SIGKILL)
-		mask |= TASK_STOPPED;
-
-	/*
 	 * Put this signal on the shared-pending queue, or fail with EAGAIN.
 	 * We always use the shared queue for process-wide signals,
 	 * to avoid several races.
@@ -1006,7 +1009,7 @@ __group_send_sig_info(int sig, struct si
 	if (unlikely(ret))
 		return ret;
 
-	__group_complete_signal(sig, p, mask);
+	__group_complete_signal(sig, p);
 	return 0;
 }
 
@@ -1367,7 +1370,6 @@ int
 send_group_sigqueue(int sig, struct sigqueue *q, struct task_struct *p)
 {
 	unsigned long flags;
-	unsigned int mask;
 	int ret = 0;
 
 	BUG_ON(!(q->flags & SIGQUEUE_PREALLOC));
@@ -1392,13 +1394,6 @@ send_group_sigqueue(int sig, struct sigq
 		q->info.si_overrun++;
 		goto out;
 	} 
-	/*
-	 * Don't bother zombies and stopped tasks (but
-	 * SIGKILL will punch through stopped state)
-	 */
-	mask = TASK_DEAD | TASK_ZOMBIE;
-	if (sig != SIGKILL)
-		mask |= TASK_STOPPED;
 
 	/*
 	 * Put this signal on the shared-pending queue.
@@ -1409,7 +1404,7 @@ send_group_sigqueue(int sig, struct sigq
 	list_add_tail(&q->list, &p->signal->shared_pending.list);
 	sigaddset(&p->signal->shared_pending.signal, sig);
 
-	__group_complete_signal(sig, p, mask);
+	__group_complete_signal(sig, p);
 out:
 	spin_unlock_irqrestore(&p->sighand->siglock, flags);
 	read_unlock(&tasklist_lock);
@@ -1442,21 +1437,24 @@ static void __wake_up_parent(struct task
 }
 
 /*
- * Let a parent know about a status change of a child.
+ * Let a parent know about the death of a child.
+ * For a stopped/continued status change, use do_notify_parent_cldstop instead.
  */
 
 void do_notify_parent(struct task_struct *tsk, int sig)
 {
 	struct siginfo info;
 	unsigned long flags;
-	int why, status;
 	struct sighand_struct *psig;
 
 	if (sig == -1)
 		BUG();
 
-	BUG_ON(tsk->group_leader != tsk && tsk->group_leader->state != TASK_ZOMBIE && !tsk->ptrace);
-	BUG_ON(tsk->group_leader == tsk && !thread_group_empty(tsk) && !tsk->ptrace);
+	/* do_notify_parent_cldstop should have been called instead.  */
+	BUG_ON(tsk->state & (TASK_STOPPED|TASK_TRACED));
+
+	BUG_ON(!tsk->ptrace &&
+	       (tsk->group_leader != tsk || !thread_group_empty(tsk)));
 
 	info.si_signo = sig;
 	info.si_errno = 0;
@@ -1467,34 +1465,19 @@ void do_notify_parent(struct task_struct
 	info.si_utime = tsk->utime;
 	info.si_stime = tsk->stime;
 
-	status = tsk->exit_code & 0x7f;
-	why = SI_KERNEL;	/* shouldn't happen */
-	switch (tsk->state) {
-	case TASK_STOPPED:
-		/* FIXME -- can we deduce CLD_TRAPPED or CLD_CONTINUED? */
-		if (tsk->ptrace & PT_PTRACED)
-			why = CLD_TRAPPED;
-		else
-			why = CLD_STOPPED;
-		break;
-
-	default:
+	info.si_status = tsk->exit_code & 0x7f;
 		if (tsk->exit_code & 0x80)
-			why = CLD_DUMPED;
+		info.si_code = CLD_DUMPED;
 		else if (tsk->exit_code & 0x7f)
-			why = CLD_KILLED;
+		info.si_code = CLD_KILLED;
 		else {
-			why = CLD_EXITED;
-			status = tsk->exit_code >> 8;
-		}
-		break;
+		info.si_code = CLD_EXITED;
+		info.si_status = tsk->exit_code >> 8;
 	}
-	info.si_code = why;
-	info.si_status = status;
 
 	psig = tsk->parent->sighand;
 	spin_lock_irqsave(&psig->siglock, flags);
-	if (sig == SIGCHLD && tsk->state != TASK_STOPPED &&
+	if (sig == SIGCHLD &&
 	    (psig->action[SIGCHLD-1].sa.sa_handler == SIG_IGN ||
 	     (psig->action[SIGCHLD-1].sa.sa_flags & SA_NOCLDWAIT))) {
 		/*
@@ -1522,26 +1505,9 @@ void do_notify_parent(struct task_struct
 	spin_unlock_irqrestore(&psig->siglock, flags);
 }
 
-
-/*
- * We need the tasklist lock because it's the only
- * thing that protects out "parent" pointer.
- *
- * exit.c calls "do_notify_parent()" directly, because
- * it already has the tasklist lock.
- */
-void
-notify_parent(struct task_struct *tsk, int sig)
-{
-	if (sig != -1) {
-		read_lock(&tasklist_lock);
-		do_notify_parent(tsk, sig);
-		read_unlock(&tasklist_lock);
-	}
-}
-
 static void
-do_notify_parent_cldstop(struct task_struct *tsk, struct task_struct *parent)
+do_notify_parent_cldstop(struct task_struct *tsk, struct task_struct *parent,
+			 int why)
 {
 	struct siginfo info;
 	unsigned long flags;
@@ -1556,8 +1522,20 @@ do_notify_parent_cldstop(struct task_str
 	info.si_utime = tsk->utime;
 	info.si_stime = tsk->stime;
 
-	info.si_status = tsk->exit_code & 0x7f;
-	info.si_code = CLD_STOPPED;
+	info.si_code = why;
+	switch (why) {
+	case CLD_CONTINUED:
+		info.si_status = SIGCONT;
+		break;
+	case CLD_STOPPED:
+		info.si_status = tsk->signal->group_exit_code & 0x7f;
+		break;
+	case CLD_TRAPPED:
+		info.si_status = tsk->exit_code & 0x7f;
+		break;
+	default:
+		BUG();
+	}
 
 	sighand = parent->sighand;
 	spin_lock_irqsave(&sighand->siglock, flags);
@@ -1571,6 +1549,68 @@ do_notify_parent_cldstop(struct task_str
 	spin_unlock_irqrestore(&sighand->siglock, flags);
 }
 
+/*
+ * This must be called with current->sighand->siglock held.
+ *
+ * This should be the path for all ptrace stops.
+ * We always set current->last_siginfo while stopped here.
+ * That makes it a way to test a stopped process for
+ * being ptrace-stopped vs being job-control-stopped.
+ */
+static void ptrace_stop(int exit_code, siginfo_t *info)
+{
+	BUG_ON(!(current->ptrace & PT_PTRACED));
+
+	/*
+	 * If there is a group stop in progress,
+	 * we must participate in the bookkeeping.
+	 */
+	if (current->signal->group_stop_count > 0)
+		--current->signal->group_stop_count;
+
+	current->last_siginfo = info;
+	current->exit_code = exit_code;
+
+	/* Let the debugger run.  */
+	set_current_state(TASK_TRACED);
+	spin_unlock_irq(&current->sighand->siglock);
+	read_lock(&tasklist_lock);
+	do_notify_parent_cldstop(current, current->parent, CLD_TRAPPED);
+	read_unlock(&tasklist_lock);
+	schedule();
+
+	/*
+	 * We are back.  Now reacquire the siglock before touching
+	 * last_siginfo, so that we are sure to have synchronized with
+	 * any signal-sending on another CPU that wants to examine it.
+	 */
+	spin_lock_irq(&current->sighand->siglock);
+	current->last_siginfo = NULL;
+
+	/*
+	 * Queued signals ignored us while we were stopped for tracing.
+	 * So check for any that we should take before resuming user mode.
+	 */
+	recalc_sigpending();
+}
+
+void ptrace_notify(int exit_code)
+{
+	siginfo_t info;
+
+	BUG_ON((exit_code & (0x7f | ~0xffff)) != SIGTRAP);
+
+	memset(&info, 0, sizeof info);
+	info.si_signo = SIGTRAP;
+	info.si_code = exit_code;
+	info.si_pid = current->pid;
+	info.si_uid = current->uid;
+
+	/* Let the debugger run.  */
+	spin_lock_irq(&current->sighand->siglock);
+	ptrace_stop(exit_code, &info);
+	spin_unlock_irq(&current->sighand->siglock);
+}
 
 #ifndef HAVE_ARCH_GET_SIGNAL_TO_DELIVER
 
@@ -1584,13 +1624,15 @@ finish_stop(int stop_count)
 	 */
 	if (stop_count < 0 || (current->ptrace & PT_PTRACED)) {
 		read_lock(&tasklist_lock);
-		do_notify_parent_cldstop(current, current->parent);
+		do_notify_parent_cldstop(current, current->parent,
+					 CLD_STOPPED);
 		read_unlock(&tasklist_lock);
 	}
 	else if (stop_count == 0) {
 		read_lock(&tasklist_lock);
 		do_notify_parent_cldstop(current->group_leader,
-					 current->group_leader->real_parent);
+					 current->group_leader->real_parent,
+					 CLD_STOPPED);
 		read_unlock(&tasklist_lock);
 	}
 
@@ -1629,7 +1671,7 @@ do_signal_stop(int signr)
 		/*
 		 * Lock must be held through transition to stopped state.
 		 */
-		current->exit_code = signr;
+		current->exit_code = current->signal->group_exit_code = signr;
 		set_current_state(TASK_STOPPED);
 		spin_unlock_irq(&sighand->siglock);
 	}
@@ -1766,25 +1808,10 @@ relock:
 		if ((current->ptrace & PT_PTRACED) && signr != SIGKILL) {
 			ptrace_signal_deliver(regs, cookie);
 
-			/*
-			 * If there is a group stop in progress,
-			 * we must participate in the bookkeeping.
-			 */
-			if (current->signal->group_stop_count > 0)
-				--current->signal->group_stop_count;
-
 			/* Let the debugger run.  */
-			current->exit_code = signr;
-			current->last_siginfo = info;
-			set_current_state(TASK_STOPPED);
-			spin_unlock_irq(&current->sighand->siglock);
-			notify_parent(current, SIGCHLD);
-			schedule();
-
-			current->last_siginfo = NULL;
+			ptrace_stop(signr, info);
 
 			/* We're back.  Did the debugger cancel the sig?  */
-			spin_lock_irq(&current->sighand->siglock);
 			signr = current->exit_code;
 			if (signr == 0)
 				continue;
@@ -1915,7 +1942,7 @@ EXPORT_SYMBOL(kill_proc);
 EXPORT_SYMBOL(kill_proc_info);
 EXPORT_SYMBOL(kill_sl);
 EXPORT_SYMBOL(kill_sl_info);
-EXPORT_SYMBOL(notify_parent);
+EXPORT_SYMBOL(ptrace_notify);
 EXPORT_SYMBOL(send_sig);
 EXPORT_SYMBOL(send_sig_info);
 EXPORT_SYMBOL(send_group_sig_info);

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] cleanup ptrace stops and remove notify_parent
  2004-08-31  3:25 Roland McGrath
  2004-08-31  3:43 ` Andrew Morton
  2004-08-31  3:59 ` Roland McGrath
@ 2004-08-31  4:22 ` Daniel Jacobowitz
  2004-08-31  4:55   ` Roland McGrath
  2 siblings, 1 reply; 25+ messages in thread
From: Daniel Jacobowitz @ 2004-08-31  4:22 UTC (permalink / raw)
  To: Roland McGrath; +Cc: Linus Torvalds, Andrew Morton, Linux Kernel Mailing List

On Mon, Aug 30, 2004 at 08:25:46PM -0700, Roland McGrath wrote:
> This patch is against Linus's current tree.
> 
> This adds a new state TASK_TRACED that is used in place of TASK_STOPPED
> when a thread stops because it is ptraced.  Now ptrace operations are only
> permitted when the target is in TASK_TRACED state, not in TASK_STOPPED.
> This means that if a process is stopped normally by a job control signal
> and then you PTRACE_ATTACH to it, you will have to send it a SIGCONT before
> you can do any ptrace operations on it.  (The SIGCONT will be reported to
> ptrace and then you can discard it instead of passing it through when you
> call PTRACE_CONT et al.)
> 
> If a traced child gets orphaned while in TASK_TRACED state, it morphs into
> TASK_STOPPED state.  This makes it again possible to resume or destroy the
> process with SIGCONT or SIGKILL.

Nice.

Unless it's been changed since I last pulled, you should also fix up
has_stopped_jobs.  I think it's broken by the introduction of
TASK_TRACED.

-- 
Daniel Jacobowitz

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] cleanup ptrace stops and remove notify_parent
  2004-08-31  4:22 ` Daniel Jacobowitz
@ 2004-08-31  4:55   ` Roland McGrath
  2004-08-31  4:59     ` Daniel Jacobowitz
  0 siblings, 1 reply; 25+ messages in thread
From: Roland McGrath @ 2004-08-31  4:55 UTC (permalink / raw)
  To: Daniel Jacobowitz
  Cc: Linus Torvalds, Andrew Morton, Linux Kernel Mailing List

> Unless it's been changed since I last pulled, you should also fix up
> has_stopped_jobs.  I think it's broken by the introduction of
> TASK_TRACED.

Actually, I don't think it was broken at all.  It has an old kludge to
avoid considering trace-stopped threads as stopped for purposes of deciding
to generate signals for an orphaned process group.  I think that the useful
thing is for it not to consider any TASK_TRACED thread as stopped here either.
That's what it will already do, and the old kludge can go now:

--- linux-2.6-ptracefix/kernel/exit.c.~1~
+++ linux-2.6-ptracefix/kernel/exit.c
@@ -199,17 +199,6 @@ static inline int has_stopped_jobs(int p
 	for_each_task_pid(pgrp, PIDTYPE_PGID, p, l, pid) {
 		if (p->state != TASK_STOPPED)
 			continue;
-
-		/* If p is stopped by a debugger on a signal that won't
-		   stop it, then don't count p as stopped.  This isn't
-		   perfect but it's a good approximation.  */
-		if (unlikely (p->ptrace)
-		    && p->exit_code != SIGSTOP
-		    && p->exit_code != SIGTSTP
-		    && p->exit_code != SIGTTOU
-		    && p->exit_code != SIGTTIN)
-			continue;
-
 		retval = 1;
 		break;
 	}


Thanks,
Roland

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] cleanup ptrace stops and remove notify_parent
  2004-08-31  4:55   ` Roland McGrath
@ 2004-08-31  4:59     ` Daniel Jacobowitz
  0 siblings, 0 replies; 25+ messages in thread
From: Daniel Jacobowitz @ 2004-08-31  4:59 UTC (permalink / raw)
  To: Roland McGrath; +Cc: Linus Torvalds, Andrew Morton, Linux Kernel Mailing List

On Mon, Aug 30, 2004 at 09:55:30PM -0700, Roland McGrath wrote:
> > Unless it's been changed since I last pulled, you should also fix up
> > has_stopped_jobs.  I think it's broken by the introduction of
> > TASK_TRACED.
> 
> Actually, I don't think it was broken at all.  It has an old kludge to
> avoid considering trace-stopped threads as stopped for purposes of deciding
> to generate signals for an orphaned process group.  I think that the useful
> thing is for it not to consider any TASK_TRACED thread as stopped here either.
> That's what it will already do, and the old kludge can go now:

[The kludge isn't all that old.  I added it about a year and a half
ago.]

You're right.  I misread the loop.  It's still only an approximation,
in that we don't know what the debugger's intention towards resuming
the program is, but I think we err on the correct side now.

-- 
Daniel Jacobowitz

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2004-09-04 13:39 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <2z7vs-2F1-13@gated-at.bofh.it>
2004-08-31 10:00 ` [PATCH] cleanup ptrace stops and remove notify_parent Andi Kleen
2004-09-01  0:27   ` Roland McGrath
2004-09-01  1:56     ` Roland McGrath
2004-09-04 13:38       ` Andi Kleen
2004-08-31 15:59 Albert Cahalan
2004-08-31 23:54 ` Roland McGrath
2004-09-01  0:27   ` Linus Torvalds
  -- strict thread matches above, loose matches on Subject: below --
2004-08-31  3:25 Roland McGrath
2004-08-31  3:43 ` Andrew Morton
2004-08-31  4:11   ` Roland McGrath
2004-08-31  4:26     ` Andrew Morton
2004-08-31  5:43       ` Roland McGrath
2004-08-31  4:27     ` Linus Torvalds
2004-08-31  4:50       ` Roland McGrath
2004-08-31 13:19       ` OGAWA Hirofumi
2004-08-31 13:56         ` Daniel Jacobowitz
2004-09-01  0:11         ` Roland McGrath
2004-09-01  4:25           ` OGAWA Hirofumi
2004-08-31  9:29     ` Christoph Hellwig
2004-08-31  9:43       ` Anton Blanchard
2004-09-01  0:00         ` Roland McGrath
2004-08-31  3:59 ` Roland McGrath
2004-08-31  4:22 ` Daniel Jacobowitz
2004-08-31  4:55   ` Roland McGrath
2004-08-31  4:59     ` Daniel Jacobowitz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox