kern/115469: [patch] ptrace(2) signal delivery broken

[Tijl Coosemans]

>Number:         115469
>Category:       kern
>Synopsis:       [patch] ptrace(2) signal delivery broken
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Mon Aug 13 15:30:01 GMT 2007
>Closed-Date:
>Last-Modified:
>Originator:     Tijl Coosemans
>Release:        FreeBSD 7.0-CURRENT i386
>Organization:
>Environment:
>Description:
In kern_sig.c:issignal(), when a thread receives a signal and the
process is being traced, ptracestop() is called, which suspends all
threads in the process and notifies the parent. When the threads
are resumed, ptracestop() returns td->td_xsig, the signal to be
actually delivered to the process.

Resuming the process is done by the parent calling
ptrace(PT_CONTINUE, sig) which stores sig in td->td_xsig but
currently also calls psignal(). The latter causes the signal to be
queued again and the process stops on it again when it is handled.
>How-To-Repeat:
You can hit on this problem when debugging a process that has its
own signal handlers. In that case when a signal is sent to the
process, the process stops and gdb notifies you of the signal. When
you let the process continue and deal with the signal, the signal
handler is called, but then when the process returns from the
handler, it stops again and again and again...
>Fix:
If a process is stopped ptrace should use td_xsig to deliver the
signal and otherwise (PT_ATTACH case) it should use psignal(),
which is what the following patch does.

--- patch-ptrace begins here ---
--- sys/kern/sys_process.c.orig	2007-08-02 15:53:10.000000000 +0200
+++ sys/kern/sys_process.c	2007-08-02 19:49:56.000000000 +0200
@@ -779,14 +779,15 @@
 			sx_xunlock(&proctree_lock);
 			proctree_locked = 0;
 		}
-		/* deliver or queue signal */
-		thread_lock(td2);
-		td2->td_flags &= ~TDF_XSIG;
-		thread_unlock(td2);
-		td2->td_xsig = data;
 		p->p_xstat = data;
 		p->p_xthread = NULL;
 		if ((p->p_flag & (P_STOPPED_SIG | P_STOPPED_TRACE)) != 0) {
+			/* deliver or queue signal */
+			thread_lock(td2);
+			td2->td_flags &= ~TDF_XSIG;
+			thread_unlock(td2);
+			td2->td_xsig = data;
+
 			PROC_SLOCK(p);
 			if (req == PT_DETACH) {
 				struct thread *td3;
@@ -809,11 +810,10 @@
 			p->p_flag &= ~(P_STOPPED_TRACE|P_STOPPED_SIG|P_WAITED);
 			thread_unsuspend(p);
 			PROC_SUNLOCK(p);
+		} else {
+			if (data)
+				psignal(p, data);
 		}
-
-		if (data)
-			psignal(p, data);
-
 		break;
 
 	case PT_WRITE_I:
--- patch-ptrace ends here ---
>Release-Note:
>Audit-Trail:
>Unformatted: