While working on Wine I hit on a race between a ptrace PT_DETACH and
subsequent PT_ATTACH which causes the SIGSTOP of this attach to get
lost and never delivered. Attached are a test program and a proposed
patch.

The test program forks a child and loops attaching and detaching to it.
It can hang during the wait4 call.

The problem occurs when the stopped child process returns from the
ptracestop() call in sys/kern/kern_sig.c:issignal() after the parent
has already returned from the next PT_ATTACH. Then the signal that
caused the process to stop is taken off the sigqueue, but this may
already be a new SIGSTOP from the next PT_ATTACH. The solution is to
take the signal off the queue before calling ptracestop().

A second cause for the problem is in sys/kern/sys_process.c:kern_ptrace()
where PT_ATTACH sets td_xsig of a thread to SIGSTOP. If this thread
then returns from ptracestop() (returning td_xsig) and it was previously
stopped by a SIGSTOP, the new SIGSTOP is ignored and deleted at the end
of issignal(). The solution is to deliver the signal via td_xsig only
when the process is currently stopped (and now continuing) and
otherwise (PT_ATTACH) using psignal(). This is similar to equivalent
code in NetBSD.

I was hoping if someone could review this to make sure I did the right
thing, because I'm not entirely familiar with this code.
#include <sys/types.h>
#include <sys/ptrace.h>
#include <sys/wait.h>
#include <signal.h>
#include <stdio.h>
#include <unistd.h>

int main( void ) {

        pid_t pid;
        int status;

        /* fork dummy child process */
        pid = fork();
        if( pid == 0 ) {
                /* child does nothing */
                for( ; ; ) {
                        sleep( 1 );
                }
        } else {
                /* parent */
                sleep( 1 );
                for( ; ; ) {
                        /* loop: attach, wait, detach */
                        printf( "attach " );
                        fflush( stdout );
                        ptrace( PT_ATTACH, pid, ( caddr_t ) 0, 0 );

                        printf( "wait " );
                        fflush( stdout );
                        wait4( pid, &status, 0, NULL );

                        printf( "detach " );
                        fflush( stdout );
                        ptrace( PT_DETACH, pid, ( caddr_t ) 1, 0 );

                        printf( "\n" );
                        fflush( stdout );
                }
        }

        return 0;
}
--- sys/kern/kern_sig.c.orig    2007-07-19 10:49:16.000000000 +0200
+++ sys/kern/kern_sig.c 2007-07-26 01:37:22.000000000 +0200
@@ -2543,6 +2543,20 @@
                        continue;
                }
                if (p->p_flag & P_TRACED && (p->p_flag & P_PPWAIT) == 0) {
+                       ksiginfo_t ksi;
+
+                       /*
+                        * clear old signal.
+                        * XXX shrug off debugger, it causes siginfo to
+                        * be thrown away.
+                        */
+                       ksiginfo_init( &ksi );
+                       sigqueue_get(&td->td_sigqueue, sig, &ksi);
+#ifdef KSE
+                       if (td->td_pflags & TDP_SA)
+                               SIGADDSET(td->td_sigmask, sig);
+#endif
+
                        /*
                         * If traced, always stop.
                         */
@@ -2550,20 +2564,7 @@
                        newsig = ptracestop(td, sig);
                        mtx_lock(&ps->ps_mtx);
 
-#ifdef KSE
-                       if (td->td_pflags & TDP_SA)
-                               SIGADDSET(td->td_sigmask, sig);
-
-#endif
                        if (sig != newsig) {
-                               ksiginfo_t ksi;
-                               /*
-                                * clear old signal.
-                                * XXX shrug off debugger, it causes siginfo to
-                                * be thrown away.
-                                */
-                               sigqueue_get(&td->td_sigqueue, sig, &ksi);
-
                                /*
                                 * If parent wants us to take the signal,
                                 * then it will leave it in p->p_xstat;
@@ -2585,6 +2586,9 @@
                                if (SIGISMEMBER(td->td_sigmask, sig))
                                        continue;
                                signotify(td);
+                       } else {
+                               /* Add old signal back. */
+                               sigqueue_add(&td->td_sigqueue, sig, &ksi);
                        }
 
                        /*
--- 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:
_______________________________________________
[email protected] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to "[EMAIL PROTECTED]"

Reply via email to