DO NOT MERGE: debuggerd: verify that traced threads belong to the right process. am: 36dd144367 (6cb199bf) · Commits · e / os / android_system_core

debuggerd/backtrace.cpp

+5 −5

Original line number	Diff line number	Diff line
		@@ -62,8 +62,8 @@ static void dump_process_footer(log_t* log, pid_t pid) {
		_LOG(log, logtype::BACKTRACE, "\n----- end %d -----\n", pid);
		}

		static void dump_thread(
		log_t* log, pid_t tid, bool attached, bool* detach_failed, int* total_sleep_time_usec) {
		static void dump_thread(log_t* log, pid_t pid, pid_t tid, bool attached,
		bool* detach_failed, int* total_sleep_time_usec) {
		char path[PATH_MAX];
		char threadnamebuf[1024];
		char* threadname = NULL;
		@@ -83,7 +83,7 @@ static void dump_thread(

		_LOG(log, logtype::BACKTRACE, "\n\"%s\" sysTid=%d\n", threadname ? threadname : "<unknown>", tid);

		if (!attached && ptrace(PTRACE_ATTACH, tid, 0, 0) < 0) {
		if (!attached && !ptrace_attach_thread(pid, tid)) {
		_LOG(log, logtype::BACKTRACE, "Could not attach to thread: %s\n", strerror(errno));
		return;
		}
		@@ -110,7 +110,7 @@ void dump_backtrace(int fd, int amfd, pid_t pid, pid_t tid, bool* detach_failed,
		log.amfd = amfd;

		dump_process_header(&log, pid);
		dump_thread(&log, tid, true, detach_failed, total_sleep_time_usec);
		dump_thread(&log, pid, tid, true, detach_failed, total_sleep_time_usec);

		char task_path[64];
		snprintf(task_path, sizeof(task_path), "/proc/%d/task", pid);
		@@ -128,7 +128,7 @@ void dump_backtrace(int fd, int amfd, pid_t pid, pid_t tid, bool* detach_failed,
		continue;
		}

		dump_thread(&log, new_tid, false, detach_failed, total_sleep_time_usec);
		dump_thread(&log, pid, new_tid, false, detach_failed, total_sleep_time_usec);
		}
		closedir(d);
		}

debuggerd/debuggerd.cpp

+28 −7

Original line number	Diff line number	Diff line
		@@ -225,12 +225,10 @@ static int read_request(int fd, debugger_request_t* out_request) {

		if (msg.action == DEBUGGER_ACTION_CRASH) {
		// Ensure that the tid reported by the crashing process is valid.
		char buf[64];
		struct stat s;
		snprintf(buf, sizeof buf, "/proc/%d/task/%d", out_request->pid, out_request->tid);
		if (stat(buf, &s)) {
		ALOGE("tid %d does not exist in pid %d. ignoring debug request\n",
		out_request->tid, out_request->pid);
		// This check needs to happen again after ptracing the requested thread to prevent a race.
		if (!pid_contains_tid(out_request->pid, out_request->tid)) {
		ALOGE("tid %d does not exist in pid %d. ignoring debug request\n", out_request->tid,
		out_request->pid);
		return -1;
		}
		} else if (cr.uid == 0
		@@ -380,9 +378,32 @@ static void handle_request(int fd) {
		// ensure that it can run as soon as we call PTRACE_CONT below.
		// See details in bionic/libc/linker/debugger.c, in function
		// debugger_signal_handler().
		if (ptrace(PTRACE_ATTACH, request.tid, 0, 0)) {
		if (!ptrace_attach_thread(request.pid, request.tid)) {
		ALOGE("ptrace attach failed: %s\n", strerror(errno));
		} else {
		// DEBUGGER_ACTION_CRASH requests can come from arbitrary processes and the tid field in
		// the request is sent from the other side. If an attacker can cause a process to be
		// spawned with the pid of their process, they could trick debuggerd into dumping that
		// process by exiting after sending the request. Validate the trusted request.uid/gid
		// to defend against this.
		if (request.action == DEBUGGER_ACTION_CRASH) {
		pid_t pid;
		uid_t uid;
		gid_t gid;
		if (get_process_info(request.tid, &pid, &uid, &gid) != 0) {
		ALOGE("debuggerd: failed to get process info for tid '%d'", request.tid);
		exit(1);
		}

		if (pid != request.pid \|\| uid != request.uid \|\| gid != request.gid) {
		ALOGE(
		"debuggerd: attached task %d does not match request: "
		"expected pid=%d,uid=%d,gid=%d, actual pid=%d,uid=%d,gid=%d",
		request.tid, request.pid, request.uid, request.gid, pid, uid, gid);
		exit(1);
		}
		}

		bool detach_failed = false;
		bool tid_unresponsive = false;
		bool attach_gdb = should_attach_gdb(&request);

debuggerd/tombstone.cpp

+1 −1

Original line number	Diff line number	Diff line
		@@ -427,7 +427,7 @@ static bool dump_sibling_thread_report(
		}

		// Skip this thread if cannot ptrace it
		if (ptrace(PTRACE_ATTACH, new_tid, 0, 0) < 0) {
		if (!ptrace_attach_thread(pid, new_tid)) {
		_LOG(log, logtype::ERROR, "ptrace attach to %d failed: %s\n", new_tid, strerror(errno));
		continue;
		}

debuggerd/utility.cpp

+29 −0

Original line number	Diff line number	Diff line
		@@ -20,6 +20,7 @@

		#include <errno.h>
		#include <signal.h>
		#include <stdlib.h>
		#include <string.h>
		#include <unistd.h>
		#include <sys/ptrace.h>
		@@ -196,3 +197,31 @@ void dump_memory(log_t* log, pid_t tid, uintptr_t addr) {
		_LOG(log, logtype::MEMORY, " %s %s\n", code_buffer, ascii_buffer);
		}
		}

		bool pid_contains_tid(pid_t pid, pid_t tid) {
		char task_path[PATH_MAX];
		if (snprintf(task_path, PATH_MAX, "/proc/%d/task/%d", pid, tid) >= PATH_MAX) {
		ALOGE("debuggerd: task path overflow (pid = %d, tid = %d)\n", pid, tid);
		exit(1);
		}

		return access(task_path, F_OK) == 0;
		}

		// Attach to a thread, and verify that it's still a member of the given process
		bool ptrace_attach_thread(pid_t pid, pid_t tid) {
		if (ptrace(PTRACE_ATTACH, tid, 0, 0) != 0) {
		return false;
		}

		// Make sure that the task we attached to is actually part of the pid we're dumping.
		if (!pid_contains_tid(pid, tid)) {
		if (ptrace(PTRACE_DETACH, tid, 0, 0) != 0) {
		ALOGE("debuggerd: failed to detach from thread '%d'", tid);
		exit(1);
		}
		return false;
		}

		return true;
		}

debuggerd/utility.h

+5 −0

Original line number	Diff line number	Diff line
		@@ -77,4 +77,9 @@ int wait_for_sigstop(pid_t, int, bool);

		void dump_memory(log_t* log, pid_t tid, uintptr_t addr);

		bool pid_contains_tid(pid_t pid, pid_t tid);

		// Attach to a thread, and verify that it's still a member of the given process
		bool ptrace_attach_thread(pid_t pid, pid_t tid);

		#endif // _DEBUGGERD_UTILITY_H