[PATCH] correctly handle "kill -TRAP $straced_programs_pid" and int3
Denys Vlasenko
dvlasenk at redhat.com
Mon Dec 29 19:32:36 UTC 2008
On Tue, 2008-12-23 at 14:06 -0800, Roland McGrath wrote:
> > I put relevant code into #ifdef LINUX as I unsure that
> > corresponding PTRACE_xxx constants even exist on other OSes.
>
> They don't exist in all Linux versions either. If there is another
> ptrace-using OS that defines the Linux-invented ptrace constants, I think
> we can assume they work compatibly. You should test #if defined PTRACE_FOO
> for the particular ones you rely on.
Yes, it failed to compile on old RHEL on ia64. Fixed that.
Currently it is guarded with this:
/* PTRACE_SETOPTIONS is not a #define. PT_SETOPTIONS is. */
/* Add more OSes after you verified it works for them. */
#if defined LINUX && defined PT_SETOPTIONS
...
> > Still, yell at me if something broke as a result,
> > and I will revert or fix it.
>
> In the past we avoided relying on PTRACE_SETOPTIONS in strace, because
> older Linux kernels (2.4 and early 2.6) either didn't have it or had spotty
> support. IIRC, there were some kernels that have PTRACE_SETOPTIONS but
> only support some of the option bits, yet it always returns success with no
> indicator of what is or isn't really supported.
Gaack.
> I think your patch is modest enough that it can be safe even with this,
> with just a little cleanup.
>
> "sigtrap80" is just a terrible name for a struct member. You're recording
> the status that's returned for syscall stops,
No, I do not, I record what WSTOPSIG() value do I expect from ptrace stops.
This member always contains SIGTRAP or SIGTRAP | 0x80, but
int sigtrap_or_sigtrap_ored_with_0x80 is a bit long. :)
Will you feel better with ptrace_stop_sig name?
> I'm not sure we really need a field in struct tcb at all. The feature is
> either supported by the kernel or it's not. If it is, we'll use it
> consistently for all the tasks we ptrace. Why not just use a global?
Hmm, good idea.
ptrace manpage says nothing about inheritance of these flags across clone.
In practice, they are inherited. Can we rely on this?
> To be robust to the aforementioned untrustworthy old kernels, all you
> really have to do is not presume it worked. That is, set syscall_status
> the first time you see a SIGTRAP|0x80 or (PTRACE_EVENT_EXEC<<8)|SIGTRAP.
> Before that, assume as now that any SIGTRAP you see might or might not
> actually be a syscall report. You can cache the fact if trying
> PTRACE_SETOPTIONS failed (with other than ESRCH), so you don't make the
> wasted ptrace call on every new thread on those old kernels. But don't
> assume just because PTRACE_SETOPTIONS did succeed that it really worked.
> Only when you've seen a new-style report once can you then expect that
> PTRACE_SETOPTIONS works every time.
I will commit attached patch. Please take a look.
--
vda
diff -d -urpN strace.6/defs.h strace.7/defs.h
--- strace.6/defs.h 2008-12-22 20:14:47.000000000 +0100
+++ strace.7/defs.h 2008-12-29 19:34:21.000000000 +0100
@@ -324,8 +324,6 @@ struct tcb {
int nclone_waiting; /* clone threads in wait4 (TCB_SUSPENDED) */
/* (1st arg of wait4()) */
#endif
- int sigtrap80; /* What sig we consider to be ptrace stop */
- /* (can be SIGTRAP or (SIGTRAP|0x80) only) */
long baddr; /* `Breakpoint' address */
long inst[2]; /* Instructions on above */
int pfd; /* proc file descriptor */
diff -d -urpN strace.6/strace.c strace.7/strace.c
--- strace.6/strace.c 2008-12-23 17:14:42.000000000 +0100
+++ strace.7/strace.c 2008-12-29 20:05:37.000000000 +0100
@@ -77,6 +77,8 @@
#endif
#endif
#endif
+extern char **environ;
+
int debug = 0, followfork = 0;
int dtime = 0, cflag = 0, xflag = 0, qflag = 0;
@@ -87,6 +89,8 @@ int not_failing_only = 0;
static int exit_code = 0;
static int strace_child = 0;
+static int ptrace_stop_sig = SIGTRAP;
+static bool ptrace_opts_set;
static char *username = NULL;
uid_t run_uid;
@@ -99,7 +103,6 @@ FILE *outf;
struct tcb **tcbtab;
unsigned int nprocs, tcbtabsize;
char *progname;
-extern char **environ;
static int detach P((struct tcb *tcp, int sig));
static int trace P((void));
@@ -943,7 +946,6 @@ alloc_tcb(int pid, int command_options_p
tcp->nclone_waiting = 0;
#endif
tcp->flags = TCB_INUSE | TCB_STARTUP;
- tcp->sigtrap80 = SIGTRAP;
tcp->outf = outf; /* Initialise to current out file */
tcp->stime.tv_sec = 0;
tcp->stime.tv_usec = 0;
@@ -1550,7 +1552,7 @@ int sig;
break;
}
error = ptrace_restart(PTRACE_CONT, tcp,
- WSTOPSIG(status) == tcp->sigtrap80 ? 0
+ WSTOPSIG(status) == ptrace_stop_sig ? 0
: WSTOPSIG(status));
if (error < 0)
break;
@@ -2416,17 +2418,24 @@ Process %d attached (waiting for parent)
* on ptrace-generated SIGTRAPs, and mark
* execve's SIGTRAP with PTRACE_EVENT_EXEC.
*/
- if (tcp->sigtrap80 == SIGTRAP
- && ptrace(PTRACE_SETOPTIONS, pid, (char *) 0,
- (void *) (PTRACE_O_TRACESYSGOOD | PTRACE_O_TRACEEXEC)) == 0) {
- tcp->sigtrap80 = SIGTRAP | 0x80;
+ if (!ptrace_opts_set) {
+ ptrace_opts_set = 1;
+ /*
+ * NB: even if this "succeeds", we can
+ * revert back to SIGTRAP if we later see
+ * that it didnt really work.
+ * Old kernels are known to lie here.
+ */
+ if (ptrace(PTRACE_SETOPTIONS, pid, (char *) 0,
+ (void *) (PTRACE_O_TRACESYSGOOD | PTRACE_O_TRACEEXEC)) == 0)
+ ptrace_stop_sig = SIGTRAP | 0x80;
}
#endif
goto tracing;
}
#if defined LINUX && defined PT_SETOPTIONS
- if (tcp->sigtrap80 != SIGTRAP && WSTOPSIG(status) == SIGTRAP) {
+ if (ptrace_stop_sig != SIGTRAP && WSTOPSIG(status) == SIGTRAP) {
/*
* We told ptrace to report SIGTRAP | 0x80 on this process
* but got bare SIGTRAP. This can be a genuine SIGTRAP:
@@ -2458,27 +2467,14 @@ Process %d attached (waiting for parent)
if (si.si_signo != SIGTRAP
|| (si.si_code != SI_KERNEL && si.si_code != SI_USER)
) {
- fprintf(stderr, "bogus SIGTRAP (si_code:%x), assuming it's ptrace stop\n", si.si_code);
- /* Set WSTOPSIG(status) = (SIGTRAP | 0x80). */
- status |= 0x8000;
+ fprintf(stderr, "bogus SIGTRAP (si_code:%x), assuming old kernel\n", si.si_code);
+ ptrace_stop_sig = SIGTRAP;
}
}
}
-
- if (WSTOPSIG(status) == (SIGTRAP | 0x80)
- /* && tcp->sigtrap80 == SIGTRAP - redundant */
- ) {
- /*
- * If tcp->sigtrap80 == SIGTRAP but we got it
- * ORed with 0x80, it's a CLONE_PTRACEd child
- * which inherited "SIGTRAP | 0x80" setting.
- * Whee. Just record this remarkable fact.
- */
- tcp->sigtrap80 = (SIGTRAP | 0x80);
- }
#endif
- if (WSTOPSIG(status) != tcp->sigtrap80) {
+ if (WSTOPSIG(status) != ptrace_stop_sig) {
/* This isn't a ptrace stop. */
if (WSTOPSIG(status) == SIGSTOP &&
More information about the Strace-devel
mailing list