[PATCH] Try to use PTRACE_SETOPTIONS to handle non-ptrace SIGTRAP

[Roland McGrath]

Sorry for the delay in reviewing this.  I think this is mostly good,
but I do have a couple of concerns.

> +#ifdef USE_PTRACE_SETOPTIONS
> +	/* Don't set TCB_WAITEXECVE when PTRACE_SETOPTIONS works. */
> +	if (use_ptrace_setoptions > 0)
> +		return 0;
> +#endif

Given

> +# define use_ptrace_setoptions 0

above, you don't need that #ifdef, you can just leave the if-constant.

> +		/* Ignore post-execve trap. */
> +		if (is_post_execve_trap(tcp, status))
> +			goto tracing;
>  		/* we handled the STATUS, we are permitted to interrupt now. */
>  		if (interrupted)
>  			return 0;

Is there any particular reason to put this before the interrupt check?
Why not put it after, so interrupting strace works as promptly when it
just traced an execve as for any other syscall?

> +		/* Check for post-execve trap.  */
> +		if (((unsigned)status >> 16) == PTRACE_EVENT_EXEC) {
> +			/* PTRACE_O_TRACEEXEC works.  */
> +			use_ptrace_setoptions = 1;
> +			return 1;
> +		}
> +		if (ptrace(PTRACE_GETSIGINFO, tcp->pid, (void *)0,
> +			   (long)&si) == 0 &&
> +		    (tprintf("[si_signo = %d, si_code = %d]\n",
> +			     si.si_signo, si.si_code),
> +		    (si.si_signo != SIGTRAP ||
> +		     (si.si_code != SI_KERNEL && si.si_code != SI_USER))))
> +			/* PTRACE_O_TRACEEXEC does not work.  */
> +			use_ptrace_setoptions = 0;

I'm not very sanguine about this siginfo check.

We know there are kernels that implement PTRACE_SETOPTIONS so it
succeeds, but that don't check the option bits for unsupported ones.
In fact some such kernels support only PTRACE_O_TRACESYSGOOD and no
other options, in particular not PTRACE_O_TRACEEXEC.  So it is
important to check whether PTRACE_EVENT_EXEC ever shows up or not.
It's good that you are doing that here.  But I'm not really sure
about this method.

There are lots of ways to get SIGTRAP.  I'm not at all sure that there
isn't some other reasonable way to wind up with a SIGTRAP that has an
si_code value you don't expect.

It seems to me it would be far safer to start with the opposite
assumption.  That is, if PTRACE_SETOPTIONS returned success, don't
assume that anything works until you see it.  When you see a
SIGTRAP|0x80, set a flag for that and expect it from then on.  When
you see a PTRACE_EVENT_EXEC, set a flag for that and expect it from
then on.  The first time, you may also need to clear TCB_WAITEXECVE.
Until the seen-PTRACE_EVENT_EXEC flag is set, internal_exec must still
set it.

Also, there is a bug in your logic for a kernel that supports
PTRACE_O_TRACESYSGOOD but not PTRACE_O_TRACEEXEC.  When you detect a
post-execve SIGTRAP, you will reset use_ptrace_setoptions to zero.
Then, the next syscall report will use:

	if (!use_ptrace_setoptions)
		return WSTOPSIG(status) == SIGTRAP;

and fail to match because it's SIGTRAP | 0x80.  This too is solved
by maintaining a separate flag just for the TRACESYSGOOD state.


Thanks,
Roland