[patch] Break by Ctrl-C before first syscall SIGSTOPs the traced process
Jan Kratochvil
jan.kratochvil at redhat.com
Fri May 25 12:04:43 UTC 2007
On Thu, 24 May 2007 10:16:49 +0200, Roland McGrath wrote:
...
> The TCB_STARTUP check in trace() is what swallows the startup SIGSTOP
> normally. As you noted, there might be a different signal first, and that
> signal should not be swallowed, though the later SIGSTOP still should be
> (if the process survives the earlier signals).
The proposed patch has a race in the TCB_STARTUP path that seen TCB_BPTSET is
processed exceptionally by clearbpt () on all the signals before the first
SIGSTOP as we still cannot clear TCB_STARTUP till SIGSTOP is seen.
TCB_STARTUP && TCB_BPTSET can IMO happen only happen for the newly
forked/cloned child and its first signal should be always SIGSTOP.
I hope I did not miss any other case.
The removed FIXVFORK call may be wrong but I am not sure if it is still valid.
> Finally, cleanup (i.e. detach) has check for TCB_STARTUP and know it should
> not add another SIGSTOP, but must do wait4's and until it can swallow the
> expected SIGSTOP (which it already does).
It was interesting to me this exception is really needed there.
Without the exception the inferior process get stopped, with strace-of-strace:
ptrace(PTRACE_DETACH, 16656, 0x1, SIG_0) = -1 ESRCH (No such process)
kill(16656, SIG_0) = 0
kill(16656, SIGSTOP) = 0
wait4(16656, [{WIFSTOPPED(s) && WSTOPSIG(s) == SIGSTOP}], __WALL, NULL) = 16656
--- SIGCHLD (Child exited) @ 0 (0) ---
ptrace(PTRACE_DETACH, 16656, 0x1, SIG_0) = 0
In the kill(..., SIGSTOP) moment the SIGSTOP was still pending, it got stacked
there twice, though.
> Does this all make sense to you? If so, do you think it's correct?
> If so, can you try a patch doing what I've described?
All your comments should be processed, your mail has been converted to the
attached patch.
Attached `slowcall.c' for easier testing of attachment to a long lasting single
syscall. I miss some strace testsuite, I saw in ChangeLog an entry (?):
* test/clone.c: improve our testcase a bit
Best Regards,
Jan
-------------- next part --------------
2007-05-24 Roland McGrath <roland at redhat.com>
Jan Kratochvil <jan.kratochvil at redhat.com>
Never interrupt when the attached traced process would be left stopped.
* strace.c (main) `-p' attaching moved to ...
(startup_attach) ... a new function, renaming C to TCBI.
Block interrupting signals since the first tracee has been attached.
New comment about INTERRUPTED in the nonthreaded case.
[LINUX] (startup_attach) Check INTERRUPTED after each attached thread.
(main) Command spawning moved to ...
(startup_child) ... a new function, replacing RETURN with EXIT.
[LINUX] (detach): New variable CATCH_SIGSTOP, do not signal a new
SIGSTOP for processes still in TCB_STARTUP.
(main): Move signals and BLOCKED_SET init before the tracees attaching,
[SUNOS4] (trace): Removed fixvfork () call as a dead code, SIGSTOP must
have been already caught before clearing TCB_STARTUP.
(trace) Removed `!WIFSTOPPED(status)' dead code.
Clear TCB_STARTUP only in the case the received signal was SIGSTOP.
New comment when TCB_BPTSET && TCB_STARTUP combination can be set.
--- strace-4.5.15-orig/strace.c 2006-12-13 22:55:39.000000000 +0100
+++ strace-4.5.15-interrupt-during-attach/strace.c 2007-05-25 13:44:36.000000000 +0200
@@ -319,6 +319,280 @@ newoutf(struct tcb *tcp)
return 0;
}
+static void
+startup_attach(void)
+{
+ int tcbi;
+ struct tcb *tcp;
+
+ /*
+ * Block user interruptions as we would leave the traced
+ * process stopped (process state T) if we would terminate in
+ * between PTRACE_ATTACH and wait4 () on SIGSTOP.
+ * We rely on cleanup () from this point on.
+ */
+ if (interactive)
+ sigprocmask(SIG_BLOCK, &blocked_set, NULL);
+
+ for (tcbi = 0; tcbi < tcbtabsize; tcbi++) {
+ tcp = tcbtab[tcbi];
+ if (!(tcp->flags & TCB_INUSE) || !(tcp->flags & TCB_ATTACHED))
+ continue;
+#ifdef LINUX
+ if (tcp->flags & TCB_CLONE_THREAD)
+ continue;
+#endif
+ /* Reinitialize the output since it may have changed. */
+ tcp->outf = outf;
+ if (newoutf(tcp) < 0)
+ exit(1);
+
+#ifdef USE_PROCFS
+ if (proc_open(tcp, 1) < 0) {
+ fprintf(stderr, "trouble opening proc file\n");
+ droptcb(tcp);
+ continue;
+ }
+#else /* !USE_PROCFS */
+# ifdef LINUX
+ if (followfork) {
+ char procdir[MAXPATHLEN];
+ DIR *dir;
+
+ sprintf(procdir, "/proc/%d/task", tcp->pid);
+ dir = opendir(procdir);
+ if (dir != NULL) {
+ unsigned int ntid = 0, nerr = 0;
+ struct dirent *de;
+ int tid;
+ while ((de = readdir(dir)) != NULL) {
+ if (de->d_fileno == 0 ||
+ de->d_name[0] == '.')
+ continue;
+ tid = atoi(de->d_name);
+ if (tid <= 0)
+ continue;
+ ++ntid;
+ if (ptrace(PTRACE_ATTACH, tid,
+ (char *) 1, 0) < 0)
+ ++nerr;
+ else if (tid != tcbtab[tcbi]->pid) {
+ if (nprocs == tcbtabsize &&
+ expand_tcbtab())
+ tcp = NULL;
+ else
+ tcp = alloctcb(tid);
+ if (tcp == NULL)
+ exit(1);
+ tcp->flags |= TCB_ATTACHED|TCB_CLONE_THREAD|TCB_CLONE_DETACHED|TCB_FOLLOWFORK;
+ tcbtab[tcbi]->nchildren++;
+ tcbtab[tcbi]->nclone_threads++;
+ tcbtab[tcbi]->nclone_detached++;
+ tcp->parent = tcbtab[tcbi];
+ }
+ if (interactive) {
+ sigprocmask(SIG_SETMASK, &empty_set, NULL);
+ if (interrupted)
+ return;
+ sigprocmask(SIG_BLOCK, &blocked_set, NULL);
+ }
+ }
+ closedir(dir);
+ if (nerr == ntid) {
+ perror("attach: ptrace(PTRACE_ATTACH, ...)");
+ droptcb(tcp);
+ continue;
+ }
+ if (!qflag) {
+ ntid -= nerr;
+ if (ntid > 1)
+ fprintf(stderr, "\
+Process %u attached with %u threads - interrupt to quit\n",
+ tcp->pid, ntid);
+ else
+ fprintf(stderr, "\
+Process %u attached - interrupt to quit\n",
+ tcp->pid);
+ }
+ continue;
+ }
+ }
+# endif
+ if (ptrace(PTRACE_ATTACH, tcp->pid, (char *) 1, 0) < 0) {
+ perror("attach: ptrace(PTRACE_ATTACH, ...)");
+ droptcb(tcp);
+ continue;
+ }
+ /* INTERRUPTED is going to be checked at the top of TRACE. */
+#endif /* !USE_PROCFS */
+ if (!qflag)
+ fprintf(stderr,
+ "Process %u attached - interrupt to quit\n",
+ tcp->pid);
+ }
+
+ if (interactive)
+ sigprocmask(SIG_SETMASK, &empty_set, NULL);
+}
+
+static void
+startup_child (char **argv)
+{
+ struct stat statbuf;
+ const char *filename;
+ char pathname[MAXPATHLEN];
+ int pid = 0;
+ struct tcb *tcp;
+
+ filename = argv[0];
+ if (strchr(filename, '/')) {
+ if (strlen(filename) > sizeof pathname - 1) {
+ errno = ENAMETOOLONG;
+ perror("strace: exec");
+ exit(1);
+ }
+ strcpy(pathname, filename);
+ }
+#ifdef USE_DEBUGGING_EXEC
+ /*
+ * Debuggers customarily check the current directory
+ * first regardless of the path but doing that gives
+ * security geeks a panic attack.
+ */
+ else if (stat(filename, &statbuf) == 0)
+ strcpy(pathname, filename);
+#endif /* USE_DEBUGGING_EXEC */
+ else {
+ char *path;
+ int m, n, len;
+
+ for (path = getenv("PATH"); path && *path; path += m) {
+ if (strchr(path, ':')) {
+ n = strchr(path, ':') - path;
+ m = n + 1;
+ }
+ else
+ m = n = strlen(path);
+ if (n == 0) {
+ if (!getcwd(pathname, MAXPATHLEN))
+ continue;
+ len = strlen(pathname);
+ }
+ else if (n > sizeof pathname - 1)
+ continue;
+ else {
+ strncpy(pathname, path, n);
+ len = n;
+ }
+ if (len && pathname[len - 1] != '/')
+ pathname[len++] = '/';
+ strcpy(pathname + len, filename);
+ if (stat(pathname, &statbuf) == 0 &&
+ /* Accept only regular files
+ with some execute bits set.
+ XXX not perfect, might still fail */
+ S_ISREG(statbuf.st_mode) &&
+ (statbuf.st_mode & 0111))
+ break;
+ }
+ }
+ if (stat(pathname, &statbuf) < 0) {
+ fprintf(stderr, "%s: %s: command not found\n",
+ progname, filename);
+ exit(1);
+ }
+ switch (pid = fork()) {
+ case -1:
+ perror("strace: fork");
+ cleanup();
+ exit(1);
+ break;
+ case 0: {
+#ifdef USE_PROCFS
+ if (outf != stderr) close (fileno (outf));
+#ifdef MIPS
+ /* Kludge for SGI, see proc_open for details. */
+ sa.sa_handler = foobar;
+ sa.sa_flags = 0;
+ sigemptyset(&sa.sa_mask);
+ sigaction(SIGINT, &sa, NULL);
+#endif /* MIPS */
+#ifndef FREEBSD
+ pause();
+#else /* FREEBSD */
+ kill(getpid(), SIGSTOP); /* stop HERE */
+#endif /* FREEBSD */
+#else /* !USE_PROCFS */
+ if (outf!=stderr)
+ close(fileno (outf));
+
+ if (ptrace(PTRACE_TRACEME, 0, (char *) 1, 0) < 0) {
+ perror("strace: ptrace(PTRACE_TRACEME, ...)");
+ exit(1);
+ }
+ if (debug)
+ kill(getpid(), SIGSTOP);
+
+ if (username != NULL || geteuid() == 0) {
+ uid_t run_euid = run_uid;
+ gid_t run_egid = run_gid;
+
+ if (statbuf.st_mode & S_ISUID)
+ run_euid = statbuf.st_uid;
+ if (statbuf.st_mode & S_ISGID)
+ run_egid = statbuf.st_gid;
+
+ /*
+ * It is important to set groups before we
+ * lose privileges on setuid.
+ */
+ if (username != NULL) {
+ if (initgroups(username, run_gid) < 0) {
+ perror("initgroups");
+ exit(1);
+ }
+ if (setregid(run_gid, run_egid) < 0) {
+ perror("setregid");
+ exit(1);
+ }
+ if (setreuid(run_uid, run_euid) < 0) {
+ perror("setreuid");
+ exit(1);
+ }
+ }
+ }
+ else
+ setreuid(run_uid, run_uid);
+
+ /*
+ * Induce an immediate stop so that the parent
+ * will resume us with PTRACE_SYSCALL and display
+ * this execve call normally.
+ */
+ kill(getpid(), SIGSTOP);
+#endif /* !USE_PROCFS */
+
+ execv(pathname, argv);
+ perror("strace: exec");
+ _exit(1);
+ break;
+ }
+ default:
+ if ((tcp = alloctcb(pid)) == NULL) {
+ cleanup();
+ exit(1);
+ }
+#ifdef USE_PROCFS
+ if (proc_open(tcp, 0) < 0) {
+ fprintf(stderr, "trouble opening proc file\n");
+ cleanup();
+ exit(1);
+ }
+#endif /* USE_PROCFS */
+ break;
+ }
+}
+
int
main(argc, argv)
int argc;
@@ -517,250 +791,6 @@ char *argv[];
qflag = 1;
}
- for (c = 0; c < tcbtabsize; c++) {
- tcp = tcbtab[c];
- if (!(tcp->flags & TCB_INUSE) || !(tcp->flags & TCB_ATTACHED))
- continue;
-#ifdef LINUX
- if (tcp->flags & TCB_CLONE_THREAD)
- continue;
-#endif
- /* Reinitialize the output since it may have changed. */
- tcp->outf = outf;
- if (newoutf(tcp) < 0)
- exit(1);
-
-#ifdef USE_PROCFS
- if (proc_open(tcp, 1) < 0) {
- fprintf(stderr, "trouble opening proc file\n");
- droptcb(tcp);
- continue;
- }
-#else /* !USE_PROCFS */
-# ifdef LINUX
- if (followfork) {
- char procdir[MAXPATHLEN];
- DIR *dir;
-
- sprintf(procdir, "/proc/%d/task", tcp->pid);
- dir = opendir(procdir);
- if (dir != NULL) {
- unsigned int ntid = 0, nerr = 0;
- struct dirent *de;
- int tid;
- while ((de = readdir(dir)) != NULL) {
- if (de->d_fileno == 0 ||
- de->d_name[0] == '.')
- continue;
- tid = atoi(de->d_name);
- if (tid <= 0)
- continue;
- ++ntid;
- if (ptrace(PTRACE_ATTACH, tid,
- (char *) 1, 0) < 0)
- ++nerr;
- else if (tid != tcbtab[c]->pid) {
- if (nprocs == tcbtabsize &&
- expand_tcbtab())
- tcp = NULL;
- else
- tcp = alloctcb(tid);
- if (tcp == NULL)
- exit(1);
- tcp->flags |= TCB_ATTACHED|TCB_CLONE_THREAD|TCB_CLONE_DETACHED|TCB_FOLLOWFORK;
- tcbtab[c]->nchildren++;
- tcbtab[c]->nclone_threads++;
- tcbtab[c]->nclone_detached++;
- tcp->parent = tcbtab[c];
- }
- }
- closedir(dir);
- if (nerr == ntid) {
- perror("attach: ptrace(PTRACE_ATTACH, ...)");
- droptcb(tcp);
- continue;
- }
- if (!qflag) {
- ntid -= nerr;
- if (ntid > 1)
- fprintf(stderr, "\
-Process %u attached with %u threads - interrupt to quit\n",
- tcp->pid, ntid);
- else
- fprintf(stderr, "\
-Process %u attached - interrupt to quit\n",
- tcp->pid);
- }
- continue;
- }
- }
-# endif
- if (ptrace(PTRACE_ATTACH, tcp->pid, (char *) 1, 0) < 0) {
- perror("attach: ptrace(PTRACE_ATTACH, ...)");
- droptcb(tcp);
- continue;
- }
-#endif /* !USE_PROCFS */
- if (!qflag)
- fprintf(stderr,
- "Process %u attached - interrupt to quit\n",
- tcp->pid);
- }
-
- if (!pflag_seen) {
- struct stat statbuf;
- char *filename;
- char pathname[MAXPATHLEN];
-
- filename = argv[optind];
- if (strchr(filename, '/')) {
- if (strlen(filename) > sizeof pathname - 1) {
- errno = ENAMETOOLONG;
- perror("strace: exec");
- exit(1);
- }
- strcpy(pathname, filename);
- }
-#ifdef USE_DEBUGGING_EXEC
- /*
- * Debuggers customarily check the current directory
- * first regardless of the path but doing that gives
- * security geeks a panic attack.
- */
- else if (stat(filename, &statbuf) == 0)
- strcpy(pathname, filename);
-#endif /* USE_DEBUGGING_EXEC */
- else {
- char *path;
- int m, n, len;
-
- for (path = getenv("PATH"); path && *path; path += m) {
- if (strchr(path, ':')) {
- n = strchr(path, ':') - path;
- m = n + 1;
- }
- else
- m = n = strlen(path);
- if (n == 0) {
- if (!getcwd(pathname, MAXPATHLEN))
- continue;
- len = strlen(pathname);
- }
- else if (n > sizeof pathname - 1)
- continue;
- else {
- strncpy(pathname, path, n);
- len = n;
- }
- if (len && pathname[len - 1] != '/')
- pathname[len++] = '/';
- strcpy(pathname + len, filename);
- if (stat(pathname, &statbuf) == 0 &&
- /* Accept only regular files
- with some execute bits set.
- XXX not perfect, might still fail */
- S_ISREG(statbuf.st_mode) &&
- (statbuf.st_mode & 0111))
- break;
- }
- }
- if (stat(pathname, &statbuf) < 0) {
- fprintf(stderr, "%s: %s: command not found\n",
- progname, filename);
- exit(1);
- }
- switch (pid = fork()) {
- case -1:
- perror("strace: fork");
- cleanup();
- exit(1);
- break;
- case 0: {
-#ifdef USE_PROCFS
- if (outf != stderr) close (fileno (outf));
-#ifdef MIPS
- /* Kludge for SGI, see proc_open for details. */
- sa.sa_handler = foobar;
- sa.sa_flags = 0;
- sigemptyset(&sa.sa_mask);
- sigaction(SIGINT, &sa, NULL);
-#endif /* MIPS */
-#ifndef FREEBSD
- pause();
-#else /* FREEBSD */
- kill(getpid(), SIGSTOP); /* stop HERE */
-#endif /* FREEBSD */
-#else /* !USE_PROCFS */
- if (outf!=stderr)
- close(fileno (outf));
-
- if (ptrace(PTRACE_TRACEME, 0, (char *) 1, 0) < 0) {
- perror("strace: ptrace(PTRACE_TRACEME, ...)");
- return -1;
- }
- if (debug)
- kill(getpid(), SIGSTOP);
-
- if (username != NULL || geteuid() == 0) {
- uid_t run_euid = run_uid;
- gid_t run_egid = run_gid;
-
- if (statbuf.st_mode & S_ISUID)
- run_euid = statbuf.st_uid;
- if (statbuf.st_mode & S_ISGID)
- run_egid = statbuf.st_gid;
-
- /*
- * It is important to set groups before we
- * lose privileges on setuid.
- */
- if (username != NULL) {
- if (initgroups(username, run_gid) < 0) {
- perror("initgroups");
- exit(1);
- }
- if (setregid(run_gid, run_egid) < 0) {
- perror("setregid");
- exit(1);
- }
- if (setreuid(run_uid, run_euid) < 0) {
- perror("setreuid");
- exit(1);
- }
- }
- }
- else
- setreuid(run_uid, run_uid);
-
- /*
- * Induce an immediate stop so that the parent
- * will resume us with PTRACE_SYSCALL and display
- * this execve call normally.
- */
- kill(getpid(), SIGSTOP);
-#endif /* !USE_PROCFS */
-
- execv(pathname, &argv[optind]);
- perror("strace: exec");
- _exit(1);
- break;
- }
- default:
- if ((tcp = alloctcb(pid)) == NULL) {
- cleanup();
- exit(1);
- }
-#ifdef USE_PROCFS
- if (proc_open(tcp, 0) < 0) {
- fprintf(stderr, "trouble opening proc file\n");
- cleanup();
- exit(1);
- }
-#endif /* USE_PROCFS */
- break;
- }
- }
-
sigemptyset(&empty_set);
sigemptyset(&blocked_set);
sa.sa_handler = SIG_IGN;
@@ -798,6 +828,11 @@ Process %u attached - interrupt to quit\
sigaction(SIGCHLD, &sa, NULL);
#endif /* USE_PROCFS */
+ if (pflag_seen)
+ startup_attach();
+ else
+ startup_child(&argv[optind]);
+
if (trace() < 0)
exit(1);
cleanup();
@@ -1291,7 +1326,7 @@ int sig;
{
int error = 0;
#ifdef LINUX
- int status, resumed;
+ int status, resumed, catch_sigstop;
struct tcb *zombie = NULL;
/* If the group leader is lingering only because of this other
@@ -1315,6 +1350,12 @@ int sig;
#undef PTRACE_DETACH
#define PTRACE_DETACH PTRACE_SUNDETACH
#endif
+ /*
+ * On TCB_STARTUP we did PTRACE_ATTACH but still did not get 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_STARTUP);
if ((error = ptrace(PTRACE_DETACH, tcp->pid, (char *) 1, sig)) == 0) {
/* On a clear day, you can see forever. */
}
@@ -1326,11 +1367,13 @@ int sig;
if (errno != ESRCH)
perror("detach: checking sanity");
}
- else if (kill(tcp->pid, SIGSTOP) < 0) {
+ else if (!catch_sigstop && kill(tcp->pid, SIGSTOP) < 0) {
if (errno != ESRCH)
perror("detach: stopping child");
}
- else {
+ else
+ catch_sigstop = 1;
+ if (catch_sigstop)
for (;;) {
#ifdef __WALL
if (wait4(tcp->pid, &status, __WALL, NULL) < 0) {
@@ -1381,7 +1424,6 @@ int sig;
break;
}
}
- }
#endif /* LINUX */
#if defined(SUNOS4)
@@ -2102,6 +2144,8 @@ trace()
#endif /* LINUX */
while (nprocs != 0) {
+ if (interrupted)
+ return 0;
if (interactive)
sigprocmask(SIG_SETMASK, &empty_set, NULL);
#ifdef LINUX
@@ -2134,9 +2178,6 @@ trace()
if (interactive)
sigprocmask(SIG_BLOCK, &blocked_set, NULL);
- if (interrupted)
- return 0;
-
if (pid == -1) {
switch (wait_errno) {
case EINTR:
@@ -2282,35 +2323,24 @@ Process %d attached (waiting for parent)
fprintf(stderr, "pid %u stopped, [%s]\n",
pid, signame(WSTOPSIG(status)));
- if (tcp->flags & TCB_STARTUP) {
+ /*
+ * Interestingly, the process may stop
+ * with STOPSIG equal to some other signal
+ * than SIGSTOP if we happend to attach
+ * just before the process takes a signal.
+ */
+ if ((tcp->flags & TCB_STARTUP) && WSTOPSIG(status) == SIGSTOP) {
/*
* This flag is there to keep us in sync.
* Next time this process stops it should
* really be entering a system call.
*/
tcp->flags &= ~TCB_STARTUP;
- if (tcp->flags & TCB_ATTACHED) {
+ if (tcp->flags & TCB_BPTSET) {
/*
- * Interestingly, the process may stop
- * with STOPSIG equal to some other signal
- * than SIGSTOP if we happend to attach
- * just before the process takes a signal.
+ * One example is a breakpoint inherited from
+ * parent through fork ().
*/
- if (!WIFSTOPPED(status)) {
- fprintf(stderr,
- "pid %u not stopped\n", pid);
- detach(tcp, WSTOPSIG(status));
- continue;
- }
- }
- else {
-#ifdef SUNOS4
- /* A child of us stopped at exec */
- if (WSTOPSIG(status) == SIGTRAP && followvfork)
- fixvfork(tcp);
-#endif /* SUNOS4 */
- }
- if (tcp->flags & TCB_BPTSET) {
if (clearbpt(tcp) < 0) /* Pretty fatal */ {
droptcb(tcp);
cleanup();
@@ -2385,6 +2415,9 @@ Process %d attached (waiting for parent)
tcp->flags &= ~TCB_SUSPENDED;
continue;
}
+ /* we handled the STATUS, we are permitted to interrupt now. */
+ if (interrupted)
+ return 0;
if (trace_syscall(tcp) < 0) {
if (tcp->flags & TCB_ATTACHED)
detach(tcp, 0);
-------------- next part --------------
#include <unistd.h>
#include <assert.h>
#include <stdlib.h>
#include <fcntl.h>
#include <errno.h>
#define FILENAME "/tmp/slowcall-data"
unsigned char buf[150 * 1024 * 1024];
int main (void)
{
ssize_t got;
int fd, i;
i = unlink (FILENAME);
assert (i == 0 || errno == ENOENT);
fd = creat (FILENAME, 0644);
assert (fd != -1);
i = unlink (FILENAME);
assert (i == 0 || errno == ENOENT);
got = write (fd, buf, sizeof (buf));
assert (got == sizeof (buf));
i = fdatasync (fd);
assert (i == 0);
return EXIT_SUCCESS;
}
More information about the Strace-devel
mailing list