Date: Fri, 30 Apr 2010 08:51:56 -0400 From: John Baldwin <jhb@freebsd.org> To: freebsd-hackers@freebsd.org Cc: Gunnar Hinriksson <tomtinn@gmail.com>, bde@freebsd.org Subject: Re: Ptrace segfault Message-ID: <201004300851.56399.jhb@freebsd.org> In-Reply-To: <w2lcbb19c781004291649p96fb1c68scef0bce53ea253b3@mail.gmail.com> References: <q2vcbb19c781004291206sc54fdb6ag53c3a763ad364e8e@mail.gmail.com> <m2wcbb19c781004291636rd7d5709dr81563f4ce6f4bf25@mail.gmail.com> <w2lcbb19c781004291649p96fb1c68scef0bce53ea253b3@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201004300851.56399.jhb>