Fix PTRACE_TRACEME emulation to try to find the "real parent" task if possible.

Resolves #3929

Author: rocallahan

CMakeLists.txt

@@ -1312,6 +1312,7 @@ set(BASIC_TESTS
   ptrace_trace_clone
   ptrace_trace_exit
   ptrace_traceme
+  ptrace_traceme_thread
   ptrace_waiter_thread
   ptracer_death
   ptracer_death_multithread

src/record_syscall.cc

@@ -2561,60 +2561,63 @@ static bool verify_ptrace_options(RecordTask* t,
   return true;
 }
 
-static bool check_ptracer_compatible(RecordTask* tracer, RecordTask* tracee) {
+static void check_ptracer_compatible(RecordTask* tracer, RecordTask* tracee) {
   // Don't allow a 32-bit process to trace a 64-bit process. That doesn't
   // make much sense (manipulating registers gets crazy), and would be hard to
   // support.
-  if (tracee->emulated_ptracer || tracee->tgid() == tracer->tgid() ||
-      (tracer->arch() == x86 && tracee->arch() == x86_64)) {
-    return false;
-  }
-  return true;
+  ASSERT(tracer, !(tracer->arch() == x86 && tracee->arch() == x86_64));
 }
 
-static RecordTask* get_ptrace_partner(RecordTask* t, pid_t pid) {
+static RecordTask* prepare_ptrace_attach(RecordTask* tracer, pid_t attach_to_tid,
+                                         TaskSyscallState& syscall_state) {
   // To simplify things, require that a ptracer be in the same pid
   // namespace as rr itself. I.e., tracee tasks sandboxed in a pid
   // namespace can't use ptrace. This is normally a requirement of
   // sandboxes anyway.
   // This could be supported, but would require some work to translate
   // rr's pids to/from the ptracer's pid namespace.
-  ASSERT(t, is_same_namespace("pid", t->tid, getpid()));
-  RecordTask* partner = t->session().find_task(pid);
-  if (!partner) {
-    // XXX This prevents a tracee from attaching to a process which isn't
+  ASSERT(tracer, is_same_namespace("pid", tracer->tid, getpid()));
+  RecordTask* tracee = tracer->session().find_task(attach_to_tid);
+  if (!tracee) {
+    // XXX This prevents a tracer from attaching to a process which isn't
     // under rr's control. We could support this but it would complicate
     // things.
-    return nullptr;
-  }
-  return partner;
-}
-
-static RecordTask* prepare_ptrace_attach(RecordTask* t, pid_t pid,
-                                         TaskSyscallState& syscall_state) {
-  RecordTask* tracee = get_ptrace_partner(t, pid);
-  if (!tracee) {
     syscall_state.emulate_result(-ESRCH);
     return nullptr;
   }
-  if (!check_ptracer_compatible(t, tracee)) {
+  if (tracee->emulated_ptracer || tracee->tgid() == tracer->tgid()) {
     syscall_state.emulate_result(-EPERM);
     return nullptr;
   }
+  check_ptracer_compatible(tracer, tracee);
   return tracee;
 }
 
-static RecordTask* prepare_ptrace_traceme(RecordTask* t,
+static RecordTask* prepare_ptrace_traceme(RecordTask* tracee,
                                           TaskSyscallState& syscall_state) {
-  RecordTask* tracer = get_ptrace_partner(t, t->get_parent_pid());
+  RecordTask* tracer = nullptr;
+  pid_t parent_pid = tracee->get_parent_pid();
+  if (tracee->creator_tid != 0) {
+    RecordTask* creator = tracee->session().find_task(tracee->creator_tid);
+    if (creator && creator->thread_group()->tgid == parent_pid) {
+      tracer = creator;
+    }
+  }
   if (!tracer) {
-    syscall_state.emulate_result(-ESRCH);
-    return nullptr;
+    ThreadGroup* tg = tracee->session().find_thread_group(parent_pid);
+    if (tg) {
+      tracer = static_cast<RecordTask*>(tg->first_running_task());
+    }
+    if (!tracer) {
+      syscall_state.emulate_result(0);
+      return nullptr;
+    }
   }
-  if (!check_ptracer_compatible(tracer, t)) {
+  if (tracee->emulated_ptracer || tracee->tgid() == tracer->tgid()) {
     syscall_state.emulate_result(-EPERM);
     return nullptr;
   }
+  check_ptracer_compatible(tracer, tracee);
   return tracer;
 }
 

src/test/ptrace_traceme_thread.c

@@ -0,0 +1,46 @@
+/* -*- Mode: C; tab-width: 8; c-basic-offset: 2; indent-tabs-mode: nil; -*- */
+
+#include "util.h"
+
+static int parent_to_child_fds[2];
+static int child_to_parent_fds[2];
+
+static void* do_thread(__attribute__((unused)) void* p) {
+  pid_t child = fork();
+  char ch;
+  if (!child) {
+    test_assert(0 == ptrace(PTRACE_TRACEME, 0, 0, 0));
+    test_assert(1 == write(child_to_parent_fds[1], "x", 1));
+    raise(SIGSTOP);
+    test_assert(1 == read(parent_to_child_fds[0], &ch, 1));
+    return NULL;
+  }
+
+  test_assert(1 == read(child_to_parent_fds[0], &ch, 1));
+  /* Wait until the tracee stops */
+  int status;
+  test_assert(child == waitpid(child, &status, 0));
+  test_assert(WIFSTOPPED(status) && WSTOPSIG(status) == SIGSTOP);
+
+  /* Ask for a ptrace notification on exit */
+  test_assert(0 == ptrace(PTRACE_SETOPTIONS, child, 0, PTRACE_O_TRACEEXIT));
+  test_assert(0 == ptrace(PTRACE_CONT, child, 0, 0));
+
+  test_assert(1 == write(parent_to_child_fds[1], "p", 1));
+
+  /* Child is now exiting. Check for the PTRACE_EVENT_EXIT notification */
+  test_assert(child == waitpid(child, &status, 0));
+  test_assert(status >> 8 == (SIGTRAP | (PTRACE_EVENT_EXIT << 8)));
+  return NULL;
+}
+
+int main(void) {
+  test_assert(0 == pipe(parent_to_child_fds));
+  test_assert(0 == pipe(child_to_parent_fds));
+
+  pthread_t thread;
+  pthread_create(&thread, NULL, do_thread, NULL);
+  pthread_join(thread, NULL);
+  atomic_puts("EXIT-SUCCESS");
+  return 0;
+}