linux: Lazily initialize ProcessInfo’s Is64Bit() and StartTime()

Lazy initialization is particularly beneficial for Is64Bit(), which uses
a different (ptrace()-based) approach than the rest of the class (which
is /proc-based). It is possible for the /proc-based Initialize() to
succeed while ptrace() would fail, as it typically would in the
ProcessInfo.Pid1 test. Because this test does not call Is64Bit(),
permission to ptrace() shouldn’t be necessary, and in fact ptrace()
shouldn’t even be called.

This enables the ProcessInfo.Pid1 test on Android (due to ptrace(), it
was actually failing on any Linux, not just Android). It also enables
the ProcessInfo.Forked test on non-Linux, as the prctl(PR_SET_DUMPABLE)
Linux-ism can be removed from it.

Bug: crashpad:30
Test: crashpad_util_test ProcessInfo.*
Change-Id: Ic883733a6aed7e7de9a0f070a5a3544126c7e976
Reviewed-on: https://chromium-review.googlesource.com/455656
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>

snapshot/mac/process_reader.cc

@@ -116,7 +116,9 @@ bool ProcessReader::Initialize(task_t task) {
     return false;
   }
 
-  is_64_bit_ = process_info_.Is64Bit();
+  if (!process_info_.Is64Bit(&is_64_bit_)) {
+    return false;
+  }
 
   task_memory_.reset(new TaskMemory(task));
   task_ = task;
@@ -125,6 +127,11 @@ bool ProcessReader::Initialize(task_t task) {
   return true;
 }
 
+void ProcessReader::StartTime(timeval* start_time) const {
+  bool rv = process_info_.StartTime(start_time);
+  DCHECK(rv);
+}
+
 bool ProcessReader::CPUTimes(timeval* user_time, timeval* system_time) const {
   INITIALIZATION_STATE_DCHECK_VALID(initialized_);
 

snapshot/mac/process_reader.h

@@ -124,9 +124,7 @@ class ProcessReader {
   //! \brief Determines the target process’ start time.
   //!
   //! \param[out] start_time The time that the process started.
-  void StartTime(timeval* start_time) const {
-    process_info_.StartTime(start_time);
-  }
+  void StartTime(timeval* start_time) const;
 
   //! \brief Determines the target process’ execution time.
   //!

util/posix/process_info.h

@@ -25,6 +25,7 @@
 
 #include "base/macros.h"
 #include "build/build_config.h"
+#include "util/misc/initialization_state.h"
 #include "util/misc/initialization_state_dcheck.h"
 
 #if defined(OS_MACOSX)
@@ -113,13 +114,21 @@ class ProcessInfo {
   //! `execve()` as a result of executing a setuid or setgid executable.
   bool DidChangePrivileges() const;
 
-  //! \return `true` if the target task is a 64-bit process.
-  bool Is64Bit() const;
+  //! \brief Determines the target process’ bitness.
+  //!
+  //! \param[out] is_64_bit `true` if the target task is a 64-bit process.
+  //!
+  //! \return `true` on success, with \a is_64_bit set. Otherwise, `false` with
+  //!     a message logged.
+  bool Is64Bit(bool* is_64_bit) const;
 
   //! \brief Determines the target process’ start time.
   //!
   //! \param[out] start_time The time that the process started.
-  void StartTime(timeval* start_time) const;
+  //!
+  //! \return `true` on success, with \a start_time set. Otherwise, `false` with
+  //!     a message logged.
+  bool StartTime(timeval* start_time) const;
 
   //! \brief Obtains the arguments used to launch a process.
   //!
@@ -142,8 +151,13 @@ class ProcessInfo {
 #if defined(OS_MACOSX)
   kinfo_proc kern_proc_info_;
 #elif defined(OS_LINUX) || defined(OS_ANDROID)
+  // Some members are marked mutable so that they can be lazily initialized by
+  // const methods. These are always InitializationState-protected so that
+  // multiple successive calls will always produce the same return value and out
+  // parameters. This is necessary for intergration with the Snapshot interface.
+  // See https://crashpad.chromium.org/bug/9.
   std::set<gid_t> supplementary_groups_;
-  timeval start_time_;
+  mutable timeval start_time_;
   pid_t pid_;
   pid_t ppid_;
   uid_t uid_;
@@ -152,7 +166,9 @@ class ProcessInfo {
   gid_t gid_;
   gid_t egid_;
   gid_t sgid_;
-  bool is_64_bit_;
+  mutable InitializationState start_time_initialized_;
+  mutable InitializationState is_64_bit_initialized_;
+  mutable bool is_64_bit_;
 #endif
   InitializationStateDcheck initialized_;
 

util/posix/process_info_linux.cc

@@ -114,6 +114,21 @@ void TimespecToTimeval(const timespec& ts, timeval* tv) {
   tv->tv_usec = ts.tv_nsec / 1000;
 }
 
+class ScopedPtraceDetach {
+ public:
+  explicit ScopedPtraceDetach(pid_t pid) : pid_(pid) {}
+  ~ScopedPtraceDetach() {
+    if (ptrace(PTRACE_DETACH, pid_, nullptr, nullptr) != 0) {
+      PLOG(ERROR) << "ptrace";
+    }
+  }
+
+ private:
+  pid_t pid_;
+
+  DISALLOW_COPY_AND_ASSIGN(ScopedPtraceDetach);
+};
+
 }  // namespace
 
 ProcessInfo::ProcessInfo()
@@ -127,6 +142,8 @@ ProcessInfo::ProcessInfo()
       gid_(-1),
       egid_(-1),
       sgid_(-1),
+      start_time_initialized_(),
+      is_64_bit_initialized_(),
       is_64_bit_(false),
       initialized_() {}
 
@@ -228,7 +245,145 @@ bool ProcessInfo::Initialize(pid_t pid) {
     }
   }
 
-  {
+  INITIALIZATION_STATE_SET_VALID(initialized_);
+  return true;
+}
+
+pid_t ProcessInfo::ProcessID() const {
+  INITIALIZATION_STATE_DCHECK_VALID(initialized_);
+  return pid_;
+}
+
+pid_t ProcessInfo::ParentProcessID() const {
+  INITIALIZATION_STATE_DCHECK_VALID(initialized_);
+  return ppid_;
+}
+
+uid_t ProcessInfo::RealUserID() const {
+  INITIALIZATION_STATE_DCHECK_VALID(initialized_);
+  return uid_;
+}
+
+uid_t ProcessInfo::EffectiveUserID() const {
+  INITIALIZATION_STATE_DCHECK_VALID(initialized_);
+  return euid_;
+}
+
+uid_t ProcessInfo::SavedUserID() const {
+  INITIALIZATION_STATE_DCHECK_VALID(initialized_);
+  return suid_;
+}
+
+gid_t ProcessInfo::RealGroupID() const {
+  INITIALIZATION_STATE_DCHECK_VALID(initialized_);
+  return gid_;
+}
+
+gid_t ProcessInfo::EffectiveGroupID() const {
+  INITIALIZATION_STATE_DCHECK_VALID(initialized_);
+  return egid_;
+}
+
+gid_t ProcessInfo::SavedGroupID() const {
+  INITIALIZATION_STATE_DCHECK_VALID(initialized_);
+  return sgid_;
+}
+
+std::set<gid_t> ProcessInfo::SupplementaryGroups() const {
+  INITIALIZATION_STATE_DCHECK_VALID(initialized_);
+  return supplementary_groups_;
+}
+
+std::set<gid_t> ProcessInfo::AllGroups() const {
+  INITIALIZATION_STATE_DCHECK_VALID(initialized_);
+
+  std::set<gid_t> all_groups = SupplementaryGroups();
+  all_groups.insert(RealGroupID());
+  all_groups.insert(EffectiveGroupID());
+  all_groups.insert(SavedGroupID());
+  return all_groups;
+}
+
+bool ProcessInfo::DidChangePrivileges() const {
+  INITIALIZATION_STATE_DCHECK_VALID(initialized_);
+  // TODO(jperaza): Is this possible to determine?
+  return false;
+}
+
+bool ProcessInfo::Is64Bit(bool* is_64_bit) const {
+  INITIALIZATION_STATE_DCHECK_VALID(initialized_);
+
+  if (is_64_bit_initialized_.is_uninitialized()) {
+    is_64_bit_initialized_.set_invalid();
+
+#if defined(ARCH_CPU_64_BITS)
+    const bool am_64_bit = true;
+#else
+    const bool am_64_bit = false;
+#endif
+
+    if (pid_ == getpid()) {
+      is_64_bit_ = am_64_bit;
+    } else {
+      if (ptrace(PTRACE_ATTACH, pid_, nullptr, nullptr) != 0) {
+        PLOG(ERROR) << "ptrace";
+        return false;
+      }
+
+      ScopedPtraceDetach ptrace_detach(pid_);
+
+      if (HANDLE_EINTR(waitpid(pid_, nullptr, __WALL)) < 0) {
+        PLOG(ERROR) << "waitpid";
+        return false;
+      }
+
+      // Allocate more buffer space than is required to hold registers for this
+      // process. If the kernel fills the extra space, the target process uses
+      // more/larger registers than this process. If the kernel fills less space
+      // than sizeof(regs) then the target process uses smaller/fewer registers.
+      struct {
+#if defined(ARCH_CPU_X86_FAMILY)
+        using PrStatusType = user_regs_struct;
+#elif defined(ARCH_CPU_ARMEL)
+        using PrStatusType = pt_regs;
+#elif defined(ARCH_CPU_ARM64)
+        using PrStatusType = user_pt_regs;
+#endif
+        PrStatusType regs;
+        char extra;
+      } regbuf;
+
+      iovec iov;
+      iov.iov_base = &regbuf;
+      iov.iov_len = sizeof(regbuf);
+      if (ptrace(PTRACE_GETREGSET,
+                 pid_,
+                 reinterpret_cast<void*>(NT_PRSTATUS),
+                 &iov) != 0) {
+        PLOG(ERROR) << "ptrace";
+        return false;
+      }
+
+      is_64_bit_ = am_64_bit == (iov.iov_len == sizeof(regbuf.regs));
+    }
+
+    is_64_bit_initialized_.set_valid();
+  }
+
+  if (!is_64_bit_initialized_.is_valid()) {
+    return false;
+  }
+
+  *is_64_bit = is_64_bit_;
+  return true;
+}
+
+bool ProcessInfo::StartTime(timeval* start_time) const {
+  INITIALIZATION_STATE_DCHECK_VALID(initialized_);
+
+  if (start_time_initialized_.is_uninitialized()) {
+    start_time_initialized_.set_invalid();
+
     char path[32];
     snprintf(path, sizeof(path), "/proc/%d/stat", pid_);
     std::string stat_contents;
@@ -295,134 +450,16 @@ bool ProcessInfo::Initialize(pid_t pid) {
     timeval boot_time_tv;
     TimespecToTimeval(boot_time_ts, &boot_time_tv);
     timeradd(&boot_time_tv, &time_after_boot, &start_time_);
+
+    start_time_initialized_.set_valid();
   }
 
-#if defined(ARCH_CPU_64_BITS)
-  const bool am_64_bit = true;
-#else
-  const bool am_64_bit = false;
-#endif
-
-  if (pid_ == getpid()) {
-    is_64_bit_ = am_64_bit;
-  } else {
-    if (ptrace(PTRACE_ATTACH, pid_, nullptr, nullptr) != 0) {
-      PLOG(ERROR) << "ptrace";
-      return false;
-    }
-
-    if (HANDLE_EINTR(waitpid(pid_, nullptr, __WALL)) < 0) {
-      PLOG(ERROR) << "waitpid";
-      return false;
-    }
-
-    // Allocate more buffer space than is required to hold registers for this
-    // process. If the kernel fills the extra space, the target process uses
-    // more/larger registers than this process. If the kernel fills less space
-    // than sizeof(regs) then the target process uses smaller/fewer registers.
-    struct {
-#if defined(ARCH_CPU_X86_FAMILY)
-      using PrStatusType = user_regs_struct;
-#elif defined(ARCH_CPU_ARMEL)
-      using PrStatusType = pt_regs;
-#elif defined(ARCH_CPU_ARM64)
-      using PrStatusType = user_pt_regs;
-#endif
-      PrStatusType regs;
-      char extra;
-    } regbuf;
-
-    iovec iov;
-    iov.iov_base = &regbuf;
-    iov.iov_len = sizeof(regbuf);
-    if (ptrace(PTRACE_GETREGSET,
-               pid_,
-               reinterpret_cast<void*>(NT_PRSTATUS),
-               &iov) != 0) {
-      PLOG(ERROR) << "ptrace";
-      return false;
-    }
-
-    is_64_bit_ = am_64_bit == (iov.iov_len == sizeof(regbuf.regs));
-
-    if (ptrace(PTRACE_DETACH, pid_, nullptr, nullptr) != 0) {
-      PLOG(ERROR) << "ptrace";
-    }
+  if (!start_time_initialized_.is_valid()) {
+    return false;
   }
 
-  INITIALIZATION_STATE_SET_VALID(initialized_);
-  return true;
-}
-
-pid_t ProcessInfo::ProcessID() const {
-  INITIALIZATION_STATE_DCHECK_VALID(initialized_);
-  return pid_;
-}
-
-pid_t ProcessInfo::ParentProcessID() const {
-  INITIALIZATION_STATE_DCHECK_VALID(initialized_);
-  return ppid_;
-}
-
-uid_t ProcessInfo::RealUserID() const {
-  INITIALIZATION_STATE_DCHECK_VALID(initialized_);
-  return uid_;
-}
-
-uid_t ProcessInfo::EffectiveUserID() const {
-  INITIALIZATION_STATE_DCHECK_VALID(initialized_);
-  return euid_;
-}
-
-uid_t ProcessInfo::SavedUserID() const {
-  INITIALIZATION_STATE_DCHECK_VALID(initialized_);
-  return suid_;
-}
-
-gid_t ProcessInfo::RealGroupID() const {
-  INITIALIZATION_STATE_DCHECK_VALID(initialized_);
-  return gid_;
-}
-
-gid_t ProcessInfo::EffectiveGroupID() const {
-  INITIALIZATION_STATE_DCHECK_VALID(initialized_);
-  return egid_;
-}
-
-gid_t ProcessInfo::SavedGroupID() const {
-  INITIALIZATION_STATE_DCHECK_VALID(initialized_);
-  return sgid_;
-}
-
-std::set<gid_t> ProcessInfo::SupplementaryGroups() const {
-  INITIALIZATION_STATE_DCHECK_VALID(initialized_);
-  return supplementary_groups_;
-}
-
-std::set<gid_t> ProcessInfo::AllGroups() const {
-  INITIALIZATION_STATE_DCHECK_VALID(initialized_);
-
-  std::set<gid_t> all_groups = SupplementaryGroups();
-  all_groups.insert(RealGroupID());
-  all_groups.insert(EffectiveGroupID());
-  all_groups.insert(SavedGroupID());
-  return all_groups;
-}
-
-bool ProcessInfo::DidChangePrivileges() const {
-  INITIALIZATION_STATE_DCHECK_VALID(initialized_);
-  // TODO(jperaza): Is this possible to determine?
-  return false;
-}
-
-bool ProcessInfo::Is64Bit() const {
-  INITIALIZATION_STATE_DCHECK_VALID(initialized_);
-  return is_64_bit_;
-}
-
-void ProcessInfo::StartTime(timeval* start_time) const {
-  INITIALIZATION_STATE_DCHECK_VALID(initialized_);
   *start_time = start_time_;
+  return true;
 }
 
 bool ProcessInfo::Arguments(std::vector<std::string>* argv) const {

util/posix/process_info_mac.cc

@@ -132,14 +132,16 @@ bool ProcessInfo::DidChangePrivileges() const {
   return kern_proc_info_.kp_proc.p_flag & P_SUGID;
 }
 
-bool ProcessInfo::Is64Bit() const {
+bool ProcessInfo::Is64Bit(bool* is_64_bit) const {
   INITIALIZATION_STATE_DCHECK_VALID(initialized_);
-  return kern_proc_info_.kp_proc.p_flag & P_LP64;
+  *is_64_bit = kern_proc_info_.kp_proc.p_flag & P_LP64;
+  return true;
 }
 
-void ProcessInfo::StartTime(timeval* start_time) const {
+bool ProcessInfo::StartTime(timeval* start_time) const {
   INITIALIZATION_STATE_DCHECK_VALID(initialized_);
   *start_time = kern_proc_info_.kp_proc.p_starttime;
+  return true;
 }
 
 bool ProcessInfo::Arguments(std::vector<std::string>* argv) const {

util/posix/process_info_test.cc

@@ -14,8 +14,9 @@
 
 #include "util/posix/process_info.h"
 
-#include <time.h>
+#include <signal.h>
 #include <stdio.h>
+#include <time.h>
 #include <unistd.h>
 
 #include <set>
@@ -40,7 +41,7 @@ namespace crashpad {
 namespace test {
 namespace {
 
-void TestProcessClone(const ProcessInfo& process_info) {
+void TestProcessSelfOrClone(const ProcessInfo& process_info) {
   // There’s no system call to obtain the saved set-user ID or saved set-group
   // ID in an easy way. Normally, they are the same as the effective user ID and
   // effective group ID, so just check against those.
@@ -80,15 +81,18 @@ void TestProcessClone(const ProcessInfo& process_info) {
   // The test executable isn’t expected to change privileges.
   EXPECT_FALSE(process_info.DidChangePrivileges());
 
+  bool is_64_bit;
+  ASSERT_TRUE(process_info.Is64Bit(&is_64_bit));
 #if defined(ARCH_CPU_64_BITS)
-  EXPECT_TRUE(process_info.Is64Bit());
+  EXPECT_TRUE(is_64_bit);
 #else
-  EXPECT_FALSE(process_info.Is64Bit());
+  EXPECT_FALSE(is_64_bit);
 #endif
 
   // Test StartTime(). This program must have started at some time in the past.
   timeval start_time;
-  process_info.StartTime(&start_time);
+  ASSERT_TRUE(process_info.StartTime(&start_time));
+  EXPECT_FALSE(start_time.tv_sec == 0 && start_time.tv_usec == 0);
   time_t now;
   time(&now);
   EXPECT_LE(start_time.tv_sec, now);
@@ -147,7 +151,7 @@ void TestSelfProcess(const ProcessInfo& process_info) {
   EXPECT_EQ(getpid(), process_info.ProcessID());
   EXPECT_EQ(getppid(), process_info.ParentProcessID());
 
-  TestProcessClone(process_info);
+  TestProcessSelfOrClone(process_info);
 }
 
 TEST(ProcessInfo, Self) {
@@ -164,9 +168,6 @@ TEST(ProcessInfo, SelfTask) {
 }
 #endif
 
-// Applications cannot ptrace PID 1 on Android, which is required for Initialize
-// to succeed.
-#if !defined(OS_ANDROID)
 TEST(ProcessInfo, Pid1) {
   // PID 1 is expected to be init or the system’s equivalent. This tests reading
   // information about another process.
@@ -183,12 +184,8 @@ TEST(ProcessInfo, Pid1) {
   EXPECT_EQ(implicit_cast<gid_t>(0), process_info.SavedGroupID());
   EXPECT_FALSE(process_info.AllGroups().empty());
 }
-#endif
-
-#if defined(OS_LINUX) || defined(OS_ANDROID)
-TEST(ProcessInfo, ForkedSelf) {
-  ASSERT_EQ(0, prctl(PR_SET_DUMPABLE, 1, 0, 0, 0)) << ErrnoMessage("prctl");
 
+TEST(ProcessInfo, Forked) {
   pid_t pid = fork();
   if (pid == 0) {
     raise(SIGSTOP);
@@ -202,10 +199,9 @@ TEST(ProcessInfo, ForkedSelf) {
   EXPECT_EQ(pid, process_info.ProcessID());
   EXPECT_EQ(getpid(), process_info.ParentProcessID());
 
-  TestProcessClone(process_info);
+  TestProcessSelfOrClone(process_info);
   kill(pid, SIGKILL);
 }
-#endif
 
 }  // namespace
 }  // namespace test