[PATCH] check process death if ptrace() fails

Denys Vlasenko dvlasenk at redhat.com
Mon Dec 8 12:45:59 UTC 2008


This patch adds check for process death after
failed ptrace() calls, most notably in upeek().
This makes strace correctly report SIGKILL
which happened after ptrace stop.

This patch depends on "pass tcp to upeek instead of just pid"
patch I send earlier.

This patch is intended to fix this bug:
https://bugzilla.redhat.com/show_bug.cgi?id=472053
(unfortunately the bug is marked private, sorry)

Run-tested. With this patch:

getpid()                                = 11720
kill(11720, SIGKILL <unfinished ...>
+++ killed by SIGKILL +++
Killed

Without this patch:

getpid()                                = 11730
kill(11730, SIGKILLupeek: ptrace(PTRACE_PEEKUSER,11730,2096,0): No such process
upeek: ptrace(PTRACE_PEEKUSER,11730,2240,0): No such process


Changelog:

2008-12-08  Denys Vlasenko  <dvlasenk at redhat.com>
	* defs.h (handle_ptrace_err): New function
	* strace.c: move process death handling in a static function
	* strace.c, util.c: Call it instead of perror


--
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-08 13:03:07.000000000 +0100
@@ -458,6 +458,7 @@ extern struct tcb *alloc_tcb P((int, int
 extern struct tcb *pid2tcb P((int));
 extern void droptcb P((struct tcb *));
 extern int expand_tcbtab P((void));
+extern void handle_ptrace_err P((struct tcb *, const char *));
 
 #define alloctcb(pid)	alloc_tcb((pid), 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-08 13:27:05.000000000 +0100
@@ -107,6 +107,10 @@ static void cleanup P((void));
 static void interrupt P((int sig));
 static sigset_t empty_set, blocked_set;
 
+#ifdef TCB_GROUP_EXITING
+static int handle_group_exit P((struct tcb *tcp, int sig));
+#endif
+
 #ifdef HAVE_SIG_ATOMIC_T
 static volatile sig_atomic_t interrupted;
 #else /* !HAVE_SIG_ATOMIC_T */
@@ -345,6 +349,84 @@ newoutf(struct tcb *tcp)
 }
 
 static void
+handle_death(struct tcb *tcp, int status)
+{
+	if (WIFSIGNALED(status)) {
+		if (tcp->pid == strace_child)
+			exit_code = 0x100 | WTERMSIG(status);
+		if (!cflag
+		    && (qual_flags[WTERMSIG(status)] & QUAL_SIGNAL)) {
+			printleader(tcp);
+			tprintf("+++ killed by %s %s+++",
+				signame(WTERMSIG(status)),
+#ifdef WCOREDUMP
+				WCOREDUMP(status) ? "(core dumped) " :
+#endif
+				"");
+			printtrailer(tcp);
+		}
+	} else { /* WIFEXITED */
+		if (tcp->pid == strace_child)
+			exit_code = WEXITSTATUS(status);
+		if (debug)
+			fprintf(stderr, "pid %u exited\n", tcp->pid);
+		if ((tcp->flags & TCB_ATTACHED)
+#ifdef TCB_GROUP_EXITING
+		    && !(tcp->parent && (tcp->parent->flags &
+					 TCB_GROUP_EXITING))
+		    && !(tcp->flags & TCB_GROUP_EXITING)
+#endif
+			)
+			fprintf(stderr,
+				"PANIC: attached pid %u exited\n",
+				tcp->pid);
+		if (tcp == tcp_last) {
+			if ((tcp->flags & (TCB_INSYSCALL|TCB_REPRINT))
+			    == TCB_INSYSCALL)
+				tprintf(" <unfinished ... exit status %d>\n",
+					WEXITSTATUS(status));
+			tcp_last = NULL;
+		}
+	}
+#ifdef TCB_GROUP_EXITING
+	handle_group_exit(tcp, -1);
+#else
+	droptcb(tcp);
+#endif
+}
+
+/* If you talked to a process (via ptrace() or otherwise)
+ * and got an error, it might just be gone (SIGKILL etc).
+ * Call this routine to check for that.
+ * If process has indeed died, it prints relevant "killed by"
+ * message and drops tcp (setting tcp->pid to 0 among other things).
+ * If not, "msg" is printed using perror (and nothing more is done).
+ */
+void
+handle_ptrace_err(struct tcb *tcp, const char *msg)
+{
+	int status;
+	pid_t pid;
+
+	/* If we already were here, do not print messages,
+	 * user already has been informed.
+	 * Ideally, we should not even be here second time... */
+	if (tcp->pid == 0)
+		return;
+
+	/* Did task die while we poked it? */
+	pid = waitpid(tcp->pid, &status, WNOHANG | __WALL);
+	if (pid > 0 && (WIFEXITED(status) || WIFSIGNALED(status))) {
+		/* The most usual case is SIGKILL */
+		/* Print "killed by" message, drop tcp */
+		handle_death(tcp, status);
+		return;
+	}
+	/* Task is not dead. Strange. Just print a message */
+	perror(msg);
+}
+
+static void
 startup_attach(void)
 {
 	int tcbi;
@@ -1373,7 +1455,7 @@ struct tcb *tcp;
 #endif
 
 	if (ptrace(PTRACE_SYSCALL, tcp->pid, (char *) 1, 0) < 0) {
-		perror("resume: ptrace(PTRACE_SYSCALL, ...)");
+		handle_ptrace_err(tcp, "resume: ptrace(PTRACE_SYSCALL, ...)");
 		return -1;
 	}
 
@@ -2176,7 +2258,7 @@ handle_group_exit(struct tcb *tcp, int s
 				leader->flags |= TCB_GROUP_EXITING;
 		}
 		else if (ptrace(PTRACE_CONT, tcp->pid, (char *) 1, sig) < 0) {
-			perror("strace: ptrace(PTRACE_CONT, ...)");
+			handle_ptrace_err(tcp, "strace: ptrace(PTRACE_CONT, ...)");
 			cleanup();
 			return -1;
 		}
@@ -2335,54 +2417,8 @@ Process %d attached (waiting for parent)
 			 */
 			continue;
 		}
-		if (WIFSIGNALED(status)) {
-			if (pid == strace_child)
-				exit_code = 0x100 | WTERMSIG(status);
-			if (!cflag
-			    && (qual_flags[WTERMSIG(status)] & QUAL_SIGNAL)) {
-				printleader(tcp);
-				tprintf("+++ killed by %s %s+++",
-					signame(WTERMSIG(status)),
-#ifdef WCOREDUMP
-					WCOREDUMP(status) ? "(core dumped) " :
-#endif
-					"");
-				printtrailer(tcp);
-			}
-#ifdef TCB_GROUP_EXITING
-			handle_group_exit(tcp, -1);
-#else
-			droptcb(tcp);
-#endif
-			continue;
-		}
-		if (WIFEXITED(status)) {
-			if (pid == strace_child)
-				exit_code = WEXITSTATUS(status);
-			if (debug)
-				fprintf(stderr, "pid %u exited\n", pid);
-			if ((tcp->flags & TCB_ATTACHED)
-#ifdef TCB_GROUP_EXITING
-			    && !(tcp->parent && (tcp->parent->flags &
-						 TCB_GROUP_EXITING))
-			    && !(tcp->flags & TCB_GROUP_EXITING)
-#endif
-				)
-				fprintf(stderr,
-					"PANIC: attached pid %u exited\n",
-					pid);
-			if (tcp == tcp_last) {
-				if ((tcp->flags & (TCB_INSYSCALL|TCB_REPRINT))
-				    == TCB_INSYSCALL)
-					tprintf(" <unfinished ... exit status %d>\n",
-						WEXITSTATUS(status));
-				tcp_last = NULL;
-			}
-#ifdef TCB_GROUP_EXITING
-			handle_group_exit(tcp, -1);
-#else
-			droptcb(tcp);
-#endif
+		if (WIFSIGNALED(status) || WIFEXITED(status)) {
+			handle_death(tcp, status);
 			continue;
 		}
 		if (!WIFSTOPPED(status)) {
@@ -2431,7 +2467,7 @@ Process %d attached (waiting for parent)
 				tcp->flags &= ~(TCB_INSYSCALL | TCB_SIGTRAPPED);
 				if (ptrace(PTRACE_SYSCALL,
 						pid, (char *) 1, 0) < 0) {
-					perror("trace: ptrace(PTRACE_SYSCALL, ...)");
+					handle_ptrace_err(tcp, "trace: ptrace(PTRACE_SYSCALL, ...)");
 					cleanup();
 					return -1;
 				}
@@ -2480,7 +2516,7 @@ Process %d attached (waiting for parent)
 			}
 			if (ptrace(PTRACE_SYSCALL, pid, (char *) 1,
 				   WSTOPSIG(status)) < 0) {
-				perror("trace: ptrace(PTRACE_SYSCALL, ...)");
+				handle_ptrace_err(tcp, "trace: ptrace(PTRACE_SYSCALL, ...)");
 				cleanup();
 				return -1;
 			}
@@ -2511,7 +2547,7 @@ Process %d attached (waiting for parent)
 			if (tcp->flags & TCB_ATTACHED)
 				detach(tcp, 0);
 			else if (ptrace(PTRACE_CONT, pid, (char *) 1, 0) < 0) {
-				perror("strace: ptrace(PTRACE_CONT, ...)");
+				handle_ptrace_err(tcp, "strace: ptrace(PTRACE_CONT, ...)");
 				cleanup();
 				return -1;
 			}
@@ -2524,7 +2560,7 @@ Process %d attached (waiting for parent)
 		}
 	tracing:
 		if (ptrace(PTRACE_SYSCALL, pid, (char *) 1, 0) < 0) {
-			perror("trace: ptrace(PTRACE_SYSCALL, ...)");
+			handle_ptrace_err(tcp, "trace: ptrace(PTRACE_SYSCALL, ...)");
 			cleanup();
 			return -1;
 		}
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-08 13:02:04.000000000 +0100
@@ -1082,7 +1082,7 @@ long *res;
 	if (val == -1 && errno) {
 		char buf[60];
 		sprintf(buf,"upeek: ptrace(PTRACE_PEEKUSER,%d,%lu,0)", tcp->pid, off);
-		perror(buf);
+		handle_ptrace_err(tcp, buf);
 		return -1;
 	}
 	*res = val;






More information about the Strace-devel mailing list