[PATCH] make strace handle SIGTRAP properly
Denys Vlasenko
dvlasenk at redhat.com
Fri May 20 16:02:14 UTC 2011
On Fri, 2011-05-20 at 14:08 +0200, Denys Vlasenko wrote:
> Hi,
>
> (please don't remove me from CC: when replying)
>
> During recent lkml discussions about fixing some long-standing problems
> with ptrace, I had to look in strace source and experiment with it a
> bit. (CC-ing some participants).
>
> One irritating thing I noticed is that we *still* don't handle
> user-generated SIGTRAPs. There are users who do want that to work:
>
> https://bugzilla.redhat.com/show_bug.cgi?id=162774
>
> Currently, strace "handles" SIGTRAP by code like this:
>
> #if defined (I386)
> if (upeek(tcp, 4*EAX, &eax) < 0)
> return -1;
> if (eax != -ENOSYS && !(tcp->flags & TCB_INSYSCALL)) {
> if (debug)
> fprintf(stderr, "stray syscall exit: eax = %ld\n", eax);
> return 0;
> }
> #elif ...
>
> Think what will happen if SIGTRAP arrives while process is in userspace
> and eax happened to contain value equal to -ENOSYS...
>
> In order to make it work properly, that is, to detect SIGTRAPs just like
> we do with any other signals, we need to use ptrace(PTRACE_SETOPTIONS,
> PTRACE_O_TRACESYSGOOD). Otherwise, syscall traps are indistinguishable
> from real SIGTRAPs. Well, there is some extremely fragile and/or
> arch-specific way way to distinguish them by looking at GETSIGINFO
> results or registers (see above), but why should we bother? Linux ptrace
> has PTRACE_O_TRACESYSGOOD for many years now, we _can_ use it.
>
> Another twist in this story is that pesky extra SIGTRAP generated after
> execve. If we start handling real SIGTRAPs, we should not be fooled by
> it.
>
> And again, even though there may be a difficult way to detect it, I
> think we should use the easy one: PTRACE_O_TRACEEXEC option. It disables
> post-execve SIGTRAP (and enables intra-execve PTRACE_EVENT_EXEC, which
> is easily distinguishable form other stops, so we can ignore it easily).
>
> Roland's worry was that in some old kernels ptrace(PTRACE_SETOPTIONS)
> was working, but the options themselves were buggy. I trust him that
> there were such kernels, but it's history now, right?
> So, I propose to attempt ptrace(PTRACE_SETOPTIONS) only on relatively
> recent kernels. (say, starting from 2.6.30? (arbitrarily chosen))
>
> Any objections to this plan?
And here's the patch against current strace git. Run-tested.
--
vda
diff -d -urpN strace.2/defs.h strace.3/defs.h
--- strace.2/defs.h 2011-05-16 15:53:49.000000000 +0200
+++ strace.3/defs.h 2011-05-20 15:31:44.136409489 +0200
@@ -504,7 +504,7 @@ typedef enum {
extern struct tcb **tcbtab;
extern int *qual_flags;
extern int debug, followfork;
-extern unsigned int ptrace_setoptions;
+extern unsigned int ptrace_setoptions_followfork;
extern int dtime, xflag, qflag;
extern cflag_t cflag;
extern int acolumn;
diff -d -urpN strace.2/process.c strace.3/process.c
--- strace.2/process.c 2011-05-16 15:53:49.000000000 +0200
+++ strace.3/process.c 2011-05-20 15:31:44.134409501 +0200
@@ -915,7 +915,7 @@ Process %u resumed (parent %d ready)\n",
int
internal_fork(struct tcb *tcp)
{
- if ((ptrace_setoptions
+ if ((ptrace_setoptions_followfork
& (PTRACE_O_TRACECLONE | PTRACE_O_TRACEFORK | PTRACE_O_TRACEVFORK))
== (PTRACE_O_TRACECLONE | PTRACE_O_TRACEFORK | PTRACE_O_TRACEVFORK))
return 0;
diff -d -urpN strace.2/strace.c strace.3/strace.c
--- strace.2/strace.c 2011-05-16 15:53:49.000000000 +0200
+++ strace.3/strace.c 2011-05-20 17:58:43.834017263 +0200
@@ -33,6 +33,7 @@
#include "defs.h"
#include <sys/types.h>
+#include <stdarg.h>
#include <signal.h>
#include <errno.h>
#include <sys/param.h>
@@ -83,7 +84,10 @@ extern char *optarg;
int debug = 0, followfork = 0;
-unsigned int ptrace_setoptions = 0;
+unsigned int ptrace_setoptions_followfork = 0;
+static unsigned int ptrace_setoptions_for_all = 0;
+/* Which WSTOPSIG(status) value marks syscall traps? */
+static unsigned int SYSCALLTRAP = SIGTRAP;
int dtime = 0, xflag = 0, qflag = 0;
cflag_t cflag = CFLAG_NONE;
static int iflag = 0, interactive = 0, pflag_seen = 0, rflag = 0, tflag = 0;
@@ -211,6 +215,27 @@ usage: strace [-CdDffhiqrtttTvVxxy] [-a
exit(exitval);
}
+static void error_msg_and_die(const char *fmt, ...) /* TODO: __attribute__ ((noreturn, format (printf, 1, 2))) */ ;
+static void error_msg_and_die(const char *fmt, ...)
+{
+ char *msg;
+ va_list p;
+
+ va_start(p, fmt);
+ msg = NULL;
+ vasprintf(&msg, fmt, p);
+ if (msg) {
+ fprintf(stderr, "%s: %s\n", progname, msg);
+ free(msg);
+ }
+ va_end(p);
+
+ cflag = 0; /* or else cleanup may print summary */
+ cleanup();
+ fflush(NULL);
+ _exit(1);
+}
+
#ifdef SVR4
#ifdef MIPS
void
@@ -702,7 +727,7 @@ startup_child (char **argv)
* and then see which options are supported by the kernel.
*/
static int
-test_ptrace_setoptions(void)
+test_ptrace_setoptions_followfork(void)
{
int pid, expected_grandchild = 0, found_grandchild = 0;
const unsigned int test_options = PTRACE_O_TRACECLONE |
@@ -727,7 +752,7 @@ test_ptrace_setoptions(void)
continue;
else if (errno == ECHILD)
break;
- perror("test_ptrace_setoptions");
+ perror("test_ptrace_setoptions_followfork");
return -1;
}
if (tracee_pid != pid) {
@@ -761,9 +786,82 @@ test_ptrace_setoptions(void)
}
}
if (expected_grandchild && expected_grandchild == found_grandchild)
- ptrace_setoptions |= test_options;
+ ptrace_setoptions_followfork |= test_options;
return 0;
}
+
+/*
+ * Test whether the kernel support PTRACE_O_TRACESYSGOOD.
+ * First fork a new child, call ptrace(PTRACE_SETOPTIONS) on it,
+ * and then see whether it will stop with (SIGTRAP | 0x80).
+ */
+static void
+test_ptrace_setoptions_for_all(void)
+{
+ const unsigned int test_options = PTRACE_O_TRACESYSGOOD | PTRACE_O_TRACEEXEC;
+ int pid;
+ int it_worked = 0;
+
+ pid = fork();
+ if (pid < 0)
+ error_msg_and_die("fork failed");
+
+ if (pid == 0) {
+ pid = getpid();
+ if (ptrace(PTRACE_TRACEME, 0L, 0L, 0L) < 0)
+ /* "parent, something is deeply wrong!" */
+ kill(pid, SIGKILL);
+ kill(pid, SIGSTOP);
+ _exit(0); /* parent should see entry into this syscall */
+ }
+
+ while (1) {
+ int status, tracee_pid;
+
+ errno = 0;
+ tracee_pid = wait(&status);
+ if (tracee_pid <= 0) {
+ if (errno == EINTR)
+ continue;
+ kill(pid, SIGKILL);
+ error_msg_and_die("%s: unexpected wait result %d", __func__, tracee_pid);
+ }
+ if (WIFEXITED(status))
+ break;
+ if (!WIFSTOPPED(status)) {
+ kill(pid, SIGKILL);
+ error_msg_and_die("%s: unexpected wait status %x", __func__, status);
+ }
+ if (WSTOPSIG(status) == SIGSTOP) {
+ /*
+ * We don't check "options aren't accepted" error.
+ * If it happens, we'll never get (SIGTRAP | 0x80),
+ * and thus will decide to not use the option.
+ * IOW: the outcome of the test will be the same.
+ */
+ ptrace(PTRACE_SETOPTIONS, pid, 0L, test_options);
+ }
+ if (WSTOPSIG(status) == (SIGTRAP | 0x80)) {
+ it_worked = 1;
+ }
+ if (ptrace(PTRACE_SYSCALL, pid, 0L, 0L) < 0) {
+ kill(pid, SIGKILL);
+ error_msg_and_die("%s: PTRACE_SYSCALL doesn't work");
+ }
+ }
+
+ if (it_worked) {
+ SYSCALLTRAP = (SIGTRAP | 0x80);
+ ptrace_setoptions_for_all = test_options;
+ if (debug)
+ fprintf(stderr, "ptrace_setoptions_for_all = %#x\n",
+ ptrace_setoptions_for_all);
+ return;
+ }
+
+ fprintf(stderr,
+ "Test for PTRACE_O_TRACESYSGOOD failed, giving up using this feature.\n");
+}
#endif
int
@@ -977,16 +1075,17 @@ main(int argc, char *argv[])
#ifdef LINUX
if (followfork) {
- if (test_ptrace_setoptions() < 0) {
+ if (test_ptrace_setoptions_followfork() < 0) {
fprintf(stderr,
"Test for options supported by PTRACE_SETOPTIONS "
"failed, giving up using this feature.\n");
- ptrace_setoptions = 0;
+ ptrace_setoptions_followfork = 0;
}
if (debug)
- fprintf(stderr, "ptrace_setoptions = %#x\n",
- ptrace_setoptions);
+ fprintf(stderr, "ptrace_setoptions_followfork = %#x\n",
+ ptrace_setoptions_followfork);
}
+ test_ptrace_setoptions_for_all();
#endif
/* Check if they want to redirect the output. */
@@ -1751,7 +1850,7 @@ int sig;
break;
}
error = ptrace_restart(PTRACE_CONT, tcp,
- WSTOPSIG(status) == SIGTRAP ? 0
+ WSTOPSIG(status) == SYSCALLTRAP ? 0
: WSTOPSIG(status));
if (error < 0)
break;
@@ -2408,6 +2507,11 @@ handle_ptrace_event(int status, struct t
}
return handle_new_child(tcp, childpid, 0);
}
+ if (status >> 16 == PTRACE_EVENT_EXEC) {
+ if (debug)
+ fprintf(stderr, "PTRACE_EVENT_EXEC on pid %d (ignored)\n", tcp->pid);
+ return 0;
+ }
return 1;
}
#endif
@@ -2597,7 +2701,7 @@ Process %d attached (waiting for parent)
fprintf(stderr, "pid %u stopped, [%s]\n",
pid, signame(WSTOPSIG(status)));
- if (ptrace_setoptions && (status >> 16)) {
+ if (status >> 16) {
if (handle_ptrace_event(status, tcp) != 1)
goto tracing;
}
@@ -2630,16 +2734,24 @@ Process %d attached (waiting for parent)
}
}
#ifdef LINUX
- if (followfork && (tcp->parent == NULL) && ptrace_setoptions)
- if (ptrace(PTRACE_SETOPTIONS, tcp->pid,
- NULL, ptrace_setoptions) < 0 &&
- errno != ESRCH)
- ptrace_setoptions = 0;
+ int options = ptrace_setoptions_for_all;
+ if (followfork && (tcp->parent == NULL))
+ options |= ptrace_setoptions_followfork;
+ if (options) {
+ if (debug)
+ fprintf(stderr, "setting opts %x on pid %d", options, tcp->pid);
+ if (ptrace(PTRACE_SETOPTIONS, tcp->pid, NULL, options) < 0) {
+ if (errno != ESRCH) {
+ /* Should never happen, really */
+ error_msg_and_die("PTRACE_SETOPTIONS");
+ }
+ }
+ }
#endif
goto tracing;
}
- if (WSTOPSIG(status) != SIGTRAP) {
+ if (WSTOPSIG(status) != SYSCALLTRAP) {
if (WSTOPSIG(status) == SIGSTOP &&
(tcp->flags & TCB_SIGTRAPPED)) {
/*
More information about the Strace-devel
mailing list