[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