Ptrace segfault
John Baldwin
jhb at freebsd.org
Fri Apr 30 14:44:13 UTC 2010
On Thursday 29 April 2010 7:49:26 pm Gunnar Hinriksson wrote:
> 2010/4/29 Gunnar Hinriksson <tomtinn at gmail.com>:
> > 2010/4/29 Gunnar Hinriksson <tomtinn at gmail.com>:
> >> 2010/4/29 Bob Bishop <rb at 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 at 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 at freebsd.org mailing list
> >>>> http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
> >>>> To unsubscribe, send any mail to "freebsd-hackers-
unsubscribe at freebsd.org"
> >>>>
> >>>>
> >>>
> >>>
> >>> --
> >>> Bob Bishop +44 (0)118 940 1243
> >>> rb at 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
More information about the freebsd-hackers
mailing list