From owner-freebsd-hackers@FreeBSD.ORG Thu Aug 2 22:29:04 2007 Return-Path: Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id AC6C116A418 for ; Thu, 2 Aug 2007 22:29:04 +0000 (UTC) (envelope-from tijl@ulyssis.org) Received: from mailrelay011.isp.belgacom.be (mailrelay011.isp.belgacom.be [195.238.6.178]) by mx1.freebsd.org (Postfix) with ESMTP id 47D3813C442 for ; Thu, 2 Aug 2007 22:29:04 +0000 (UTC) (envelope-from tijl@ulyssis.org) Received: from 219.106-247-81.adsl-dyn.isp.belgacom.be (HELO kalimero.kotnet.org) ([81.247.106.219]) by mailrelay011.isp.belgacom.be with ESMTP; 02 Aug 2007 23:59:27 +0200 Received: from localhost (localhost [127.0.0.1]) by kalimero.kotnet.org (8.14.1/8.14.1) with ESMTP id l72LxQDx056765 for ; Thu, 2 Aug 2007 23:59:26 +0200 (CEST) (envelope-from tijl@ulyssis.org) From: Tijl Coosemans To: freebsd-hackers@freebsd.org Date: Thu, 2 Aug 2007 23:59:24 +0200 User-Agent: KMail/1.9.7 MIME-Version: 1.0 Content-Disposition: inline X-Length: 907 X-UID: 23 Content-Type: Multipart/Mixed; boundary="Boundary-00=_9OlsGsmBmDL0Wv2" Message-Id: <200708022359.25700.tijl@ulyssis.org> Subject: ptrace(2) race testcase + proposed patch X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 02 Aug 2007 22:29:04 -0000 --Boundary-00=_9OlsGsmBmDL0Wv2 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Content-Disposition: inline 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. --Boundary-00=_9OlsGsmBmDL0Wv2 Content-Type: text/plain; charset="us-ascii"; name="race.c" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="race.c" #include #include #include #include #include #include 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; } --Boundary-00=_9OlsGsmBmDL0Wv2 Content-Type: text/plain; charset="us-ascii"; name="patch-ptrace" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="patch-ptrace" --- 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: --Boundary-00=_9OlsGsmBmDL0Wv2--