From 75fe85c2ee83a31afe0c8f1468da28deb1c2bc28 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Fri, 9 Mar 2012 15:15:24 +0100 Subject: [PATCH] Fix the case where we try to detach unattached processes Before this change: $ strace -D -p1 strace: -D and -p are mutually exclusive options Process 1 detached <==== WRONG! (and we try to SIGSTOP it!!!) * defs.h: Change the meaning of TCB_ATTACHED: now it means "this tracee is attached to us". Add TCB_STRACE_CHILD: "this tracee is our child". * strace.c (kill_save_errno): Move up. No code changes. (process_opt_p_list): Don't set TCB_ATTACHED on new tcb. (startup_attach): Change how we work with TCB_ATTACHED. Set TCB_STRACE_CHILD on -D. (startup_child): Use kill_save_errno instead of kill. Set TCB_ATTACHED and TCB_STRACE_CHILD on attached strace child. If we are in -D case, don't set TCB_ATTACHED (we aren't attached yet). (detach): do not do PTRACE_DETACH if TCB_ATTACHED is not set. (cleanup): Check TCB_STRACE_CHILD instead of TCB_ATTACHED. (trace): Likewise. Signed-off-by: Denys Vlasenko --- defs.h | 11 ++---- strace.c | 115 +++++++++++++++++++++++++++++-------------------------- 2 files changed, 63 insertions(+), 63 deletions(-) diff --git a/defs.h b/defs.h index a14950b3..c96b0310 100644 --- a/defs.h +++ b/defs.h @@ -352,14 +352,9 @@ struct tcb { * Use entering(tcp) / exiting(tcp) to check this bit to make code more readable. */ #define TCB_INSYSCALL 00010 -/* - * Process is not our own child. - * In "strace [-f] PROG" case, traced child (PROG) will have this bit not set, - * but any its children (including threads) will. - * Note: if bit is set, it does *not* mean that attaching was already done. - * There is a small window at the init time when it's not true. - */ -#define TCB_ATTACHED 00020 +#define TCB_ATTACHED 00020 /* It is attached already */ +/* Are we PROG from "strace PROG [ARGS]" invocation? */ +#define TCB_STRACE_CHILD 00040 #define TCB_BPTSET 00100 /* "Breakpoint" set after fork(2) */ #define TCB_REPRINT 01000 /* We should reprint this syscall on exit */ #define TCB_FILTERED 02000 /* This system call has been filtered out */ diff --git a/strace.c b/strace.c index 727e490b..26cf91fa 100644 --- a/strace.c +++ b/strace.c @@ -329,6 +329,14 @@ set_cloexec_flag(int fd) fcntl(fd, F_SETFD, newflags); /* never fails */ } +static void kill_save_errno(pid_t pid, int sig) +{ + int saved_errno = errno; + + (void) kill(pid, sig); + errno = saved_errno; +} + /* * When strace is setuid executable, we have to swap uids * before and after filesystem and process management operations. @@ -429,7 +437,6 @@ static void process_opt_p_list(char *opt) * pidof uses space as delim, pgrep uses newline. :( */ int pid; - struct tcb *tcp; char *delim = opt + strcspn(opt, ", \n\t"); char c = *delim; @@ -446,8 +453,7 @@ static void process_opt_p_list(char *opt) return; } *delim = c; - tcp = alloc_tcb(pid, 0); - tcp->flags |= TCB_ATTACHED; + alloc_tcb(pid, 0); if (c == '\0') break; opt = delim + 1; @@ -492,9 +498,12 @@ startup_attach(void) for (tcbi = 0; tcbi < tcbtabsize; tcbi++) { tcp = tcbtab[tcbi]; + if (!(tcp->flags & TCB_INUSE)) + continue; + /* Is this a process we should attach to, but not yet attached? */ - if ((tcp->flags & (TCB_ATTACHED | TCB_STARTUP)) != TCB_ATTACHED) - continue; /* no */ + if (tcp->flags & TCB_ATTACHED) + continue; /* no, we already attached it */ /* Reinitialize the output since it may have changed */ tcp->outf = outf; @@ -552,7 +561,7 @@ startup_attach(void) : "Process %u attached - interrupt to quit\n", tcp->pid, ntid); } - if (!(tcp->flags & TCB_STARTUP)) { + if (!(tcp->flags & TCB_ATTACHED)) { /* -p PID, we failed to attach to PID itself * but did attach to some of its sibling threads. * Drop PID's tcp. @@ -567,7 +576,7 @@ startup_attach(void) droptcb(tcp); continue; } - tcp->flags |= TCB_STARTUP | post_attach_sigstop; + tcp->flags |= TCB_ATTACHED | TCB_STARTUP | post_attach_sigstop; if (debug) fprintf(stderr, "attach to pid %d (main) succeeded\n", tcp->pid); @@ -576,7 +585,7 @@ startup_attach(void) * It is our grandparent we trace, not a -p PID. * Don't want to just detach on exit, so... */ - tcp->flags &= ~TCB_ATTACHED; + tcp->flags |= TCB_STRACE_CHILD; /* * Make parent go away. * Also makes grandparent's wait() unblock. @@ -747,7 +756,7 @@ startup_child(char **argv) perror_msg_and_die("waitpid"); } if (!WIFSTOPPED(status) || WSTOPSIG(status) != SIGSTOP) { - kill(pid, SIGKILL); + kill_save_errno(pid, SIGKILL); perror_msg_and_die("Unexpected wait status %x", status); } } @@ -757,7 +766,7 @@ startup_child(char **argv) */ if (ptrace_attach_or_seize(pid)) { - kill(pid, SIGKILL); + kill_save_errno(pid, SIGKILL); perror_msg_and_die("Can't attach to %d", pid); } if (!strace_vforked) @@ -765,9 +774,9 @@ startup_child(char **argv) } tcp = alloctcb(pid); if (!strace_vforked) - tcp->flags |= TCB_STARTUP | post_attach_sigstop; + tcp->flags |= TCB_ATTACHED | TCB_STRACE_CHILD | TCB_STARTUP | post_attach_sigstop; else - tcp->flags |= TCB_STARTUP; + tcp->flags |= TCB_ATTACHED | TCB_STRACE_CHILD | TCB_STARTUP; } else { /* With -D, *we* are child here, IOW: different pid. Fetch it: */ @@ -775,19 +784,9 @@ startup_child(char **argv) /* The tracee is our parent: */ pid = getppid(); tcp = alloctcb(pid); - /* We want subsequent startup_attach() to attach to it: */ - tcp->flags |= TCB_ATTACHED; } } -static void kill_save_errno(pid_t pid, int sig) -{ - int saved_errno = errno; - - (void) kill(pid, sig); - errno = saved_errno; -} - /* * Test whether the kernel support PTRACE_O_TRACECLONE et al options. * First fork a new child, call ptrace with PTRACE_SETOPTIONS on it, @@ -1440,15 +1439,15 @@ droptcb(struct tcb *tcp) } /* detach traced process; continue with sig - 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. */ - + * 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. + */ static int detach(struct tcb *tcp) { - int error = 0; - int status, catch_sigstop; + int error; + int status, sigstop_expected; if (tcp->flags & TCB_BPTSET) clearbpt(tcp); @@ -1462,31 +1461,36 @@ detach(struct tcb *tcp) #undef PTRACE_DETACH #define PTRACE_DETACH PTRACE_SUNDETACH #endif - /* - * We attached but possibly didn't see the expected SIGSTOP. - * We must catch exactly one as otherwise the detached process - * would be left stopped (process state T). - */ - catch_sigstop = (tcp->flags & TCB_IGNORE_ONE_SIGSTOP); - error = ptrace(PTRACE_DETACH, tcp->pid, (char *) 1, 0); - if (error == 0) { - /* On a clear day, you can see forever. */ - } - else if (errno != ESRCH) { - /* Shouldn't happen. */ - perror("detach: ptrace(PTRACE_DETACH, ...)"); - } - else if (my_tkill(tcp->pid, 0) < 0) { - if (errno != ESRCH) - perror("detach: checking sanity"); - } - else if (!catch_sigstop && my_tkill(tcp->pid, SIGSTOP) < 0) { - if (errno != ESRCH) - perror("detach: stopping child"); + + sigstop_expected = 0; + if (tcp->flags & TCB_ATTACHED) { + /* + * We attached but possibly didn't see the expected SIGSTOP. + * We must catch exactly one as otherwise the detached process + * would be left stopped (process state T). + */ + sigstop_expected = (tcp->flags & TCB_IGNORE_ONE_SIGSTOP); + error = ptrace(PTRACE_DETACH, tcp->pid, (char *) 1, 0); + if (error == 0) { + /* On a clear day, you can see forever. */ + } + else if (errno != ESRCH) { + /* Shouldn't happen. */ + perror("detach: ptrace(PTRACE_DETACH, ...)"); + } + else if (my_tkill(tcp->pid, 0) < 0) { + if (errno != ESRCH) + perror("detach: checking sanity"); + } + else if (!sigstop_expected && my_tkill(tcp->pid, SIGSTOP) < 0) { + if (errno != ESRCH) + perror("detach: stopping child"); + } + else + sigstop_expected = 1; } - else - catch_sigstop = 1; - if (catch_sigstop) { + + if (sigstop_expected) { for (;;) { #ifdef __WALL if (wait4(tcp->pid, &status, __WALL, NULL) < 0) { @@ -1532,7 +1536,7 @@ detach(struct tcb *tcp) } } - if (!qflag) + if (!qflag && (tcp->flags & TCB_ATTACHED)) fprintf(stderr, "Process %u detached\n", tcp->pid); droptcb(tcp); @@ -1564,7 +1568,7 @@ cleanup(void) tprints(" \n"); printing_tcp = NULL; } - if (tcp->flags & TCB_ATTACHED) + if (!(tcp->flags & TCB_STRACE_CHILD)) detach(tcp); else { kill(tcp->pid, SIGCONT); @@ -2008,7 +2012,8 @@ trace(void) * Observed case: exit_group() terminating * all processes in thread group. */ - if (tcp->flags & TCB_ATTACHED) { + if (!(tcp->flags & TCB_STRACE_CHILD)) { +//TODO: why do we handle our child differently?? if (printing_tcp) { /* Do we have dangling line "syscall(param, param"? * Finish the line then. -- 2.40.0