[PATCH] check process death if ptrace() fails, v2
Denys Vlasenko
dvlasenk at redhat.com
Wed Dec 10 09:24:47 UTC 2008
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;
More information about the Strace-devel
mailing list