Bug 1068410 - Remove procfs-searching workaround for credential-passing bug. r=ted
authorJed Davis <jld@mozilla.com>
Fri, 03 Oct 2014 14:55:03 -0700
changeset 208768 958697c700caf29a61dbd84e113646ce2e456f5d
parent 208767 5ac6d1958f310a962b5a2779b208495a80cd7b65
child 208769 afeff2d265bdf1d9b27284de44b6185082691f91
push id50000
push userjedavis@mozilla.com
push dateFri, 03 Oct 2014 21:55:22 +0000
treeherdermozilla-inbound@afeff2d265bd [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersted
bugs1068410
milestone35.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1068410 - Remove procfs-searching workaround for credential-passing bug. r=ted
toolkit/crashreporter/breakpad-patches/20-bug1068410-no-procfs-search.patch
toolkit/crashreporter/google-breakpad/src/client/linux/crash_generation/crash_generation_server.cc
new file mode 100644
--- /dev/null
+++ b/toolkit/crashreporter/breakpad-patches/20-bug1068410-no-procfs-search.patch
@@ -0,0 +1,151 @@
+commit 467c77076b232246970551cfba645d235381f588
+Author: Jed Davis <jld@mozilla.com>
+Date:   Wed Sep 17 11:34:38 2014 -0700
+
+    Bug 1068410 - Remove procfs-searching workaround for credential-passing bug.
+
+diff --git a/toolkit/crashreporter/google-breakpad/src/client/linux/crash_generation/crash_generation_server.cc b/toolkit/crashreporter/google-breakpad/src/client/linux/crash_generation/crash_generation_server.cc
+index f1e16b1..420e4b2 100644
+--- a/toolkit/crashreporter/google-breakpad/src/client/linux/crash_generation/crash_generation_server.cc
++++ b/toolkit/crashreporter/google-breakpad/src/client/linux/crash_generation/crash_generation_server.cc
+@@ -51,116 +51,6 @@
+ 
+ static const char kCommandQuit = 'x';
+ 
+-static bool
+-GetInodeForFileDescriptor(ino_t* inode_out, int fd)
+-{
+-  assert(inode_out);
+-
+-  struct stat buf;
+-  if (fstat(fd, &buf) < 0)
+-    return false;
+-
+-  if (!S_ISSOCK(buf.st_mode))
+-    return false;
+-
+-  *inode_out = buf.st_ino;
+-  return true;
+-}
+-
+-// expected prefix of the target of the /proc/self/fd/%d link for a socket
+-static const char kSocketLinkPrefix[] = "socket:[";
+-
+-// Parse a symlink in /proc/pid/fd/$x and return the inode number of the
+-// socket.
+-//   inode_out: (output) set to the inode number on success
+-//   path: e.g. /proc/1234/fd/5 (must be a UNIX domain socket descriptor)
+-static bool
+-GetInodeForProcPath(ino_t* inode_out, const char* path)
+-{
+-  assert(inode_out);
+-  assert(path);
+-
+-  char buf[PATH_MAX];
+-  if (!google_breakpad::SafeReadLink(path, buf)) {
+-    return false;
+-  }
+-
+-  if (0 != memcmp(kSocketLinkPrefix, buf, sizeof(kSocketLinkPrefix) - 1)) {
+-    return false;
+-  }
+-
+-  char* endptr;
+-  const uint64_t inode_ul =
+-      strtoull(buf + sizeof(kSocketLinkPrefix) - 1, &endptr, 10);
+-  if (*endptr != ']')
+-    return false;
+-
+-  if (inode_ul == ULLONG_MAX) {
+-    return false;
+-  }
+-
+-  *inode_out = inode_ul;
+-  return true;
+-}
+-
+-static bool
+-FindProcessHoldingSocket(pid_t* pid_out, ino_t socket_inode)
+-{
+-  assert(pid_out);
+-  bool already_found = false;
+-
+-  DIR* proc = opendir("/proc");
+-  if (!proc) {
+-    return false;
+-  }
+-
+-  std::vector<pid_t> pids;
+-
+-  struct dirent* dent;
+-  while ((dent = readdir(proc))) {
+-    char* endptr;
+-    const unsigned long int pid_ul = strtoul(dent->d_name, &endptr, 10);
+-    if (pid_ul == ULONG_MAX || '\0' != *endptr)
+-      continue;
+-    pids.push_back(pid_ul);
+-  }
+-  closedir(proc);
+-
+-  for (std::vector<pid_t>::const_iterator
+-       i = pids.begin(); i != pids.end(); ++i) {
+-    const pid_t current_pid = *i;
+-    char buf[PATH_MAX];
+-    snprintf(buf, sizeof(buf), "/proc/%d/fd", current_pid);
+-    DIR* fd = opendir(buf);
+-    if (!fd)
+-      continue;
+-
+-    while ((dent = readdir(fd))) {
+-      if (snprintf(buf, sizeof(buf), "/proc/%d/fd/%s", current_pid,
+-                   dent->d_name) >= static_cast<int>(sizeof(buf))) {
+-        continue;
+-      }
+-
+-      ino_t fd_inode;
+-      if (GetInodeForProcPath(&fd_inode, buf)
+-          && fd_inode == socket_inode) {
+-        if (already_found) {
+-          closedir(fd);
+-          return false;
+-        }
+-
+-        already_found = true;
+-        *pid_out = current_pid;
+-        break;
+-      }
+-    }
+-
+-    closedir(fd);
+-  }
+-
+-  return already_found;
+-}
+-
+ namespace google_breakpad {
+ 
+ CrashGenerationServer::CrashGenerationServer(
+@@ -367,23 +257,6 @@ CrashGenerationServer::ClientEvent(short revents)
+     return true;
+   }
+ 
+-  // Kernel bug workaround (broken in 2.6.30 at least):
+-  // The kernel doesn't translate PIDs in SCM_CREDENTIALS across PID
+-  // namespaces. Thus |crashing_pid| might be garbage from our point of view.
+-  // In the future we can remove this workaround, but we have to wait a couple
+-  // of years to be sure that it's worked its way out into the world.
+-
+-  ino_t inode_number;
+-  if (!GetInodeForFileDescriptor(&inode_number, signal_fd)) {
+-    HANDLE_EINTR(close(signal_fd));
+-    return true;
+-  }
+-
+-  if (!FindProcessHoldingSocket(&crashing_pid, inode_number - 1)) {
+-    HANDLE_EINTR(close(signal_fd));
+-    return true;
+-  }
+-
+   string minidump_filename;
+   if (!MakeMinidumpFilename(minidump_filename))
+     return true;
--- a/toolkit/crashreporter/google-breakpad/src/client/linux/crash_generation/crash_generation_server.cc
+++ b/toolkit/crashreporter/google-breakpad/src/client/linux/crash_generation/crash_generation_server.cc
@@ -46,126 +46,16 @@
 #include "client/linux/handler/exception_handler.h"
 #include "client/linux/minidump_writer/minidump_writer.h"
 #include "common/linux/eintr_wrapper.h"
 #include "common/linux/guid_creator.h"
 #include "common/linux/safe_readlink.h"
 
 static const char kCommandQuit = 'x';
 
-static bool
-GetInodeForFileDescriptor(ino_t* inode_out, int fd)
-{
-  assert(inode_out);
-
-  struct stat buf;
-  if (fstat(fd, &buf) < 0)
-    return false;
-
-  if (!S_ISSOCK(buf.st_mode))
-    return false;
-
-  *inode_out = buf.st_ino;
-  return true;
-}
-
-// expected prefix of the target of the /proc/self/fd/%d link for a socket
-static const char kSocketLinkPrefix[] = "socket:[";
-
-// Parse a symlink in /proc/pid/fd/$x and return the inode number of the
-// socket.
-//   inode_out: (output) set to the inode number on success
-//   path: e.g. /proc/1234/fd/5 (must be a UNIX domain socket descriptor)
-static bool
-GetInodeForProcPath(ino_t* inode_out, const char* path)
-{
-  assert(inode_out);
-  assert(path);
-
-  char buf[PATH_MAX];
-  if (!google_breakpad::SafeReadLink(path, buf)) {
-    return false;
-  }
-
-  if (0 != memcmp(kSocketLinkPrefix, buf, sizeof(kSocketLinkPrefix) - 1)) {
-    return false;
-  }
-
-  char* endptr;
-  const uint64_t inode_ul =
-      strtoull(buf + sizeof(kSocketLinkPrefix) - 1, &endptr, 10);
-  if (*endptr != ']')
-    return false;
-
-  if (inode_ul == ULLONG_MAX) {
-    return false;
-  }
-
-  *inode_out = inode_ul;
-  return true;
-}
-
-static bool
-FindProcessHoldingSocket(pid_t* pid_out, ino_t socket_inode)
-{
-  assert(pid_out);
-  bool already_found = false;
-
-  DIR* proc = opendir("/proc");
-  if (!proc) {
-    return false;
-  }
-
-  std::vector<pid_t> pids;
-
-  struct dirent* dent;
-  while ((dent = readdir(proc))) {
-    char* endptr;
-    const unsigned long int pid_ul = strtoul(dent->d_name, &endptr, 10);
-    if (pid_ul == ULONG_MAX || '\0' != *endptr)
-      continue;
-    pids.push_back(pid_ul);
-  }
-  closedir(proc);
-
-  for (std::vector<pid_t>::const_iterator
-       i = pids.begin(); i != pids.end(); ++i) {
-    const pid_t current_pid = *i;
-    char buf[PATH_MAX];
-    snprintf(buf, sizeof(buf), "/proc/%d/fd", current_pid);
-    DIR* fd = opendir(buf);
-    if (!fd)
-      continue;
-
-    while ((dent = readdir(fd))) {
-      if (snprintf(buf, sizeof(buf), "/proc/%d/fd/%s", current_pid,
-                   dent->d_name) >= static_cast<int>(sizeof(buf))) {
-        continue;
-      }
-
-      ino_t fd_inode;
-      if (GetInodeForProcPath(&fd_inode, buf)
-          && fd_inode == socket_inode) {
-        if (already_found) {
-          closedir(fd);
-          return false;
-        }
-
-        already_found = true;
-        *pid_out = current_pid;
-        break;
-      }
-    }
-
-    closedir(fd);
-  }
-
-  return already_found;
-}
-
 namespace google_breakpad {
 
 CrashGenerationServer::CrashGenerationServer(
   const int listen_fd,
   OnClientDumpRequestCallback dump_callback,
   void* dump_context,
   OnClientExitingCallback exit_callback,
   void* exit_context,
@@ -362,33 +252,16 @@ CrashGenerationServer::ClientEvent(short
   }
 
   if (crashing_pid == -1 || signal_fd == -1) {
     if (signal_fd)
       HANDLE_EINTR(close(signal_fd));
     return true;
   }
 
-  // Kernel bug workaround (broken in 2.6.30 at least):
-  // The kernel doesn't translate PIDs in SCM_CREDENTIALS across PID
-  // namespaces. Thus |crashing_pid| might be garbage from our point of view.
-  // In the future we can remove this workaround, but we have to wait a couple
-  // of years to be sure that it's worked its way out into the world.
-
-  ino_t inode_number;
-  if (!GetInodeForFileDescriptor(&inode_number, signal_fd)) {
-    HANDLE_EINTR(close(signal_fd));
-    return true;
-  }
-
-  if (!FindProcessHoldingSocket(&crashing_pid, inode_number - 1)) {
-    HANDLE_EINTR(close(signal_fd));
-    return true;
-  }
-
   string minidump_filename;
   if (!MakeMinidumpFilename(minidump_filename))
     return true;
 
   if (!google_breakpad::WriteMinidump(minidump_filename.c_str(),
                                       crashing_pid, crash_context,
                                       kCrashContextSize)) {
     HANDLE_EINTR(close(signal_fd));