From owner-freebsd-hackers@FreeBSD.ORG Fri Apr 30 14:44:13 2010 Return-Path: Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 76317106566B; Fri, 30 Apr 2010 14:44:13 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from cyrus.watson.org (cyrus.watson.org [65.122.17.42]) by mx1.freebsd.org (Postfix) with ESMTP id 37ACF8FC17; Fri, 30 Apr 2010 14:44:13 +0000 (UTC) Received: from bigwig.baldwin.cx (66.111.2.69.static.nyinternet.net [66.111.2.69]) by cyrus.watson.org (Postfix) with ESMTPSA id AE62646B35; Fri, 30 Apr 2010 10:44:12 -0400 (EDT) Received: from jhbbsd.localnet (smtp.hudson-trading.com [209.249.190.9]) by bigwig.baldwin.cx (Postfix) with ESMTPA id 5C71B8A025; Fri, 30 Apr 2010 10:44:00 -0400 (EDT) From: John Baldwin To: freebsd-hackers@freebsd.org Date: Fri, 30 Apr 2010 08:51:56 -0400 User-Agent: KMail/1.12.1 (FreeBSD/7.3-CBSD-20100217; KDE/4.3.1; amd64; ; ) References: In-Reply-To: MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201004300851.56399.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.0.1 (bigwig.baldwin.cx); Fri, 30 Apr 2010 10:44:00 -0400 (EDT) X-Virus-Scanned: clamav-milter 0.95.1 at bigwig.baldwin.cx X-Virus-Status: Clean X-Spam-Status: No, score=-2.6 required=4.2 tests=AWL,BAYES_00 autolearn=ham version=3.2.5 X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on bigwig.baldwin.cx Cc: Gunnar Hinriksson , bde@freebsd.org Subject: Re: Ptrace segfault 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: Fri, 30 Apr 2010 14:44:13 -0000 On Thursday 29 April 2010 7:49:26 pm Gunnar Hinriksson wrote: > 2010/4/29 Gunnar Hinriksson : > > 2010/4/29 Gunnar Hinriksson : > >> 2010/4/29 Bob Bishop : > >>> Hi, > >>> > >>> On 29 Apr 2010, at 22:37, Garrett Cooper wrote: > >>> > >>>> On Thu, Apr 29, 2010 at 12:06 PM, Gunnar Hinriksson wrote: > >>>>> Hello > >>>>> > >>>>> Im having a little problem using ptrace on my system. > >>>>> If I use ptrace to attach to another process the child process > >>>>> segfaults once I detach. > >>>>> For example using this simple program. > >>>>> > >>>>> #include > >>>>> #include > >>>>> #include > >>>>> #include > >>>>> #include > >>>>> > >>>>> int main(int argc, char *argv[]) > >>>>> { > >>>>> int pid = atoi(argv[1]); > >>>>> ptrace(PT_ATTACH, pid, 0, 0); > >>>>> wait(NULL); > >>>>> ptrace(PT_DETACH, pid, 0, 0); > >>>>> return 0; > >>>>> } > >>>>> > >>>>> Am I using ptrace incorrectly or is there perhaps a bug in ptrace that > >>>>> causes the child to always segfault ? > >>>> > >>>> Nope -- it's a bug in your code. From ptrace(2): > >>>> > >>>> PT_CONTINUE The traced process continues execution. The addr argument > >>>> is an address specifying the place where execution is to be > >>>> resumed (a new value for the program counter), or > >>>> (caddr_t)1 to indicate that execution is to pick up where > >>>> it left off. The data argument provides a signal number to > >>>> be delivered to the traced process as it resumes execution, > >>>> or 0 if no signal is to be sent. > >>>> > >>>> [...] > >>>> > >>>> PT_DETACH This request is like PT_CONTINUE, except that it does not > >>> ^^^^^^^^^^^ > >>>> allow specifying an alternate place to continue execution, > >>> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > >>>> and after it succeeds, the traced process is no longer > >>>> traced and continues execution normally. > >>>> > >>>> Note very carefully the fact that PT_DETACH is like PT_CONTINUE, > >>>> and that PT_CONTINUE says that addr references the memory where the > >>>> execution is going to be resumed. > >>> > >>> Looks to me like a bug in ptrace(PT_DETACH,...) which to agree with ptrace(2) ought either to > >>> (a) fail (EINVAL?) if addr != (caddr_t)1, or > >>> (b) ignore addr entirely; it's not clear which. > >>> > >>> OP inferred (b) which is reasonable. > >>> > >>>> HTH, > >>>> -Garrett > >>>> _______________________________________________ > >>>> freebsd-hackers@freebsd.org mailing list > >>>> http://lists.freebsd.org/mailman/listinfo/freebsd-hackers > >>>> To unsubscribe, send any mail to "freebsd-hackers- unsubscribe@freebsd.org" > >>>> > >>>> > >>> > >>> > >>> -- > >>> Bob Bishop +44 (0)118 940 1243 > >>> rb@gid.co.uk fax +44 (0)118 940 1295 > >>> mobile +44 (0)783 626 4518 > >>> > >>> > >>> > >>> > >>> > >>> > >> > >> Hello > >> > >> I didn't want to make the code to big so I omitted any tests. > >> About wait(), you are supposed to use wait to check if the child > >> process has stopped successfully. > >> I guess the correct usage would be something like this if im > >> understanding the documentation correctly. > >> if ( wait(WIFSTOPPED(SIGSTOP)) == pid ) > >> Im not sure if there is any posix way of describing how ptrace should > >> be implemented but I know linux ptrace ignores the addr on DETACH. > >> So my vote would be that ptrace ignores addr on detach to make it > >> compatible with code for linux. > >> > >> With Regards > >> > >> Gunnar > >> > > > > Hello > > > > I started looking around in the kernel sources and I think i've found > > out how it is implemented. > > in /usr/src/sys/kern/sys_process.c line 744 the following code is found. > > > > if (addr != (void *)1) { > > error = ptrace_set_pc(td2, (u_long) (uintfptr_t)addr); > > if (error) > > break; > > } > > > > So it is changing the address if the addr is anything but 1. > > I tested my program by passing 1 as an argument in the addr field and > > the segfaults stopped. > > > > So the question is if this code should be removed so that PT_DETACH > > ignores addr or leave it be and perhaps update the documentation to > > reflect that it allows the addr to the changed. > > > > With Regards > > > > Gunnar > > > > After reading the code more carefully the correct way would be to > modify it like this. > --- > case PT_CONTINUE: > { > if (addr != (void *)1) { > error = ptrace_set_pc(td2, (u_long) (uintfptr_t)addr); > } > } > if (error) > break; > --- > Note that im just learning programming so this might not be the > correct way to do it. I think this might be the cleanest. None of the other commands here need to adjust the pc (PT_STEP requires that addr be '1' in the manpage for example), so this change cleans up the logic some by only trying to set the pc for PT_CONTINUE. It also moves some PT_DETACH logic up into the switch as a PT_DETACH case as well. Index: sys_process.c =================================================================== --- sys_process.c (revision 207329) +++ sys_process.c (working copy) @@ -899,6 +899,14 @@ if (error) goto out; break; + case PT_CONTINUE: + if (addr != (void *)1) { + error = ptrace_set_pc(td2, + (u_long)(uintfptr_t)addr); + if (error) + goto out; + } + break; case PT_TO_SCE: p->p_stops |= S_PT_SCE; break; @@ -908,15 +916,7 @@ case PT_SYSCALL: p->p_stops |= S_PT_SCE | S_PT_SCX; break; - } - - if (addr != (void *)1) { - error = ptrace_set_pc(td2, (u_long)(uintfptr_t)addr); - if (error) - break; - } - - if (req == PT_DETACH) { + case PT_DETACH: /* reset process parent */ if (p->p_oppid != p->p_pptr->p_pid) { struct proc *pp; @@ -941,6 +941,7 @@ /* should we send SIGCHLD? */ /* childproc_continued(p); */ + break; } sendsig: -- John Baldwin