[PATCH 3/3] Handle followfork using ptrace_setoptions if available

Roland McGrath roland at redhat.com
Tue Oct 5 04:45:48 UTC 2010


> +++ b/process.c
> @@ -794,6 +794,8 @@ change_syscall(struct tcb *tcp, int new)
>  #ifdef LINUX
>  void reparent(struct tcb *tcp, struct tcb *tcpchild)
>  {
> +	if (sysent[tcp->scno].sys_func != sys_clone)
> +		return;

I'm not entirely clear on why this is part of the third patch instead of
the first.  I guess it doesn't really matter.

>  int
>  internal_fork(struct tcb *tcp)
>  {
> +	if ((ptrace_setoptions
> +	    & (PTRACE_O_TRACECLONE | PTRACE_O_TRACEFORK | PTRACE_O_TRACEVFORK))
> +	   == (PTRACE_O_TRACECLONE | PTRACE_O_TRACEFORK | PTRACE_O_TRACEVFORK))
> +		return 0;

I don't think you want to skip all of internal_fork.
e.g. I think fork_tcb is still useful.

I think you just want to conditionalize the setbpt call.

> +int handle_ptrace_event(int status, struct tcb *tcp)

This can be static.

> +		ptrace(PTRACE_GETEVENTMSG, tcp->pid, NULL, &childpid);

Probably all the ptrace calls you make on tracees should use do_ptrace.
You need to check for failure.  If ptrace returns ESRCH, that can be
"normal" meaning the child suddenly died before we got here.  Other
errors are unexpected but need to be diagnosed.

I'm not really clear on why you made a whole separate copy of this
code.  The guts are mostly the same as part of internal_fork, so you
could move them into a common function.

> +#ifdef LINUX
> +			if (followfork && (tcp->parent == NULL) && ptrace_setoptions)
> +				if (ptrace(PTRACE_SETOPTIONS, tcp->pid,
> +				    NULL, ptrace_setoptions) < 0)
> +					/* should not happen. */
> +					ptrace_setoptions = 0;
> +#endif

This error checking is wrong.  An ESRCH failure is a "normal" case of a
child dying, and should not disable using the feature.  Other errors
should be diagnosed.


Thanks,
Roland




More information about the Strace-devel mailing list