Non-SFI mode: Use dummy PID for NaCl's IPC channel on Linux platform.

ipc/ipc_channel_posix.cc

Index: ipc/ipc_channel_posix.cc
diff --git a/ipc/ipc_channel_posix.cc b/ipc/ipc_channel_posix.cc
index 79fc7f203a6cba777a7305dabf838109f8653f83..fa91e520bfcfde698fc248c8f16a9c38e6fd9ca2 100644
--- a/ipc/ipc_channel_posix.cc
+++ b/ipc/ipc_channel_posix.cc
@@ -176,9 +176,9 @@ void Channel::NotifyProcessForkedForTesting() {
 
 //------------------------------------------------------------------------------
 
-#if defined(OS_LINUX)
+#if defined(OS_LINUX) || defined(OS_NACL_NONSFI)
 int ChannelPosix::global_pid_ = 0;
-#endif  // OS_LINUX
+#endif  // defined(OS_LINUX) || defined(OS_NACL_NONSFI)
 
 ChannelPosix::ChannelPosix(const IPC::ChannelHandle& channel_handle,
                            Mode mode, Listener* listener)
@@ -645,12 +645,12 @@ bool ChannelPosix::IsNamedServerInitialized(
   return base::PathExists(base::FilePath(channel_id));
 }
 
-#if defined(OS_LINUX)
+#if defined(OS_LINUX) || defined(OS_NACL_NONSFI)
 // static
 void ChannelPosix::SetGlobalPid(int pid) {
   global_pid_ = pid;
 }
-#endif  // OS_LINUX
+#endif  // defined(OS_LINUX) || defined(OS_NACL_NONSFI)
 
 // Called by libevent when we can read from the pipe without blocking.
 void ChannelPosix::OnFileCanReadWithoutBlocking(int fd) {
@@ -771,14 +771,15 @@ void ChannelPosix::ClosePipeOnError() {
 }
 
 int ChannelPosix::GetHelloMessageProcId() const {
-  int pid = base::GetCurrentProcId();
-#if defined(OS_LINUX)
-  // Our process may be in a sandbox with a separate PID namespace.
-  if (global_pid_) {
-    pid = global_pid_;
-  }
+#if defined(OS_LINUX) || defined(OS_NACL_NONSFI)
+  // Our process may be in a sandbox with a separate PID namespace in Linux.
+  // In nacl_helper_nonsfi, getpid() invoked by GetCurrentProcId() is not
+  // allowed and would cause a SIGSYS crashing because of the seccomp sandbox.
+  // |global_pid_| should be initialized in advance in the process.
+  if (global_pid_)
+    return global_pid_;

[mdempsky, 2014/11/07 03:05:34]

Will global_pid_ ever be set to anything other than -1 for nonsfi?  Could we
simplify things by changing this to:

#if defined(OS_NACL_NONSFI)
  return -1;
#else
#if defined(OS_LINUX)
  if (global_pid_)
    return global_pid_;
#endif
  return base::GetCurrentProcId();
#endif

and then you don't even need SetGlobalPid() for nonsfi?

I might also suggest using 0 instead of -1 for the nonsfi PID (because Linux
already uses 0 to indicate an unrepresentable PID), but not if it ends up
introducing any other complications.

[hidehiko, 2014/11/07 05:43:21]

This is both for SFI (under OS_LINUX) and Non-SFI (under OS_NACL_NONSFI), so it
may be better to be shared for consistency. WDYT?
Also, 0 does not work. It is used as "Not yet initialized" in IPC. Actually,
we'd hit a real issue at IPC::ChannelProxy::Context::OnAddFilter, if we use 0
instead.

[Mark Seaborn, 2014/12/03 01:54:00]

I agree with Matthew's comment about doing:
#if defined(OS_NACL_NONSFI)
  return -1;
#...

Furthermore, why not do the following?
#if defined(OS_NACL_NONSFI) || defined(OS_LINUX)
  return -1;
#...

AFAICT, the PID is not required on Linux.  My understanding is that the PID was
originally required for Windows, and this usage leaked across onto other
platforms.

[hidehiko, 2014/12/03 17:03:19]

Ok, done, in mdempsky@'s way.
My focus is Non-SFI. Under OS_LINUX, some other code, like zygote, uses the
feature, which is out of our focus. Also, I have no way to confirm if it is ok
to do, or there is some problems. So, I defer to mdempsky@ and jln@ for the
decision. If it could be safely removable, please let me know. Then, I'd remove
global_pid_ itself.

 #endif
-  return pid;
+  return base::GetCurrentProcId();
 }
 
 void ChannelPosix::QueueHelloMessage() {
@@ -1111,11 +1112,11 @@ bool Channel::IsNamedServerInitialized(
   return ChannelPosix::IsNamedServerInitialized(channel_id);
 }
 
-#if defined(OS_LINUX)
+#if defined(OS_LINUX) || defined(OS_NACL_NONSFI)
 // static
 void Channel::SetGlobalPid(int pid) {
   ChannelPosix::SetGlobalPid(pid);
 }
-#endif  // OS_LINUX
+#endif  // defined(OS_LINUX) || defined(OS_NACL_NONSFI)
 
 }  // namespace IPC