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)) {
                                /*


------------------------------------------------------------------------------
What Every C/C++ and Fortran developer Should Know!
Read this article and learn how Intel has extended the reach of its 
next-generation tools to help Windows* and Linux* C/C++ and Fortran 
developers boost performance applications - including clusters. 
http://p.sf.net/sfu/intel-dev2devmay
_______________________________________________
Strace-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/strace-devel

Reply via email to