[prev in list] [next in list] [prev in thread] [next in thread] 

List:       freebsd-hackers
Subject:    Re: Ptrace segfault
From:       John Baldwin <jhb () freebsd ! org>
Date:       2010-04-30 12:51:56
Message-ID: 201004300851.56399.jhb () freebsd ! org
[Download RAW message or body]

On Thursday 29 April 2010 7:49:26 pm Gunnar Hinriksson wrote:
> 2010/4/29 Gunnar Hinriksson <tomtinn@gmail.com>:
> > 2010/4/29 Gunnar Hinriksson <tomtinn@gmail.com>:
> >> 2010/4/29 Bob Bishop <rb@gid.co.uk>:
> >>> Hi,
> >>>
> >>> On 29 Apr 2010, at 22:37, Garrett Cooper wrote:
> >>>
> >>>> On Thu, Apr 29, 2010 at 12:06 PM, Gunnar Hinriksson <tomtinn@gmail.com> 
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 <stdio.h>
> >>>>> #include <stdlib.h>
> >>>>> #include <sys/types.h>
> >>>>> #include <sys/ptrace.h>
> >>>>> #include <sys/wait.h>
> >>>>>
> >>>>> 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
_______________________________________________
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"
[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic