Fix IPC OnChannelConnected() to send correct PID on Linux/CrOS

Sandboxed renderers on Linux/CrOS are in a PID namespace, so they don't know
their own global PID.  Thus the PID sent in the IPC channel Hello message
contains an unexpected value, which is used in the OnChannelConnected()
callback into chrome.

This causes problems like the Task Manager not showing any data for FPS,
JavaScript memory and image cache memory.  The task manager is attempting to
use the PID/process handle from BrowserMessageFilter, which got it from
IPC::Channel::Listener::OnChannelConnected(), and it doesn't match the
global PID of each renderer.

BUG=70179
TEST=manual, open a few tabs, then open task manager, right-click to turn on JavaScript memory and image memory. Verify there are non-zero values for FPS, JavaScript memory, image cache memory

Review URL: http://codereview.chromium.org/7661004

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@97481 0039d316-1c4b-4281-b951-d872f2087c98

7 files changed, 113 insertions, 13 deletions

diff --git a/ipc/ipc_channel.h b/ipc/ipc_channel.h
index cd57b4b..862c603 100644
--- a/ipc/ipc_channel.h
+++ b/ipc/ipc_channel.h
@@ -171,6 +171,19 @@ class IPC_EXPORT Channel : public Message::Sender {
   void ResetToAcceptingConnectionState();
 #endif  // defined(OS_POSIX) && !defined(OS_NACL)
 
+#if defined(OS_LINUX)
+  // Configures the channel to defer OnChannelConnected() until we know the
+  // global PID of the peer.  On Linux, with sandboxed renderers, the browser
+  // cannot use the process id sent by the renderer in the hello message,
+  // because the renderer is in its own private PID namespace.  With these
+  // renderers we need to defer our call to OnChannelConnected(peer_pid) until
+  // we know the global PID.
+  void SetNeedsOverridePeerPid();
+
+  // Overrides the peer PID and calls OnChannelConnected() if necessary.
+  void OverridePeerPid(int32 peer_pid);
+#endif  // defined(OS_LINUX)
+
   // Returns true if a named server channel is initialized on the given channel
   // ID. Even if true, the server may have already accepted a connection.
   static bool IsNamedServerInitialized(const std::string& channel_id);
diff --git a/ipc/ipc_channel_posix.cc b/ipc/ipc_channel_posix.cc
index fe60691..27231d1 100644
--- a/ipc/ipc_channel_posix.cc
+++ b/ipc/ipc_channel_posix.cc
@@ -309,7 +309,9 @@ Channel::ChannelImpl::ChannelImpl(const IPC::ChannelHandle& channel_handle,
 #endif  // IPC_USES_READWRITE
       pipe_name_(channel_handle.name),
       listener_(listener),
-      must_unlink_(false) {
+      must_unlink_(false),
+      needs_override_peer_pid_(false),
+      override_peer_pid_(0) {
   memset(input_buf_, 0, sizeof(input_buf_));
   memset(input_cmsg_buf_, 0, sizeof(input_cmsg_buf_));
   if (!CreatePipe(channel_handle)) {
@@ -727,7 +729,14 @@ bool Channel::ChannelImpl::ProcessIncomingMessages() {
             CHECK(descriptor.auto_close);
           }
 #endif  // IPC_USES_READWRITE
-          listener_->OnChannelConnected(pid);
+          if (needs_override_peer_pid_) {
+            // If we already have the peer PID, use it.  Otherwise we'll call
+            // OnChannelConnected() in OverridePeerPid() below.
+            if (override_peer_pid_ != 0)
+              listener_->OnChannelConnected(override_peer_pid_);
+          } else {
+            listener_->OnChannelConnected(pid);
+          }
         } else {
           listener_->OnMessageReceived(m);
         }
@@ -993,6 +1002,27 @@ void Channel::ChannelImpl::ResetToAcceptingConnectionState() {
   input_overflow_fds_.clear();
 }
 
+#if defined(OS_LINUX)
+void Channel::ChannelImpl::SetNeedsOverridePeerPid() {
+  needs_override_peer_pid_ = true;
+}
+
+void Channel::ChannelImpl::OverridePeerPid(int32 peer_pid) {
+  DCHECK(needs_override_peer_pid_);
+  override_peer_pid_ = peer_pid;
+
+  // The browser learns the global PID of the renderers on the UI thread, and
+  // must post the data to the IO thread for us to use it here.  Therefore
+  // there is a race between the IPC channel processing the hello message
+  // and this function being called.  If fd_pipe_ != -1 then we've already
+  // received the hello message and we skipped OnChannelConnected() above,
+  // so call it here.
+  if (fd_pipe_ != -1) {
+    listener_->OnChannelConnected(peer_pid);
+  }
+}
+#endif  // defined(OS_LINUX)
+
 // static
 bool Channel::ChannelImpl::IsNamedServerInitialized(
     const std::string& channel_id) {
@@ -1208,6 +1238,16 @@ void Channel::ResetToAcceptingConnectionState() {
   channel_impl_->ResetToAcceptingConnectionState();
 }
 
+#if defined(OS_LINUX)
+void Channel::SetNeedsOverridePeerPid() {
+  channel_impl_->SetNeedsOverridePeerPid();
+}
+
+void Channel::OverridePeerPid(int32 peer_pid) {
+  channel_impl_->OverridePeerPid(peer_pid);
+}
+#endif  // defined(OS_LINUX)
+
 // static
 bool Channel::IsNamedServerInitialized(const std::string& channel_id) {
   return ChannelImpl::IsNamedServerInitialized(channel_id);
diff --git a/ipc/ipc_channel_posix.h b/ipc/ipc_channel_posix.h
index b66b1fc..c1aaa2b 100644
--- a/ipc/ipc_channel_posix.h
+++ b/ipc/ipc_channel_posix.h
@@ -62,6 +62,10 @@ class Channel::ChannelImpl : public MessageLoopForIO::Watcher {
   bool HasAcceptedConnection() const;
   bool GetClientEuid(uid_t* client_euid) const;
   void ResetToAcceptingConnectionState();
+#if defined(OS_LINUX)
+  void SetNeedsOverridePeerPid();
+  void OverridePeerPid(int32 peer_pid);
+#endif  // defined(OS_LINUX)
   static bool IsNamedServerInitialized(const std::string& channel_id);
 
  private:
@@ -147,6 +151,10 @@ class Channel::ChannelImpl : public MessageLoopForIO::Watcher {
   // True if we are responsible for unlinking the unix domain socket file.
   bool must_unlink_;
 
+  // See IPC::Channel::SetNeedsOverridePeerPid() header comment for details.
+  bool needs_override_peer_pid_;
+  int32 override_peer_pid_;
+
   DISALLOW_IMPLICIT_CONSTRUCTORS(ChannelImpl);
 };
 
diff --git a/ipc/ipc_channel_proxy.cc b/ipc/ipc_channel_proxy.cc
index dc990c2..db801fd 100644
--- a/ipc/ipc_channel_proxy.cc
+++ b/ipc/ipc_channel_proxy.cc
@@ -71,10 +71,15 @@ ChannelProxy::Context::~Context() {
 }
 
 void ChannelProxy::Context::CreateChannel(const IPC::ChannelHandle& handle,
-                                          const Channel::Mode& mode) {
+                                          const Channel::Mode& mode,
+                                          bool needs_override_peer_pid) {
   DCHECK(channel_.get() == NULL);
   channel_id_ = handle.name;
   channel_.reset(new Channel(handle, mode, this));
+#if defined(OS_LINUX)
+  if (needs_override_peer_pid)
+    channel_->SetNeedsOverridePeerPid();
+#endif
 }
 
 bool ChannelProxy::Context::TryFilters(const Message& message) {
@@ -226,6 +231,14 @@ void ChannelProxy::Context::OnRemoveFilter(MessageFilter* filter) {
   NOTREACHED() << "filter to be removed not found";
 }
 
+#if defined(OS_LINUX)
+// Called on the IPC::Channel thread
+void ChannelProxy::Context::OnOverridePeerPid(int32 peer_pid) {
+  if (channel_.get())
+    channel_->OverridePeerPid(peer_pid);
+}
+#endif  // defined(OS_LINUX)
+
 // Called on the listener's thread
 void ChannelProxy::Context::AddFilter(MessageFilter* filter) {
   base::AutoLock auto_lock(pending_filters_lock_);
@@ -282,20 +295,23 @@ void ChannelProxy::Context::OnDispatchError() {
 ChannelProxy::ChannelProxy(const IPC::ChannelHandle& channel_handle,
                            Channel::Mode mode,
                            Channel::Listener* listener,
-                           base::MessageLoopProxy* ipc_thread)
+                           base::MessageLoopProxy* ipc_thread,
+                           bool needs_override_peer_pid)
     : context_(new Context(listener, ipc_thread)),
       outgoing_message_filter_(NULL) {
-  Init(channel_handle, mode, ipc_thread, true);
+  Init(channel_handle, mode, ipc_thread, true, needs_override_peer_pid);
 }
 
 ChannelProxy::ChannelProxy(const IPC::ChannelHandle& channel_handle,
                            Channel::Mode mode,
                            base::MessageLoopProxy* ipc_thread,
+                           bool needs_override_peer_pid,
                            Context* context,
                            bool create_pipe_now)
     : context_(context),
       outgoing_message_filter_(NULL) {
-  Init(channel_handle, mode, ipc_thread, create_pipe_now);
+  Init(channel_handle, mode, ipc_thread, create_pipe_now,
+       needs_override_peer_pid);
 }
 
 ChannelProxy::~ChannelProxy() {
@@ -305,7 +321,8 @@ ChannelProxy::~ChannelProxy() {
 void ChannelProxy::Init(const IPC::ChannelHandle& channel_handle,
                         Channel::Mode mode,
                         base::MessageLoopProxy* ipc_thread_loop,
-                        bool create_pipe_now) {
+                        bool create_pipe_now,
+                        bool needs_override_peer_pid) {
 #if defined(OS_POSIX)
   // When we are creating a server on POSIX, we need its file descriptor
   // to be created immediately so that it can be accessed and passed
@@ -321,10 +338,11 @@ void ChannelProxy::Init(const IPC::ChannelHandle& channel_handle,
     // low-level pipe so that the client can connect.  Without creating
     // the pipe immediately, it is possible for a listener to attempt
     // to connect and get an error since the pipe doesn't exist yet.
-    context_->CreateChannel(channel_handle, mode);
+    context_->CreateChannel(channel_handle, mode, needs_override_peer_pid);
   } else {
     context_->ipc_message_loop()->PostTask(FROM_HERE, NewRunnableMethod(
-        context_.get(), &Context::CreateChannel, channel_handle, mode));
+        context_.get(), &Context::CreateChannel, channel_handle, mode,
+        needs_override_peer_pid));
   }
 
   // complete initialization on the background thread
@@ -392,6 +410,13 @@ bool ChannelProxy::GetClientEuid(uid_t* client_euid) const {
 }
 #endif
 
+#if defined(OS_LINUX)
+void ChannelProxy::OverridePeerPid(int32 peer_pid) {
+  context_->ipc_message_loop()->PostTask(FROM_HERE, NewRunnableMethod(
+      context_.get(), &Context::OnOverridePeerPid, peer_pid));
+}
+#endif  // defined(OS_LINUX)
+
 //-----------------------------------------------------------------------------
 
 }  // namespace IPC
diff --git a/ipc/ipc_channel_proxy.h b/ipc/ipc_channel_proxy.h
index 792e3d6..d047b78 100644
--- a/ipc/ipc_channel_proxy.h
+++ b/ipc/ipc_channel_proxy.h
@@ -118,7 +118,8 @@ class IPC_EXPORT ChannelProxy : public Message::Sender {
   ChannelProxy(const IPC::ChannelHandle& channel_handle,
                Channel::Mode mode,
                Channel::Listener* listener,
-               base::MessageLoopProxy* ipc_thread_loop);
+               base::MessageLoopProxy* ipc_thread_loop,
+               bool needs_override_peer_pid);
 
   virtual ~ChannelProxy();
 
@@ -161,6 +162,11 @@ class IPC_EXPORT ChannelProxy : public Message::Sender {
   bool GetClientEuid(uid_t* client_euid) const;
 #endif  // defined(OS_POSIX)
 
+#if defined(OS_LINUX)
+  // Calls through to the underlying channel's method.
+  void OverridePeerPid(int32 peer_pid);
+#endif  // defined(OS_LINUX)
+
  protected:
   class Context;
   // A subclass uses this constructor if it needs to add more information
@@ -169,6 +175,7 @@ class IPC_EXPORT ChannelProxy : public Message::Sender {
   ChannelProxy(const IPC::ChannelHandle& channel_handle,
                Channel::Mode mode,
                base::MessageLoopProxy* ipc_thread_loop,
+               bool needs_override_peer_pid,
                Context* context,
                bool create_pipe_now);
 
@@ -217,12 +224,16 @@ class IPC_EXPORT ChannelProxy : public Message::Sender {
 
     // Create the Channel
     void CreateChannel(const IPC::ChannelHandle& channel_handle,
-                       const Channel::Mode& mode);
+                       const Channel::Mode& mode,
+                       bool needs_override_peer_pid);
 
     // Methods called on the IO thread.
     void OnSendMessage(Message* message_ptr);
     void OnAddFilter();
     void OnRemoveFilter(MessageFilter* filter);
+#if defined(OS_LINUX)
+    void OnOverridePeerPid(int32 peer_pid);
+#endif
 
     // Methods called on the listener thread.
     void AddFilter(MessageFilter* filter);
@@ -257,7 +268,8 @@ class IPC_EXPORT ChannelProxy : public Message::Sender {
   friend class SendTask;
 
   void Init(const IPC::ChannelHandle& channel_handle, Channel::Mode mode,
-            base::MessageLoopProxy* ipc_thread_loop, bool create_pipe_now);
+            base::MessageLoopProxy* ipc_thread_loop, bool create_pipe_now,
+            bool needs_override_peer_pid);
 
   // By maintaining this indirection (ref-counted) to our internal state, we
   // can safely be destroyed while the background thread continues to do stuff
diff --git a/ipc/ipc_sync_channel.cc b/ipc/ipc_sync_channel.cc
index c135ef3..9cb6454 100644
--- a/ipc/ipc_sync_channel.cc
+++ b/ipc/ipc_sync_channel.cc
@@ -377,6 +377,7 @@ SyncChannel::SyncChannel(
     WaitableEvent* shutdown_event)
     : ChannelProxy(
           channel_handle, mode, ipc_message_loop,
+          false /* needs_override_peer_pid */,
           new SyncContext(listener, ipc_message_loop, shutdown_event),
           create_pipe_now),
       sync_messages_with_no_timeout_allowed_(true) {
diff --git a/ipc/ipc_tests.cc b/ipc/ipc_tests.cc
index 2727681..7e808a7 100644
--- a/ipc/ipc_tests.cc
+++ b/ipc/ipc_tests.cc
@@ -251,7 +251,8 @@ TEST_F(IPCChannelTest, ChannelProxyTest) {
   {
     // setup IPC channel proxy
     IPC::ChannelProxy chan(kTestClientChannel, IPC::Channel::MODE_SERVER,
-                           &channel_listener, thread.message_loop_proxy());
+                           &channel_listener, thread.message_loop_proxy(),
+                           false /* needs_override_peer_pid */);
 
     channel_listener.Init(&chan);