Merge "Replaced include log/logger.h with log/log.h in debuggerd"
diff --git a/debuggerd/crash_dump.cpp b/debuggerd/crash_dump.cpp
index fe2db7e..e4f88b0 100644
--- a/debuggerd/crash_dump.cpp
+++ b/debuggerd/crash_dump.cpp
@@ -57,8 +57,8 @@
 }
 
 // Attach to a thread, and verify that it's still a member of the given process
-static bool ptrace_attach_thread(pid_t pid, pid_t tid, std::string* error) {
-  if (ptrace(PTRACE_ATTACH, tid, 0, 0) != 0) {
+static bool ptrace_seize_thread(pid_t pid, pid_t tid, std::string* error) {
+  if (ptrace(PTRACE_SEIZE, tid, 0, 0) != 0) {
     *error = StringPrintf("failed to attach to thread %d: %s", tid, strerror(errno));
     return false;
   }
@@ -71,6 +71,12 @@
     *error = StringPrintf("thread %d is not in process %d", tid, pid);
     return false;
   }
+
+  // Put the task into ptrace-stop state.
+  if (ptrace(PTRACE_INTERRUPT, tid, 0, 0) != 0) {
+    PLOG(FATAL) << "failed to interrupt thread " << tid;
+  }
+
   return true;
 }
 
@@ -160,11 +166,15 @@
   return true;
 }
 
+static void signal_handler(int) {
+  // We can't log easily, because the heap might be corrupt.
+  // Just die and let the surrounding log context explain things.
+  _exit(1);
+}
+
 static void abort_handler(pid_t target, const bool& tombstoned_connected,
                           unique_fd& tombstoned_socket, unique_fd& output_fd,
                           const char* abort_msg) {
-  LOG(ERROR) << abort_msg;
-
   // If we abort before we get an output fd, contact tombstoned to let any
   // potential listeners know that we failed.
   if (!tombstoned_connected) {
@@ -177,7 +187,6 @@
 
   dprintf(output_fd.get(), "crash_dump failed to dump process %d: %s\n", target, abort_msg);
 
-  // Don't dump ourselves.
   _exit(1);
 }
 
@@ -203,6 +212,11 @@
     abort_handler(target, tombstoned_connected, tombstoned_socket, output_fd, abort_msg);
   });
 
+  // Don't try to dump ourselves.
+  struct sigaction action = {};
+  action.sa_handler = signal_handler;
+  debuggerd_register_handlers(&action);
+
   if (argc != 2) {
     return 1;
   }
@@ -243,10 +257,13 @@
     exit(0);
   }
 
+  // Die if we take too long.
+  alarm(20);
+
   check_process(target_proc_fd, target);
 
   std::string attach_error;
-  if (!ptrace_attach_thread(target, main_tid, &attach_error)) {
+  if (!ptrace_seize_thread(target, main_tid, &attach_error)) {
     LOG(FATAL) << attach_error;
   }
 
@@ -271,9 +288,9 @@
 
   LOG(INFO) << "performing dump of process " << target << " (target tid = " << main_tid << ")";
 
-  // At this point, the thread that made the request has been PTRACE_ATTACHed
-  // and has the signal that triggered things queued. Send PTRACE_CONT, and
-  // then wait for the signal.
+  // At this point, the thread that made the request has been attached and is
+  // in ptrace-stopped state. After resumption, the triggering signal that has
+  // been queued will be delivered.
   if (ptrace(PTRACE_CONT, main_tid, 0, 0) != 0) {
     PLOG(ERROR) << "PTRACE_CONT(" << main_tid << ") failed";
     exit(1);
@@ -302,17 +319,16 @@
   // Now that we have the signal that kicked things off, attach all of the
   // sibling threads, and then proceed.
   bool fatal_signal = signo != DEBUGGER_SIGNAL;
-  int resume_signal = fatal_signal ? signo : 0;
   std::set<pid_t> siblings;
   std::set<pid_t> attached_siblings;
-  if (resume_signal == 0) {
+  if (fatal_signal) {
     if (!android::procinfo::GetProcessTids(target, &siblings)) {
       PLOG(FATAL) << "failed to get process siblings";
     }
     siblings.erase(main_tid);
 
     for (pid_t sibling_tid : siblings) {
-      if (!ptrace_attach_thread(target, sibling_tid, &attach_error)) {
+      if (!ptrace_seize_thread(target, sibling_tid, &attach_error)) {
         LOG(WARNING) << attach_error;
       } else {
         attached_siblings.insert(sibling_tid);
@@ -338,40 +354,39 @@
                       attached_siblings, abort_address, fatal_signal ? &amfd_data : nullptr);
   }
 
+  // We don't actually need to PTRACE_DETACH, as long as our tracees aren't in
+  // group-stop state, which is true as long as no stopping signals are sent.
+
   bool wait_for_gdb = android::base::GetBoolProperty("debug.debuggerd.wait_for_gdb", false);
-  if (wait_for_gdb) {
+  if (!fatal_signal || siginfo.si_code == SI_USER) {
     // Don't wait_for_gdb when the process didn't actually crash.
-    if (!fatal_signal) {
-      wait_for_gdb = false;
-    } else {
-      // Use ALOGI to line up with output from engrave_tombstone.
-      ALOGI(
-        "***********************************************************\n"
-        "* Process %d has been suspended while crashing.\n"
-        "* To attach gdbserver and start gdb, run this on the host:\n"
-        "*\n"
-        "*     gdbclient.py -p %d\n"
-        "*\n"
-        "***********************************************************",
-        target, main_tid);
-    }
+    wait_for_gdb = false;
   }
 
-  for (pid_t tid : attached_siblings) {
-    // Don't send the signal to sibling threads.
-    if (ptrace(PTRACE_DETACH, tid, 0, wait_for_gdb ? SIGSTOP : 0) != 0) {
-      PLOG(ERROR) << "ptrace detach from " << tid << " failed";
+  // If the process crashed or we need to send it SIGSTOP for wait_for_gdb,
+  // get it in a state where it can receive signals, and then send the relevant
+  // signal.
+  if (wait_for_gdb || fatal_signal) {
+    if (ptrace(PTRACE_INTERRUPT, main_tid, 0, 0) != 0) {
+      PLOG(ERROR) << "failed to use PTRACE_INTERRUPT on " << main_tid;
     }
-  }
 
-  if (ptrace(PTRACE_DETACH, main_tid, 0, wait_for_gdb ? SIGSTOP : resume_signal)) {
-    PLOG(ERROR) << "ptrace detach from main thread " << main_tid << " failed";
+    if (tgkill(target, main_tid, wait_for_gdb ? SIGSTOP : signo) != 0) {
+      PLOG(ERROR) << "failed to resend signal " << signo << " to " << main_tid;
+    }
   }
 
   if (wait_for_gdb) {
-    if (tgkill(target, main_tid, resume_signal) != 0) {
-      PLOG(ERROR) << "failed to resend signal to process " << target;
-    }
+    // Use ALOGI to line up with output from engrave_tombstone.
+    ALOGI(
+      "***********************************************************\n"
+      "* Process %d has been suspended while crashing.\n"
+      "* To attach gdbserver and start gdb, run this on the host:\n"
+      "*\n"
+      "*     gdbclient.py -p %d\n"
+      "*\n"
+      "***********************************************************",
+      target, main_tid);
   }
 
   if (fatal_signal) {
diff --git a/debuggerd/debuggerd_test.cpp b/debuggerd/debuggerd_test.cpp
index f51b5f2..e7503e9 100644
--- a/debuggerd/debuggerd_test.cpp
+++ b/debuggerd/debuggerd_test.cpp
@@ -17,6 +17,7 @@
 #include <err.h>
 #include <fcntl.h>
 #include <unistd.h>
+#include <sys/prctl.h>
 #include <sys/types.h>
 
 #include <chrono>
@@ -90,6 +91,7 @@
   // Returns -1 if we fail to read a response from tombstoned, otherwise the received return code.
   void FinishIntercept(int* result);
 
+  void StartProcess(std::function<void()> function);
   void StartCrasher(const std::string& crash_type);
   void FinishCrasher();
   void AssertDeath(int signo);
@@ -166,9 +168,8 @@
   }
 }
 
-void CrasherTest::StartCrasher(const std::string& crash_type) {
-  std::string type = "wait-" + crash_type;
-
+void CrasherTest::StartProcess(std::function<void()> function) {
+  unique_fd read_pipe;
   unique_fd crasher_read_pipe;
   if (!Pipe(&crasher_read_pipe, &crasher_pipe)) {
     FAIL() << "failed to create pipe: " << strerror(errno);
@@ -182,9 +183,17 @@
     dup2(crasher_read_pipe.get(), STDIN_FILENO);
     dup2(devnull.get(), STDOUT_FILENO);
     dup2(devnull.get(), STDERR_FILENO);
+    function();
+    _exit(0);
+  }
+}
+
+void CrasherTest::StartCrasher(const std::string& crash_type) {
+  std::string type = "wait-" + crash_type;
+  StartProcess([type]() {
     execl(CRASHER_PATH, CRASHER_PATH, type.c_str(), nullptr);
     err(1, "exec failed");
-  }
+  });
 }
 
 void CrasherTest::FinishCrasher() {
@@ -340,24 +349,15 @@
   AssertDeath(SIGABRT);
 }
 
+// wait_for_gdb shouldn't trigger on manually sent signals.
 TEST_F(CrasherTest, wait_for_gdb_signal) {
   if (!android::base::SetProperty(kWaitForGdbKey, "1")) {
     FAIL() << "failed to enable wait_for_gdb";
   }
 
   StartCrasher("abort");
-  ASSERT_EQ(0, kill(crasher_pid, SIGABRT)) << strerror(errno);
-
-  std::this_thread::sleep_for(500ms);
-
-  int status;
-  ASSERT_EQ(crasher_pid, (TIMEOUT(1, waitpid(crasher_pid, &status, WUNTRACED))));
-  ASSERT_TRUE(WIFSTOPPED(status));
-  ASSERT_EQ(SIGSTOP, WSTOPSIG(status));
-
-  ASSERT_EQ(0, kill(crasher_pid, SIGCONT)) << strerror(errno);
-
-  AssertDeath(SIGABRT);
+  ASSERT_EQ(0, kill(crasher_pid, SIGSEGV)) << strerror(errno);
+  AssertDeath(SIGSEGV);
 }
 
 TEST_F(CrasherTest, backtrace) {
@@ -388,3 +388,20 @@
   ConsumeFd(std::move(output_fd), &result);
   ASSERT_MATCH(result, R"(#00 pc [0-9a-f]+\s+ /system/lib)" ARCH_SUFFIX R"(/libc.so \(tgkill)");
 }
+
+TEST_F(CrasherTest, PR_SET_DUMPABLE_0_crash) {
+  StartProcess([]() {
+    prctl(PR_SET_DUMPABLE, 0);
+    volatile char* null = static_cast<char*>(nullptr);
+    *null = '\0';
+  });
+  AssertDeath(SIGSEGV);
+}
+
+TEST_F(CrasherTest, PR_SET_DUMPABLE_0_raise) {
+  StartProcess([]() {
+    prctl(PR_SET_DUMPABLE, 0);
+    raise(SIGUSR1);
+  });
+  AssertDeath(SIGUSR1);
+}
diff --git a/debuggerd/handler/debuggerd_handler.cpp b/debuggerd/handler/debuggerd_handler.cpp
index 6033a6b..8ea06c0 100644
--- a/debuggerd/handler/debuggerd_handler.cpp
+++ b/debuggerd/handler/debuggerd_handler.cpp
@@ -31,6 +31,7 @@
 #include <errno.h>
 #include <fcntl.h>
 #include <inttypes.h>
+#include <linux/futex.h>
 #include <pthread.h>
 #include <sched.h>
 #include <signal.h>
@@ -46,6 +47,7 @@
 #include <sys/wait.h>
 #include <unistd.h>
 
+#include "private/bionic_futex.h"
 #include "private/libc_logging.h"
 
 // see man(2) prctl, specifically the section about PR_GET_NAME
@@ -61,6 +63,9 @@
 
 static debuggerd_callbacks_t g_callbacks;
 
+// Mutex to ensure only one crashing thread dumps itself.
+static pthread_mutex_t crash_mutex = PTHREAD_MUTEX_INITIALIZER;
+
 // Don't use __libc_fatal because it exits via abort, which might put us back into a signal handler.
 #define fatal(...)                                             \
   do {                                                         \
@@ -151,8 +156,7 @@
 }
 
 struct debugger_thread_info {
-  pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
-  bool crash_dump_started = false;
+  bool crash_dump_started;
   pid_t crashing_tid;
   pid_t pseudothread_tid;
   int signal_number;
@@ -228,16 +232,41 @@
     }
   }
 
-  pthread_mutex_unlock(&thread_info->mutex);
+  syscall(__NR_exit, 0);
   return 0;
 }
 
+static void resend_signal(siginfo_t* info) {
+  // Signals can either be fatal or nonfatal.
+  // For fatal signals, crash_dump will send us the signal we crashed with
+  // before resuming us, so that processes using waitpid on us will see that we
+  // exited with the correct exit status (e.g. so that sh will report
+  // "Segmentation fault" instead of "Killed"). For this to work, we need
+  // to deregister our signal handler for that signal before continuing.
+  if (info->si_signo != DEBUGGER_SIGNAL) {
+    signal(info->si_signo, SIG_DFL);
+  }
+
+  // We need to return from our signal handler so that crash_dump can see the
+  // signal via ptrace and dump the thread that crashed. However, returning
+  // does not guarantee that the signal will be thrown again, even for SIGSEGV
+  // and friends, since the signal could have been sent manually. We blocked
+  // all signals when registering the handler, so resending the signal (using
+  // rt_tgsigqueueinfo(2) to preserve SA_SIGINFO) will cause it to be delivered
+  // when our signal handler returns.
+  int rc = syscall(SYS_rt_tgsigqueueinfo, getpid(), gettid(), info->si_signo, info);
+  if (rc != 0) {
+    fatal("failed to resend signal during crash: %s", strerror(errno));
+  }
+
+  if (info->si_signo == DEBUGGER_SIGNAL) {
+    pthread_mutex_unlock(&crash_mutex);
+  }
+}
+
 // Handler that does crash dumping by forking and doing the processing in the child.
 // Do this by ptracing the relevant thread, and then execing debuggerd to do the actual dump.
 static void debuggerd_signal_handler(int signal_number, siginfo_t* info, void*) {
-  // Mutex to prevent multiple crashing threads from trying to talk
-  // to debuggerd at the same time.
-  static pthread_mutex_t crash_mutex = PTHREAD_MUTEX_INITIALIZER;
   int ret = pthread_mutex_lock(&crash_mutex);
   if (ret != 0) {
     __libc_format_log(ANDROID_LOG_INFO, "libc", "pthread_mutex_lock failed: %s", strerror(ret));
@@ -250,59 +279,10 @@
     info = nullptr;
   }
 
-  log_signal_summary(signal_number, info);
-  if (prctl(PR_GET_DUMPABLE, 0, 0, 0, 0) == 0) {
-    // process has disabled core dumps and PTRACE_ATTACH, and does not want to be dumped.
-    // Honor that intention by not connecting to debuggerd and asking it to dump our internal state.
-    __libc_format_log(ANDROID_LOG_INFO, "libc",
-                      "Suppressing debuggerd output because prctl(PR_GET_DUMPABLE)==0");
-
-    pthread_mutex_unlock(&crash_mutex);
-    return;
-  }
-
-  void* abort_message = nullptr;
-  if (g_callbacks.get_abort_message) {
-    abort_message = g_callbacks.get_abort_message();
-  }
-
-  debugger_thread_info thread_info = {
-    .crashing_tid = gettid(),
-    .signal_number = signal_number,
-    .info = info
-  };
-  pthread_mutex_lock(&thread_info.mutex);
-
-  // Essentially pthread_create without CLONE_FILES (see debuggerd_dispatch_pseudothread).
-  pid_t child_pid = clone(debuggerd_dispatch_pseudothread, pseudothread_stack,
-                          CLONE_THREAD | CLONE_SIGHAND | CLONE_VM | CLONE_CHILD_SETTID,
-                          &thread_info, nullptr, nullptr, &thread_info.pseudothread_tid);
-  if (child_pid == -1) {
-    fatal("failed to spawn debuggerd dispatch thread: %s", strerror(errno));
-  }
-
-  // Wait for the child to finish and unlock the mutex.
-  // This relies on bionic behavior that isn't guaranteed by the standard.
-  pthread_mutex_lock(&thread_info.mutex);
-
-  // Signals can either be fatal or nonfatal.
-  // For fatal signals, crash_dump will PTRACE_CONT us with the signal we
-  // crashed with, so that processes using waitpid on us will see that we
-  // exited with the correct exit status (e.g. so that sh will report
-  // "Segmentation fault" instead of "Killed"). For this to work, we need
-  // to deregister our signal handler for that signal before continuing.
-  if (signal_number != DEBUGGER_SIGNAL) {
-    signal(signal_number, SIG_DFL);
-  }
-
-  // We need to return from the signal handler so that debuggerd can dump the
-  // thread that crashed, but returning here does not guarantee that the signal
-  // will be thrown again, even for SIGSEGV and friends, since the signal could
-  // have been sent manually. Resend the signal with rt_tgsigqueueinfo(2) to
-  // preserve the SA_SIGINFO contents.
-  struct siginfo si;
+  struct siginfo si = {};
   if (!info) {
     memset(&si, 0, sizeof(si));
+    si.si_signo = signal_number;
     si.si_code = SI_USER;
     si.si_pid = getpid();
     si.si_uid = getuid();
@@ -314,23 +294,59 @@
     // check to allow all si_code values in calls coming from inside the house.
   }
 
+  log_signal_summary(signal_number, info);
+  if (prctl(PR_GET_DUMPABLE, 0, 0, 0, 0) == 0) {
+    // The process has disabled core dumps and PTRACE_ATTACH, and does not want to be dumped.
+    __libc_format_log(ANDROID_LOG_INFO, "libc",
+                      "Suppressing debuggerd output because prctl(PR_GET_DUMPABLE)==0");
+
+    resend_signal(info);
+    return;
+  }
+
+  void* abort_message = nullptr;
+  if (g_callbacks.get_abort_message) {
+    abort_message = g_callbacks.get_abort_message();
+  }
   // Populate si_value with the abort message address, if found.
   if (abort_message) {
     info->si_value.sival_ptr = abort_message;
   }
 
-  // Only resend the signal if we know that either crash_dump has ptraced us or
-  // the signal was fatal.
-  if (thread_info.crash_dump_started || signal_number != DEBUGGER_SIGNAL) {
-    int rc = syscall(SYS_rt_tgsigqueueinfo, getpid(), gettid(), signal_number, info);
-    if (rc != 0) {
-      fatal("failed to resend signal during crash: %s", strerror(errno));
-    }
+  debugger_thread_info thread_info = {
+    .crash_dump_started = false,
+    .pseudothread_tid = -1,
+    .crashing_tid = gettid(),
+    .signal_number = signal_number,
+    .info = info
+  };
+
+  // Essentially pthread_create without CLONE_FILES (see debuggerd_dispatch_pseudothread).
+  pid_t child_pid =
+    clone(debuggerd_dispatch_pseudothread, pseudothread_stack,
+          CLONE_THREAD | CLONE_SIGHAND | CLONE_VM | CLONE_CHILD_SETTID | CLONE_CHILD_CLEARTID,
+          &thread_info, nullptr, nullptr, &thread_info.pseudothread_tid);
+  if (child_pid == -1) {
+    fatal("failed to spawn debuggerd dispatch thread: %s", strerror(errno));
   }
 
-  if (signal_number == DEBUGGER_SIGNAL) {
-    pthread_mutex_unlock(&crash_mutex);
+  // Wait for the child to start...
+  __futex_wait(&thread_info.pseudothread_tid, -1, nullptr);
+
+  // and then wait for it to finish.
+  __futex_wait(&thread_info.pseudothread_tid, child_pid, nullptr);
+
+  // Signals can either be fatal or nonfatal.
+  // For fatal signals, crash_dump will PTRACE_CONT us with the signal we
+  // crashed with, so that processes using waitpid on us will see that we
+  // exited with the correct exit status (e.g. so that sh will report
+  // "Segmentation fault" instead of "Killed"). For this to work, we need
+  // to deregister our signal handler for that signal before continuing.
+  if (signal_number != DEBUGGER_SIGNAL) {
+    signal(signal_number, SIG_DFL);
   }
+
+  resend_signal(info);
 }
 
 void debuggerd_init(debuggerd_callbacks_t* callbacks) {
@@ -363,15 +379,5 @@
 
   // Use the alternate signal stack if available so we can catch stack overflows.
   action.sa_flags |= SA_ONSTACK;
-
-  sigaction(SIGABRT, &action, nullptr);
-  sigaction(SIGBUS, &action, nullptr);
-  sigaction(SIGFPE, &action, nullptr);
-  sigaction(SIGILL, &action, nullptr);
-  sigaction(SIGSEGV, &action, nullptr);
-#if defined(SIGSTKFLT)
-  sigaction(SIGSTKFLT, &action, nullptr);
-#endif
-  sigaction(SIGTRAP, &action, nullptr);
-  sigaction(DEBUGGER_SIGNAL, &action, nullptr);
+  debuggerd_register_handlers(&action);
 }
diff --git a/debuggerd/include/debuggerd/handler.h b/debuggerd/include/debuggerd/handler.h
index 302f4c2..7196e0a 100644
--- a/debuggerd/include/debuggerd/handler.h
+++ b/debuggerd/include/debuggerd/handler.h
@@ -39,4 +39,18 @@
 // to the log.
 #define DEBUGGER_SIGNAL (__SIGRTMIN + 3)
 
+static void __attribute__((__unused__)) debuggerd_register_handlers(struct sigaction* action) {
+  sigaction(SIGABRT, action, nullptr);
+  sigaction(SIGBUS, action, nullptr);
+  sigaction(SIGFPE, action, nullptr);
+  sigaction(SIGILL, action, nullptr);
+  sigaction(SIGSEGV, action, nullptr);
+#if defined(SIGSTKFLT)
+  sigaction(SIGSTKFLT, action, nullptr);
+#endif
+  sigaction(SIGSYS, action, nullptr);
+  sigaction(SIGTRAP, action, nullptr);
+  sigaction(DEBUGGER_SIGNAL, action, nullptr);
+}
+
 __END_DECLS
diff --git a/debuggerd/tombstoned/tombstoned.cpp b/debuggerd/tombstoned/tombstoned.cpp
index 63e3dbd..8705ece 100644
--- a/debuggerd/tombstoned/tombstoned.cpp
+++ b/debuggerd/tombstoned/tombstoned.cpp
@@ -85,7 +85,13 @@
     std::string path = android::base::StringPrintf("%stombstone_%02zu", kTombstoneDirectory, i);
     struct stat st;
     if (stat(path.c_str(), &st) != 0) {
-      PLOG(ERROR) << "failed to stat " << path;
+      if (errno == ENOENT) {
+        oldest_tombstone = i;
+        break;
+      } else {
+        PLOG(ERROR) << "failed to stat " << path;
+        continue;
+      }
     }
 
     if (st.st_mtime < oldest_time) {
diff --git a/debuggerd/tombstoned/tombstoned.rc b/debuggerd/tombstoned/tombstoned.rc
index eaae9c4..b8345ca 100644
--- a/debuggerd/tombstoned/tombstoned.rc
+++ b/debuggerd/tombstoned/tombstoned.rc
@@ -2,6 +2,9 @@
     user tombstoned
     group system
 
+    # Don't start tombstoned until after the real /data is mounted.
+    class late_start
+
     socket tombstoned_crash seqpacket 0666 system system
     socket tombstoned_intercept seqpacket 0666 system system
     writepid /dev/cpuset/system-background/tasks
diff --git a/init/init.cpp b/init/init.cpp
index ddc707f..1ce3c35 100644
--- a/init/init.cpp
+++ b/init/init.cpp
@@ -835,7 +835,12 @@
     parser.AddSectionParser("service",std::make_unique<ServiceParser>());
     parser.AddSectionParser("on", std::make_unique<ActionParser>());
     parser.AddSectionParser("import", std::make_unique<ImportParser>());
-    parser.ParseConfig("/init.rc");
+    std::string bootscript = property_get("ro.boot.init_rc");
+    if (bootscript.empty()) {
+        parser.ParseConfig("/init.rc");
+    } else {
+        parser.ParseConfig(bootscript);
+    }
 
     ActionManager& am = ActionManager::GetInstance();
 
diff --git a/init/property_service.cpp b/init/property_service.cpp
index 72fcb5b..9146d56 100644
--- a/init/property_service.cpp
+++ b/init/property_service.cpp
@@ -27,6 +27,7 @@
 #include <sys/poll.h>
 
 #include <memory>
+#include <vector>
 
 #include <cutils/misc.h>
 #include <cutils/sockets.h>
@@ -48,6 +49,7 @@
 
 #include <fs_mgr.h>
 #include <android-base/file.h>
+#include <android-base/strings.h>
 #include "bootimg.h"
 
 #include "property_service.h"
@@ -70,30 +72,30 @@
     }
 }
 
-static int check_mac_perms(const char *name, char *sctx, struct ucred *cr)
-{
-    char *tctx = NULL;
-    int result = 0;
+static bool check_mac_perms(const std::string& name, char* sctx, struct ucred* cr) {
+
+    if (!sctx) {
+      return false;
+    }
+
+    if (!sehandle_prop) {
+      return false;
+    }
+
+    char* tctx = nullptr;
+    if (selabel_lookup(sehandle_prop, &tctx, name.c_str(), 1) != 0) {
+      return false;
+    }
+
     property_audit_data audit_data;
 
-    if (!sctx)
-        goto err;
-
-    if (!sehandle_prop)
-        goto err;
-
-    if (selabel_lookup(sehandle_prop, &tctx, name, 1) != 0)
-        goto err;
-
-    audit_data.name = name;
+    audit_data.name = name.c_str();
     audit_data.cr = cr;
 
-    if (selinux_check_access(sctx, tctx, "property_service", "set", reinterpret_cast<void*>(&audit_data)) == 0)
-        result = 1;
+    bool has_access = (selinux_check_access(sctx, tctx, "property_service", "set", &audit_data) == 0);
 
     freecon(tctx);
- err:
-    return result;
+    return has_access;
 }
 
 static int check_control_mac_perms(const char *name, char *sctx, struct ucred *cr)
@@ -142,11 +144,9 @@
     }
 }
 
-bool is_legal_property_name(const std::string &name)
-{
+bool is_legal_property_name(const std::string& name) {
     size_t namelen = name.size();
 
-    if (namelen >= PROP_NAME_MAX) return false;
     if (namelen < 1) return false;
     if (name[0] == '.') return false;
     if (name[namelen - 1] == '.') return false;
@@ -169,58 +169,218 @@
     return true;
 }
 
-int property_set(const char* name, const char* value) {
-    size_t valuelen = strlen(value);
+uint32_t property_set(const std::string& name, const std::string& value) {
+    size_t valuelen = value.size();
 
     if (!is_legal_property_name(name)) {
         LOG(ERROR) << "property_set(\"" << name << "\", \"" << value << "\") failed: bad name";
-        return -1;
+        return PROP_ERROR_INVALID_NAME;
     }
+
     if (valuelen >= PROP_VALUE_MAX) {
         LOG(ERROR) << "property_set(\"" << name << "\", \"" << value << "\") failed: "
                    << "value too long";
-        return -1;
+        return PROP_ERROR_INVALID_VALUE;
     }
 
-    if (strcmp("selinux.restorecon_recursive", name) == 0 && valuelen > 0) {
-        if (restorecon(value, SELINUX_ANDROID_RESTORECON_RECURSE) != 0) {
+    if (name == "selinux.restorecon_recursive" && valuelen > 0) {
+        if (restorecon(value.c_str(), SELINUX_ANDROID_RESTORECON_RECURSE) != 0) {
             LOG(ERROR) << "Failed to restorecon_recursive " << value;
         }
     }
 
-    prop_info* pi = (prop_info*) __system_property_find(name);
+    prop_info* pi = (prop_info*) __system_property_find(name.c_str());
     if (pi != nullptr) {
         // ro.* properties are actually "write-once".
-        if (!strncmp(name, "ro.", 3)) {
+        if (android::base::StartsWith(name, "ro.")) {
             LOG(ERROR) << "property_set(\"" << name << "\", \"" << value << "\") failed: "
                        << "property already set";
-            return -1;
+            return PROP_ERROR_READ_ONLY_PROPERTY;
         }
 
-        __system_property_update(pi, value, valuelen);
+        __system_property_update(pi, value.c_str(), valuelen);
     } else {
-        int rc = __system_property_add(name, strlen(name), value, valuelen);
+        int rc = __system_property_add(name.c_str(), name.size(), value.c_str(), valuelen);
         if (rc < 0) {
             LOG(ERROR) << "property_set(\"" << name << "\", \"" << value << "\") failed: "
                        << "__system_property_add failed";
-            return rc;
+            return PROP_ERROR_SET_FAILED;
         }
     }
 
     // Don't write properties to disk until after we have read all default
     // properties to prevent them from being overwritten by default values.
-    if (persistent_properties_loaded && strncmp("persist.", name, strlen("persist.")) == 0) {
-        write_persistent_property(name, value);
+    if (persistent_properties_loaded && android::base::StartsWith(name, "persist.")) {
+        write_persistent_property(name.c_str(), value.c_str());
     }
-    property_changed(name, value);
-    return 0;
+    property_changed(name.c_str(), value.c_str());
+    return PROP_SUCCESS;
 }
 
-static void handle_property_set_fd()
-{
-    prop_msg msg;
-    int r;
-    char * source_ctx = NULL;
+class SocketConnection {
+ public:
+  SocketConnection(int socket, const struct ucred& cred)
+      : socket_(socket), cred_(cred) {}
+
+  ~SocketConnection() {
+    close(socket_);
+  }
+
+  bool RecvUint32(uint32_t* value, uint32_t* timeout_ms) {
+    return RecvFully(value, sizeof(*value), timeout_ms);
+  }
+
+  bool RecvChars(char* chars, size_t size, uint32_t* timeout_ms) {
+    return RecvFully(chars, size, timeout_ms);
+  }
+
+  bool RecvString(std::string* value, uint32_t* timeout_ms) {
+    uint32_t len = 0;
+    if (!RecvUint32(&len, timeout_ms)) {
+      return false;
+    }
+
+    if (len == 0) {
+      *value = "";
+      return true;
+    }
+
+    std::vector<char> chars(len);
+    if (!RecvChars(&chars[0], len, timeout_ms)) {
+      return false;
+    }
+
+    *value = std::string(&chars[0], len);
+    return true;
+  }
+
+  bool SendUint32(uint32_t value) {
+    int result = TEMP_FAILURE_RETRY(send(socket_, &value, sizeof(value), 0));
+    return result == sizeof(value);
+  }
+
+  int socket() {
+    return socket_;
+  }
+
+  const struct ucred& cred() {
+    return cred_;
+  }
+
+ private:
+  bool PollIn(uint32_t* timeout_ms) {
+    /* Default 2 sec timeout for caller to send data. */
+    struct pollfd ufds[1];
+    ufds[0].fd = socket_;
+    ufds[0].events = POLLIN;
+    ufds[0].revents = 0;
+    while (timeout_ms > 0) {
+      Timer timer;
+      int nr = poll(ufds, 1, *timeout_ms);
+      uint64_t millis = timer.duration_ns()/1000000;
+      *timeout_ms = (millis > *timeout_ms) ? 0 : *timeout_ms - millis;
+
+      if (nr > 0) {
+        return true;
+      }
+
+      if (nr == 0) {
+        // Timeout
+        break;
+      }
+
+      if (nr < 0 && errno != EINTR) {
+        PLOG(ERROR) << "sys_prop: error waiting for uid " << cred_.uid << " to send property message";
+        return false;
+      } else { // errno == EINTR
+        // Timer rounds milliseconds down in case of EINTR we want it to be rounded up
+        // to avoid slowing init down by causing EINTR with under millisecond timeout.
+        if (*timeout_ms > 0) {
+          --(*timeout_ms);
+        }
+      }
+    }
+
+    LOG(ERROR) << "sys_prop: timeout waiting for uid " << cred_.uid << " to send property message.";
+    return false;
+  }
+
+  bool RecvFully(void* data_ptr, size_t size, uint32_t* timeout_ms) {
+    size_t bytes_left = size;
+    char* data = static_cast<char*>(data_ptr);
+    while (*timeout_ms > 0 && bytes_left > 0) {
+      if (!PollIn(timeout_ms)) {
+        return false;
+      }
+
+      int result = TEMP_FAILURE_RETRY(recv(socket_, data, bytes_left, MSG_DONTWAIT));
+      if (result <= 0) {
+        return false;
+      }
+
+      bytes_left -= result;
+      data += result;
+    }
+
+    return bytes_left == 0;
+  }
+
+  int socket_;
+  struct ucred cred_;
+
+  DISALLOW_IMPLICIT_CONSTRUCTORS(SocketConnection);
+};
+
+static void handle_property_set(SocketConnection& socket,
+                                const std::string& name,
+                                const std::string& value,
+                                bool legacy_protocol) {
+  const char* cmd_name = legacy_protocol ? "PROP_MSG_SETPROP" : "PROP_MSG_SETPROP2";
+  if (!is_legal_property_name(name)) {
+    LOG(ERROR) << "sys_prop(" << cmd_name << "): illegal property name \"" << name << "\"";
+    socket.SendUint32(PROP_ERROR_INVALID_NAME);
+    return;
+  }
+
+  struct ucred cr = socket.cred();
+  char* source_ctx = nullptr;
+  getpeercon(socket.socket(), &source_ctx);
+
+  if (android::base::StartsWith(name, "ctl.")) {
+    if (check_control_mac_perms(value.c_str(), source_ctx, &cr)) {
+      handle_control_message(name.c_str() + 4, value.c_str());
+      if (!legacy_protocol) {
+        socket.SendUint32(PROP_SUCCESS);
+      }
+    } else {
+      LOG(ERROR) << "sys_prop(" << cmd_name << "): Unable to " << (name.c_str() + 4)
+                 << " service ctl [" << value << "]"
+                 << " uid:" << cr.uid
+                 << " gid:" << cr.gid
+                 << " pid:" << cr.pid;
+      if (!legacy_protocol) {
+        socket.SendUint32(PROP_ERROR_HANDLE_CONTROL_MESSAGE);
+      }
+    }
+  } else {
+    if (check_mac_perms(name, source_ctx, &cr)) {
+      uint32_t result = property_set(name, value);
+      if (!legacy_protocol) {
+        socket.SendUint32(result);
+      }
+    } else {
+      LOG(ERROR) << "sys_prop(" << cmd_name << "): permission denied uid:" << cr.uid << " name:" << name;
+      if (!legacy_protocol) {
+        socket.SendUint32(PROP_ERROR_PERMISSION_DENIED);
+      }
+    }
+  }
+
+  freecon(source_ctx);
+}
+
+static void handle_property_set_fd() {
+    static constexpr uint32_t kDefaultSocketTimeout = 2000; /* ms */
 
     int s = accept(property_set_fd, nullptr, nullptr);
     if (s == -1) {
@@ -236,72 +396,50 @@
         return;
     }
 
-    static constexpr int timeout_ms = 2 * 1000;  /* Default 2 sec timeout for caller to send property. */
-    struct pollfd ufds[1];
-    ufds[0].fd = s;
-    ufds[0].events = POLLIN;
-    ufds[0].revents = 0;
-    int nr = TEMP_FAILURE_RETRY(poll(ufds, 1, timeout_ms));
-    if (nr == 0) {
-        LOG(ERROR) << "sys_prop: timeout waiting for uid " << cr.uid << " to send property message.";
-        close(s);
-        return;
-    } else if (nr < 0) {
-        PLOG(ERROR) << "sys_prop: error waiting for uid " << cr.uid << " to send property message";
-        close(s);
+    SocketConnection socket(s, cr);
+    uint32_t timeout_ms = kDefaultSocketTimeout;
+
+    uint32_t cmd = 0;
+
+    if (!socket.RecvUint32(&cmd, &timeout_ms)) {
+        PLOG(ERROR) << "sys_prop: error while reading command from the socket";
+        socket.SendUint32(PROP_ERROR_READ_CMD);
         return;
     }
 
-    r = TEMP_FAILURE_RETRY(recv(s, &msg, sizeof(msg), MSG_DONTWAIT));
-    if(r != sizeof(prop_msg)) {
-        PLOG(ERROR) << "sys_prop: mis-match msg size received: " << r << " expected: " << sizeof(prop_msg);
-        close(s);
-        return;
-    }
+    switch(cmd) {
+    case PROP_MSG_SETPROP: {
+        char prop_name[PROP_NAME_MAX];
+        char prop_value[PROP_VALUE_MAX];
 
-    switch(msg.cmd) {
-    case PROP_MSG_SETPROP:
-        msg.name[PROP_NAME_MAX-1] = 0;
-        msg.value[PROP_VALUE_MAX-1] = 0;
-
-        if (!is_legal_property_name(msg.name)) {
-            LOG(ERROR) << "sys_prop: illegal property name \"" << msg.name << "\"";
-            close(s);
-            return;
+        if (!socket.RecvChars(prop_name, PROP_NAME_MAX, &timeout_ms) ||
+            !socket.RecvChars(prop_value, PROP_VALUE_MAX, &timeout_ms)) {
+          PLOG(ERROR) << "sys_prop(PROP_MSG_SETPROP): error while reading name/value from the socket";
+          return;
         }
 
-        getpeercon(s, &source_ctx);
+        prop_name[PROP_NAME_MAX-1] = 0;
+        prop_value[PROP_VALUE_MAX-1] = 0;
 
-        if(memcmp(msg.name,"ctl.",4) == 0) {
-            // Keep the old close-socket-early behavior when handling
-            // ctl.* properties.
-            close(s);
-            if (check_control_mac_perms(msg.value, source_ctx, &cr)) {
-                handle_control_message((char*) msg.name + 4, (char*) msg.value);
-            } else {
-                LOG(ERROR) << "sys_prop: Unable to " << (msg.name + 4)
-                           << " service ctl [" << msg.value << "]"
-                           << " uid:" << cr.uid
-                           << " gid:" << cr.gid
-                           << " pid:" << cr.pid;
-            }
-        } else {
-            if (check_mac_perms(msg.name, source_ctx, &cr)) {
-                property_set((char*) msg.name, (char*) msg.value);
-            } else {
-                LOG(ERROR) << "sys_prop: permission denied uid:" << cr.uid << " name:" << msg.name;
-            }
-
-            // Note: bionic's property client code assumes that the
-            // property server will not close the socket until *AFTER*
-            // the property is written to memory.
-            close(s);
-        }
-        freecon(source_ctx);
+        handle_property_set(socket, prop_value, prop_value, true);
         break;
+      }
 
+    case PROP_MSG_SETPROP2: {
+        std::string name;
+        std::string value;
+        if (!socket.RecvString(&name, &timeout_ms) ||
+            !socket.RecvString(&value, &timeout_ms)) {
+          PLOG(ERROR) << "sys_prop(PROP_MSG_SETPROP2): error while reading name/value from the socket";
+          socket.SendUint32(PROP_ERROR_READ_DATA);
+          return;
+        }
+
+        handle_property_set(socket, name, value, false);
+        break;
+      }
     default:
-        close(s);
+        socket.SendUint32(PROP_ERROR_INVALID_CMD);
         break;
     }
 }
@@ -510,6 +648,8 @@
 }
 
 void start_property_service() {
+    property_set("ro.property_service.version", "2");
+
     property_set_fd = create_socket(PROP_SERVICE_NAME, SOCK_STREAM | SOCK_CLOEXEC | SOCK_NONBLOCK,
                                     0666, 0, 0, NULL);
     if (property_set_fd == -1) {
diff --git a/init/property_service.h b/init/property_service.h
index e3a2acb..5d59473 100644
--- a/init/property_service.h
+++ b/init/property_service.h
@@ -27,14 +27,14 @@
     const char* name;
 };
 
-extern void property_init(void);
-extern void property_load_boot_defaults(void);
-extern void load_persist_props(void);
-extern void load_system_props(void);
-extern void start_property_service(void);
+void property_init(void);
+void property_load_boot_defaults(void);
+void load_persist_props(void);
+void load_system_props(void);
+void start_property_service(void);
 std::string property_get(const char* name);
-extern int property_set(const char *name, const char *value);
-extern bool is_legal_property_name(const std::string &name);
+uint32_t property_set(const std::string& name, const std::string& value);
+bool is_legal_property_name(const std::string& name);
 
 
 #endif  /* _INIT_PROPERTY_H */
diff --git a/init/seccomp.cpp b/init/seccomp.cpp
index d9f2f79..d632302 100644
--- a/init/seccomp.cpp
+++ b/init/seccomp.cpp
@@ -170,6 +170,12 @@
     // Needed for trusty
     AllowSyscall(f, __NR_syncfs);
 
+    // Needed for strace
+    AllowSyscall(f, __NR_tkill);  // __NR_tkill
+
+    // Needed for kernel to restart syscalls
+    AllowSyscall(f, __NR_restart_syscall);
+
      // arm64-only filter - autogenerated from bionic syscall usage
     for (size_t i = 0; i < arm64_filter_size; ++i)
         f.push_back(arm64_filter[i]);
@@ -201,6 +207,12 @@
     // Syscalls needed to run GFXBenchmark
     AllowSyscall(f, 190); // __NR_vfork
 
+    // Needed for strace
+    AllowSyscall(f, 238);  // __NR_tkill
+
+    // Needed for kernel to restart syscalls
+    AllowSyscall(f, 0);  // __NR_restart_syscall
+
     // arm32-on-arm64 only filter - autogenerated from bionic syscall usage
     for (size_t i = 0; i < arm_filter_size; ++i)
         f.push_back(arm_filter[i]);
diff --git a/logcat/tests/logcat_test.cpp b/logcat/tests/logcat_test.cpp
index 8a390f6..337861e 100644
--- a/logcat/tests/logcat_test.cpp
+++ b/logcat/tests/logcat_test.cpp
@@ -23,6 +23,7 @@
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <sys/wait.h>
+#include <unistd.h>
 
 #include <memory>
 #include <string>
@@ -54,6 +55,11 @@
 TEST(logcat, buckets) {
     FILE *fp;
 
+#undef LOG_TAG
+#define LOG_TAG "inject"
+    RLOGE("logcat.buckets");
+    sleep(1);
+
     ASSERT_TRUE(NULL != (fp = popen(
       "logcat -b radio -b events -b system -b main -d 2>/dev/null",
       "r")));
@@ -165,13 +171,39 @@
         if (!ep) {
             continue;
         }
-        ep -= 7;
+        static const size_t tag_field_width = 7;
+        ep -= tag_field_width;
         *ep = '\0';
         return cp;
     }
     return NULL;
 }
 
+// If there is not enough background noise in the logs, then spam the logs to
+// permit tail checking so that the tests can progress.
+static size_t inject(ssize_t count) {
+    if (count <= 0) return 0;
+
+    static const size_t retry = 3;
+    size_t errors = retry;
+    size_t num = 0;
+    for(;;) {
+        log_time ts(CLOCK_MONOTONIC);
+        if (__android_log_btwrite(0, EVENT_TYPE_LONG, &ts, sizeof(ts)) >= 0) {
+            if (++num >= (size_t)count) {
+                sleep(1); // let data settle end-to-end
+                return num;
+            }
+            errors = retry;
+            usleep(50);
+        } else if (--errors <= 0) {
+            return num;
+        }
+    }
+    // NOTREACH
+    return num;
+}
+
 TEST(logcat, tz) {
 
     if (android_log_clockid() == CLOCK_MONOTONIC) {
@@ -201,7 +233,7 @@
 
         pclose(fp);
 
-    } while ((count < 3) && --tries && (sleep(1), true));
+    } while ((count < 3) && --tries && inject(3 - count));
 
     ASSERT_EQ(3, count);
 }
@@ -236,8 +268,7 @@
         char buffer[BIG_BUFFER];
 
         snprintf(buffer, sizeof(buffer),
-          "logcat -v long -b radio -b events -b system -b main -t %d 2>/dev/null",
-          num);
+                 "logcat -v long -b all -t %d 2>/dev/null", num);
 
         FILE *fp;
         ASSERT_TRUE(NULL != (fp = popen(buffer, "r")));
@@ -250,7 +281,7 @@
 
         pclose(fp);
 
-    } while ((count < num) && --tries && (sleep(1), true));
+    } while ((count < num) && --tries && inject(num - count));
 
     ASSERT_EQ(num, count);
 }
@@ -273,26 +304,34 @@
 
 TEST(logcat, tail_time) {
     FILE *fp;
-
-    ASSERT_TRUE(NULL != (fp = popen("logcat -v long -b all -t 10 2>&1", "r")));
-
+    int count;
     char buffer[BIG_BUFFER];
     char *last_timestamp = NULL;
     char *first_timestamp = NULL;
-    int count = 0;
-
     char *cp;
-    while ((cp = fgetLongTime(buffer, sizeof(buffer), fp))) {
-        ++count;
-        if (!first_timestamp) {
-            first_timestamp = strdup(cp);
-        }
-        free(last_timestamp);
-        last_timestamp = strdup(cp);
-    }
-    pclose(fp);
 
-    EXPECT_EQ(10, count);
+    int tries = 3; // in case run too soon after system start or buffer clear
+
+    // Do not be tempted to use -v usec because that increases the
+    // chances of an occasional test failure by 1000 (see below).
+    do {
+        ASSERT_TRUE(NULL != (fp = popen("logcat -v long -b all -t 10 2>&1", "r")));
+
+        count = 0;
+
+        while ((cp = fgetLongTime(buffer, sizeof(buffer), fp))) {
+            ++count;
+            if (!first_timestamp) {
+                first_timestamp = strdup(cp);
+            }
+            free(last_timestamp);
+            last_timestamp = strdup(cp);
+        }
+        pclose(fp);
+
+    } while ((count < 10) && --tries && inject(10 - count));
+
+    EXPECT_EQ(10, count); // We want _some_ history, too small, falses below
     EXPECT_TRUE(last_timestamp != NULL);
     EXPECT_TRUE(first_timestamp != NULL);
 
diff --git a/rootdir/init.rc b/rootdir/init.rc
index 8d974fa..d6ae6f0 100644
--- a/rootdir/init.rc
+++ b/rootdir/init.rc
@@ -359,9 +359,6 @@
     start vold
     installkey /data
 
-    # start tombstoned to record early-boot crashes.
-    start tombstoned
-
     # Start bootcharting as soon as possible after the data partition is
     # mounted to collect more data.
     mkdir /data/bootchart 0755 shell shell