Always search TIDs for the crashing processes.

chrome/browser/crash_handler_host_linux.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/crash_handler_host_linux.h"
 
 #include <stdint.h>
 #include <stdlib.h>
 #include <sys/socket.h>
 #include <sys/syscall.h>
@@ skipping 203 matching lines @@
   if (crashing_pid == -1 || partner_fd == -1 || signal_fd == -1) {
     LOG(ERROR) << "Death signal message didn't contain all expected control"
                << " messages";
     if (partner_fd >= 0)
       HANDLE_EINTR(close(partner_fd));
     if (signal_fd >= 0)
       HANDLE_EINTR(close(signal_fd));
     return;
   }
 
   // Kernel bug workaround (broken in 2.6.30 at least):

[Lei Zhang, 2011/06/20 21:37:10]

Can we update this comment block to:

// Kernel bug workaround (broken in 2.6.30 and 2.6.32, working in 2.6.38)
// 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.
// TODO(thestig) Remove the workaround when Ubuntu Lucid is deprecated.

[kmixter1, 2011/06/20 23:49:17]

Done.

   // 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.
 
   // The crashing process closes its copy of the signal_fd immediately after
   // calling sendmsg(). We can thus not reliably look for with with
   // FindProcessHoldingSocket(). But by necessity, it has to keep the
   // partner_fd open until the crashdump is complete.
   uint64_t inode_number;
   if (!base::FileDescriptorGetInode(&inode_number, partner_fd)) {
     LOG(WARNING) << "Failed to get inode number for passed socket";
     HANDLE_EINTR(close(partner_fd));
     HANDLE_EINTR(close(signal_fd));
     return;
   }
   HANDLE_EINTR(close(partner_fd));
 
   pid_t actual_crashing_pid = -1;
   if (!base::FindProcessHoldingSocket(&actual_crashing_pid, inode_number)) {
     LOG(WARNING) << "Failed to find process holding other end of crash reply "
                     "socket";
     HANDLE_EINTR(close(signal_fd));
     return;
   }
 
-  if (actual_crashing_pid != crashing_pid) {
-    crashing_pid = actual_crashing_pid;
+  crashing_pid = actual_crashing_pid;
 
-    // The crashing TID set inside the compromised context via sys_gettid()
-    // in ExceptionHandler::HandleSignal is also wrong and needs to be
-    // translated.
-    //
-    // We expect the crashing thread to be in sys_read(), waiting for use to
-    // write to |signal_fd|. Most newer kernels where we have the different pid
-    // namespaces also have /proc/[pid]/syscall, so we can look through
-    // |actual_crashing_pid|'s thread group and find the thread that's in the
-    // read syscall with the right arguments.
+  // The crashing TID set inside the compromised context via sys_gettid()
+  // in ExceptionHandler::HandleSignal is also wrong and needs to be

[Lei Zhang, 2011/06/20 21:37:10]

s/is also/might be/

[kmixter1, 2011/06/20 23:49:17]

Done.

+  // translated.
+  //
+  // We expect the crashing thread to be in sys_read(), waiting for use to
+  // write to |signal_fd|. Most newer kernels where we have the different pid
+  // namespaces also have /proc/[pid]/syscall, so we can look through
+  // |actual_crashing_pid|'s thread group and find the thread that's in the
+  // read syscall with the right arguments.
 
-    std::string expected_syscall_data;
-    // /proc/[pid]/syscall is formatted as follows:
-    // syscall_number arg1 ... arg6 sp pc
-    // but we just check syscall_number through arg3.
-    base::StringAppendF(&expected_syscall_data, "%d 0x%x %p 0x1 ",
-                        SYS_read, tid_fd, tid_buf_addr);
-    pid_t crashing_tid =
-        base::FindThreadIDWithSyscall(crashing_pid, expected_syscall_data);
-    if (crashing_tid == -1) {
-      // We didn't find the thread we want. Maybe it didn't reach sys_read()
-      // yet, or the kernel doesn't support /proc/[pid]/syscall or the thread
-      // went away.  We'll just take a guess here and assume the crashing
-      // thread is the thread group leader.
-      crashing_tid = crashing_pid;
-    }
+  std::string expected_syscall_data;
+  // /proc/[pid]/syscall is formatted as follows:
+  // syscall_number arg1 ... arg6 sp pc
+  // but we just check syscall_number through arg3.
+  base::StringAppendF(&expected_syscall_data, "%d 0x%x %p 0x1 ",
+                      SYS_read, tid_fd, tid_buf_addr);
+  bool syscall_supported = false;
+  pid_t crashing_tid =
+      base::FindThreadIDWithSyscall(crashing_pid,
+                                    expected_syscall_data,
+                                    &syscall_supported);
+  if (crashing_tid == -1 && syscall_supported) {
+    // We didn't find the thread we want. Maybe it didn't reach
+    // sys_read() yet or the thread went away.  We'll just take a
+    // guess here and assume the crashing thread is the thread group
+    // leader.  If procfs syscall is not supported by the kernel, then
+    // we assume the kernel also does not support TID namespacing and
+    // trust the TID passed by the crashing process.
+    crashing_tid = crashing_pid;
+  }
 
-    ExceptionHandler::CrashContext* bad_context =
-        reinterpret_cast<ExceptionHandler::CrashContext*>(crash_context);
-    bad_context->tid = crashing_tid;
-  }
+  ExceptionHandler::CrashContext* bad_context =
+      reinterpret_cast<ExceptionHandler::CrashContext*>(crash_context);
+  bad_context->tid = crashing_tid;
 
   // Sanitize the string data a bit more
   guid[kGuidSize] = crash_url[kMaxActiveURLSize] = distro[kDistroSize] = 0;
 
   // Freed in CrashDumpTask();
   BreakpadInfo* info = new BreakpadInfo;
 
   info->process_type_length = process_type_.length();
   char* process_type_str = new char[info->process_type_length + 1];
   process_type_.copy(process_type_str, info->process_type_length);
@@ skipping 146 matching lines @@
 }
 
 void PpapiCrashHandlerHostLinux::SetProcessType() {
   process_type_ = "ppapi";
 }
 
 // static
 PpapiCrashHandlerHostLinux* PpapiCrashHandlerHostLinux::GetInstance() {
   return Singleton<PpapiCrashHandlerHostLinux>::get();
 }