On Tue, 2008-12-09 at 21:57 -0800, Roland McGrath wrote:
> > >  but probably better instead to have a
> > > check for a saved ptrace error in the main loop after the WIFSIGNALED and
> > > WIFEXITED cases.  If we get further and there is a saved ptrace error, 
> > > then
> > > print the "<unavailable>" or perhaps something with the strerror string.
> > 
> > I'm not sure what are you referring to by "get further".
> 
> I mean when it didn't take the WIFSIGNALED or WIFEXITED cases (which do
> "continue;") so it gets to:
> 
>               if (!WIFSTOPPED(status)) {
>                       fprintf(stderr, "PANIC: pid %u not stopped\n", pid);
>                       droptcb(tcp);
>                       continue;
>               }
>               if (debug)
>                       fprintf(stderr, "pid %u stopped, [%s]\n",
>                               pid, signame(WSTOPSIG(status)));

I am not sure there are guarantees this will necessarily happen.


> kill(20732, SIGKILL)                   = ? <unavailable>
> +++ killed by SIGKILL +++
> 
> In another case where there was result-arg printing, that might be:
> 
> foo(123, 345, ... <unavailable>)      = ? <unavailable>

Done

>               if (trace_syscall(tcp) < 0 && !tcp->ptrace_errno) {
> 
> or something along those lines.
> 
> > With it, I basically blindly reset error indicator and blindly
> > perform PTRACE_SYSCALL. I can't just fall through
> > to already existing PTRACE_SYSCALL further below, it would error
> > out. Does not look too nice.
> 
> Please make the "tracing:" call apply the logic I described above.
> 
> Probably the right place to reset ptrace_errno is in printleader or
> whereever exactly it's used to decide to print something.

Done

Please see updated patch below. Not run-tested yet.
--
vda



diff -d -urpN strace.1/defs.h strace.2/defs.h
--- strace.1/defs.h     2008-12-04 19:31:05.000000000 +0100
+++ strace.2/defs.h     2008-12-09 17:37:28.000000000 +0100
@@ -336,6 +336,7 @@ struct tcb {
        prstatus_t status;      /* procfs status structure */
 #endif
 #endif
+       int ptrace_errno;
 #ifdef FREEBSD
        struct procfs_status status;
        int pfd_reg;
@@ -466,6 +467,10 @@ extern void set_overhead P((int));
 extern void qualify P((char *));
 extern int get_scno P((struct tcb *));
 extern long known_scno P((struct tcb *));
+extern long secure_ptrace P((int request, struct tcb *tcp, void *addr, void 
*data));
+extern long secure_ptrace_SYSCALL P((struct tcb *tcp, int sig));
+extern long secure_ptrace_CONT P((struct tcb *tcp, int sig));
+extern long secure_ptrace_DETACH P((struct tcb *tcp, int sig));
 extern int trace_syscall P((struct tcb *));
 extern int count_syscall P((struct tcb *, struct timeval *));
 extern void printxval P((const struct xlat *, int, const char *));
diff -d -urpN strace.1/process.c strace.2/process.c
--- strace.1/process.c  2008-12-04 19:16:29.000000000 +0100
+++ strace.2/process.c  2008-12-09 19:00:00.000000000 +0100
@@ -964,7 +964,8 @@ struct tcb *tcp;
 
                        tcpchild->flags &= ~(TCB_SUSPENDED|TCB_STARTUP);
                        if (ptrace(PTRACE_SYSCALL, pid, (char *) 1, 0) < 0) {
-                               perror("resume: ptrace(PTRACE_SYSCALL, ...)");
+                               if (errno != ESRCH) /* if not killed meanwhile 
*/
+                                       perror("resume: ptrace(PTRACE_SYSCALL, 
...)");
                                return -1;
                        }
 
diff -d -urpN strace.1/strace.c strace.2/strace.c
--- strace.1/strace.c   2008-12-04 19:13:06.000000000 +0100
+++ strace.2/strace.c   2008-12-10 10:18:43.000000000 +0100
@@ -1372,8 +1372,7 @@ struct tcb *tcp;
                tcp->parent->nclone_waiting--;
 #endif
 
-       if (ptrace(PTRACE_SYSCALL, tcp->pid, (char *) 1, 0) < 0) {
-               perror("resume: ptrace(PTRACE_SYSCALL, ...)");
+       if (secure_ptrace_SYSCALL(tcp, 0) < 0) {
                return -1;
        }
 
@@ -1453,6 +1452,7 @@ resume_from_tcp (struct tcb *tcp)
    Never call DETACH twice on the same process as both unattached and
    attached-unstopped processes give the same ESRCH.  For unattached process we
    would SIGSTOP it and wait for its SIGSTOP notification forever.  */
+/* TODO: no callers check return value, can return void as well */
 
 static int
 detach(tcp, sig)
@@ -1491,12 +1491,13 @@ int sig;
         * detached process would be left stopped (process state T).
         */
        catch_sigstop = (tcp->flags & TCB_STARTUP);
-       if ((error = ptrace(PTRACE_DETACH, tcp->pid, (char *) 1, sig)) == 0) {
+       error = secure_ptrace_DETACH(tcp, sig);
+       if (error == 0) {
                /* On a clear day, you can see forever. */
        }
        else if (errno != ESRCH) {
-               /* Shouldn't happen. */
-               perror("detach: ptrace(PTRACE_DETACH, ...)");
+               /* Shouldn't happen. But if it did,
+                * secure_ptrace_DETACH() already complained */
        }
        else if (my_tgkill((tcp->flags & TCB_CLONE_THREAD ? tcp->parent->pid
                                                          : tcp->pid),
@@ -1547,21 +1548,14 @@ int sig;
                                break;
                        }
                        if (WSTOPSIG(status) == SIGSTOP) {
-                               if ((error = ptrace(PTRACE_DETACH,
-                                   tcp->pid, (char *) 1, sig)) < 0) {
-                                       if (errno != ESRCH)
-                                               perror("detach: 
ptrace(PTRACE_DETACH, ...)");
-                                       /* I died trying. */
-                               }
+                               secure_ptrace_DETACH(tcp, sig);
                                break;
                        }
-                       if ((error = ptrace(PTRACE_CONT, tcp->pid, (char *) 1,
-                           WSTOPSIG(status) == SIGTRAP ?
-                           0 : WSTOPSIG(status))) < 0) {
-                               if (errno != ESRCH)
-                                       perror("detach: ptrace(PTRACE_CONT, 
...)");
+                       error = secure_ptrace_CONT(tcp,
+                                       WSTOPSIG(status) == SIGTRAP ? 0
+                                       : WSTOPSIG(status));
+                       if (error < 0)
                                break;
-                       }
                }
 #endif /* LINUX */
 
@@ -1570,8 +1564,7 @@ int sig;
        if (sig && kill(tcp->pid, sig) < 0)
                perror("detach: kill");
        sig = 0;
-       if ((error = ptrace(PTRACE_DETACH, tcp->pid, (char *) 1, sig)) < 0)
-               perror("detach: ptrace(PTRACE_DETACH, ...)");
+       error = secure_ptrace_DETACH(tcp, sig);
 #endif /* SUNOS4 */
 
 #ifndef USE_PROCFS
@@ -2175,8 +2168,7 @@ handle_group_exit(struct tcb *tcp, int s
                        if (leader != NULL && leader != tcp)
                                leader->flags |= TCB_GROUP_EXITING;
                }
-               else if (ptrace(PTRACE_CONT, tcp->pid, (char *) 1, sig) < 0) {
-                       perror("strace: ptrace(PTRACE_CONT, ...)");
+               else if (secure_ptrace_CONT(tcp, sig) < 0) {
                        cleanup();
                        return -1;
                }
@@ -2429,9 +2421,7 @@ Process %d attached (waiting for parent)
                                 * Hope we are back in control now.
                                 */
                                tcp->flags &= ~(TCB_INSYSCALL | TCB_SIGTRAPPED);
-                               if (ptrace(PTRACE_SYSCALL,
-                                               pid, (char *) 1, 0) < 0) {
-                                       perror("trace: ptrace(PTRACE_SYSCALL, 
...)");
+                               if (secure_ptrace_SYSCALL(tcp, 0) < 0) {
                                        cleanup();
                                        return -1;
                                }
@@ -2478,9 +2468,7 @@ Process %d attached (waiting for parent)
 #endif
                                continue;
                        }
-                       if (ptrace(PTRACE_SYSCALL, pid, (char *) 1,
-                                  WSTOPSIG(status)) < 0) {
-                               perror("trace: ptrace(PTRACE_SYSCALL, ...)");
+                       if (secure_ptrace_SYSCALL(tcp, WSTOPSIG(status)) < 0) {
                                cleanup();
                                return -1;
                        }
@@ -2490,7 +2478,7 @@ Process %d attached (waiting for parent)
                /* we handled the STATUS, we are permitted to interrupt now. */
                if (interrupted)
                        return 0;
-               if (trace_syscall(tcp) < 0) {
+               if (trace_syscall(tcp) < 0 && !tcp->ptrace_errno) {
                        if (tcp->flags & TCB_ATTACHED)
                                detach(tcp, 0);
                        else {
@@ -2510,8 +2498,7 @@ Process %d attached (waiting for parent)
 #endif
                        if (tcp->flags & TCB_ATTACHED)
                                detach(tcp, 0);
-                       else if (ptrace(PTRACE_CONT, pid, (char *) 1, 0) < 0) {
-                               perror("strace: ptrace(PTRACE_CONT, ...)");
+                       else if (secure_ptrace_CONT(tcp, 0) < 0) {
                                cleanup();
                                return -1;
                        }
@@ -2523,8 +2510,9 @@ Process %d attached (waiting for parent)
                        continue;
                }
        tracing:
-               if (ptrace(PTRACE_SYSCALL, pid, (char *) 1, 0) < 0) {
-                       perror("trace: ptrace(PTRACE_SYSCALL, ...)");
+               /* ESRCH: task is dead or not stopped, we will handle it
+                * at waitpid. Otherwise we are confused and bail out. */
+               if (secure_ptrace_SYSCALL(tcp, 0) < 0 && errno != ESRCH) {
                        cleanup();
                        return -1;
                }
@@ -2575,6 +2563,11 @@ struct tcb *tcp;
        if (tcp_last && (!outfname || followfork < 2 || tcp_last == tcp)) {
                tcp_last->flags |= TCB_REPRINT;
                tprintf(" <unfinished ...>\n");
+       } else if (tcp_last->ptrace_errno) {
+               if (tcp_last->flags & TCB_INSYSCALL)
+                       tprintf(" <unavailable>) =");
+               tprintf(" ? <unavailable>\n");
+               tcp_last->ptrace_errno = 0;
        }
        curcol = 0;
        if ((followfork == 1 || pflag_seen > 1) && outfname)
diff -d -urpN strace.1/util.c strace.2/util.c
--- strace.1/util.c     2008-12-04 19:30:18.000000000 +0100
+++ strace.2/util.c     2008-12-10 10:01:02.000000000 +0100
@@ -240,6 +240,78 @@ xlookup(const struct xlat *xlat, int val
        return NULL;
 }
 
+/* Ptrace wrappers which track ESRCH errors.
+ * We assume that ESRCH indicates likely process death (SIGKILL?),
+ * modulo bugs where process somehow ended up not stopped.
+ * Unfortunately kernel uses ESRCH for that case too. Oh well.
+ */
+/* upeek uses this (we are in the midst of decode) */
+//TODO: use this in all other ptrace() calls in decode (most are in util.c)
+long
+secure_ptrace(int request, struct tcb *tcp, void *addr, void *data)
+{
+       long l;
+
+       if (tcp->ptrace_errno) {
+               errno = tcp->ptrace_errno;
+               return -1;
+       }
+
+       errno = 0;
+       l = ptrace(request, tcp->pid, addr, data);
+       /* Non-ESRCH errors might be our invalid reg/mem accesses,
+        * we do not trip on them */
+       if (errno == ESRCH) {
+               tcp->ptrace_errno = ESRCH;
+       }
+       return l;
+}
+
+/* Used when we want to unblock stopped traced process */
+static long
+secure_ptrace_op(int op, struct tcb *tcp, int sig)
+{
+       long l;
+
+       if (tcp->ptrace_errno) {
+               errno = tcp->ptrace_errno;
+               return -1;
+       }
+
+       errno = 0;
+       l = ptrace(op, tcp->pid, (void *) 1, (void *) (long) sig);
+       tcp->ptrace_errno = errno;
+       /* ESRCH: probably SIGKILLed, don't report. Otherwise complain.  */
+       if (tcp->ptrace_errno && tcp->ptrace_errno != ESRCH) {
+               const char *msg = "SYSCALL";
+               if (op == PTRACE_CONT)
+                       msg = "CONT";
+               if (op == PTRACE_DETACH)
+                       msg = "DETACH";
+               fprintf(stderr, "strace: ptrace(PTRACE_%s,1,%d): %s",
+                               msg, sig, strerror(tcp->ptrace_errno));
+       }
+       return l;
+}
+
+long
+secure_ptrace_SYSCALL(struct tcb *tcp, int sig)
+{
+       return secure_ptrace_op(PTRACE_SYSCALL, tcp, sig);
+}
+
+long
+secure_ptrace_CONT(struct tcb *tcp, int sig)
+{
+       return secure_ptrace_op(PTRACE_CONT, tcp, sig);
+}
+
+long
+secure_ptrace_DETACH(struct tcb *tcp, int sig)
+{
+       return secure_ptrace_op(PTRACE_DETACH, tcp, sig);
+}
+
 /*
  * Print entry in struct xlat table, if there.
  */
@@ -1078,11 +1150,13 @@ long *res;
        }
 #endif /* SUNOS4_KERNEL_ARCH_KLUDGE */
        errno = 0;
-       val = ptrace(PTRACE_PEEKUSER, tcp->pid, (char *) off, 0);
+       val = secure_ptrace(PTRACE_PEEKUSER, tcp, (char *) off, 0);
        if (val == -1 && errno) {
-               char buf[60];
-               sprintf(buf,"upeek: ptrace(PTRACE_PEEKUSER,%d,%lu,0)", 
tcp->pid, off);
-               perror(buf);
+               if (errno != ESRCH) {
+                       char buf[60];
+                       sprintf(buf,"upeek: ptrace(PTRACE_PEEKUSER,%d,%lu,0)", 
tcp->pid, off);
+                       perror(buf);
+               }
                return -1;
        }
        *res = val;



------------------------------------------------------------------------------
SF.Net email is Sponsored by MIX09, March 18-20, 2009 in Las Vegas, Nevada.
The future of the web can't happen without you.  Join us at MIX09 to help
pave the way to the Next Web now. Learn more and register at
http://ad.doubleclick.net/clk;208669438;13503038;i?http://2009.visitmix.com/
_______________________________________________
Strace-devel mailing list
Strace-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/strace-devel

Reply via email to