Skip to content

Commit

Permalink
Fix IPC OnChannelConnected() to send correct PID on Linux/CrOS
Browse files Browse the repository at this point in the history
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/7778031

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@99040 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
jamescook@chromium.org committed Aug 31, 2011
1 parent 943b861 commit e1d67a8
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 6 deletions.
21 changes: 17 additions & 4 deletions content/browser/zygote_main_linux.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include "base/command_line.h"
#include "base/eintr_wrapper.h"
#include "base/file_path.h"
#include "base/file_util.h"
#include "base/global_descriptors_posix.h"
#include "base/hash_tables.h"
#include "base/linux_util.h"
Expand All @@ -41,6 +42,7 @@
#include "content/common/zygote_fork_delegate_linux.h"
#include "skia/ext/SkFontHost_fontconfig_control.h"
#include "unicode/timezone.h"
#include "ipc/ipc_channel.h"
#include "ipc/ipc_switches.h"

#if defined(OS_LINUX)
Expand Down Expand Up @@ -290,15 +292,24 @@ class Zygote {
} else if (pid == 0) {
// In the child process.
close(pipe_fds[1]);
char buffer[1];
base::ProcessId real_pid;
// Wait until the parent process has discovered our PID. We
// should not fork any child processes (which the seccomp
// sandbox does) until then, because that can interfere with the
// parent's discovery of our PID.
if (HANDLE_EINTR(read(pipe_fds[0], buffer, 1)) != 1 ||
buffer[0] != 'x') {
if (!file_util::ReadFromFD(pipe_fds[0],
reinterpret_cast<char*>(&real_pid),
sizeof(real_pid))) {
LOG(FATAL) << "Failed to synchronise with parent zygote process";
}
if (real_pid <= 0) {
LOG(FATAL) << "Invalid pid from parent zygote";
}
#if defined(OS_LINUX)
// Sandboxed processes need to send the global, non-namespaced PID when
// setting up an IPC channel to their parent.
IPC::Channel::SetGlobalPid(real_pid);
#endif
close(pipe_fds[0]);
close(dummy_fd);
return 0;
Expand Down Expand Up @@ -341,7 +352,9 @@ class Zygote {
goto error;
}
} else {
if (HANDLE_EINTR(write(pipe_fds[1], "x", 1)) != 1) {
int written =
HANDLE_EINTR(write(pipe_fds[1], &real_pid, sizeof(real_pid)));
if (written != sizeof(real_pid)) {
LOG(ERROR) << "Failed to synchronise with child process";
goto error;
}
Expand Down
7 changes: 7 additions & 0 deletions ipc/ipc_channel.h
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,13 @@ class IPC_EXPORT Channel : public Message::Sender {
// ID. Even if true, the server may have already accepted a connection.
static bool IsNamedServerInitialized(const std::string& channel_id);

#if defined(OS_LINUX)
// Sandboxed processes live in a PID namespace, so when sending the IPC hello
// message from client to server we need to send the PID from the global
// PID namespace.
static void SetGlobalPid(int pid);
#endif

protected:
// Used in Chrome by the TestSink to provide a dummy channel implementation
// for testing. TestSink overrides the "interesting" functions in Channel so
Expand Down
32 changes: 30 additions & 2 deletions ipc/ipc_channel_posix.cc
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,10 @@ bool SocketWriteErrorIsRecoverable() {
} // namespace
//------------------------------------------------------------------------------

#if defined(OS_LINUX)
int Channel::ChannelImpl::global_pid_ = 0;
#endif // OS_LINUX

Channel::ChannelImpl::ChannelImpl(const IPC::ChannelHandle& channel_handle,
Mode mode, Listener* listener)
: mode_(mode),
Expand Down Expand Up @@ -997,6 +1001,13 @@ bool Channel::ChannelImpl::IsNamedServerInitialized(
return file_util::PathExists(FilePath(channel_id));
}

#if defined(OS_LINUX)
// static
void Channel::ChannelImpl::SetGlobalPid(int pid) {
global_pid_ = pid;
}
#endif // OS_LINUX

// Called by libevent when we can read from the pipe without blocking.
void Channel::ChannelImpl::OnFileCanReadWithoutBlocking(int fd) {
bool send_server_hello_msg = false;
Expand Down Expand Up @@ -1109,13 +1120,23 @@ void Channel::ChannelImpl::ClosePipeOnError() {
}
}

int Channel::ChannelImpl::GetHelloMessageProcId() {
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_;
}
#endif
return pid;
}

void Channel::ChannelImpl::QueueHelloMessage() {
// Create the Hello message
scoped_ptr<Message> msg(new Message(MSG_ROUTING_NONE,
HELLO_MESSAGE_TYPE,
IPC::Message::PRIORITY_NORMAL));

if (!msg->WriteInt(base::GetCurrentProcId())) {
if (!msg->WriteInt(GetHelloMessageProcId())) {
NOTREACHED() << "Unable to pickle hello message proc id";
}
#if defined(IPC_USES_READWRITE)
Expand Down Expand Up @@ -1211,4 +1232,11 @@ bool Channel::IsNamedServerInitialized(const std::string& channel_id) {
return ChannelImpl::IsNamedServerInitialized(channel_id);
}

#if defined(OS_LINUX)
// static
void Channel::SetGlobalPid(int pid) {
ChannelImpl::SetGlobalPid(pid);
}
#endif // OS_LINUX

} // namespace IPC
9 changes: 9 additions & 0 deletions ipc/ipc_channel_posix.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,9 @@ class Channel::ChannelImpl : public MessageLoopForIO::Watcher {
bool GetClientEuid(uid_t* client_euid) const;
void ResetToAcceptingConnectionState();
static bool IsNamedServerInitialized(const std::string& channel_id);
#if defined(OS_LINUX)
static void SetGlobalPid(int pid);
#endif // OS_LINUX

private:
bool CreatePipe(const IPC::ChannelHandle& channel_handle);
Expand All @@ -72,6 +75,7 @@ class Channel::ChannelImpl : public MessageLoopForIO::Watcher {

bool AcceptConnection();
void ClosePipeOnError();
int GetHelloMessageProcId();
void QueueHelloMessage();
bool IsHelloMessage(const Message* m) const;

Expand Down Expand Up @@ -147,6 +151,11 @@ class Channel::ChannelImpl : public MessageLoopForIO::Watcher {
// True if we are responsible for unlinking the unix domain socket file.
bool must_unlink_;

#if defined(OS_LINUX)
// If non-zero, overrides the process ID sent in the hello message.
static int global_pid_;
#endif // OS_LINUX

DISALLOW_IMPLICIT_CONSTRUCTORS(ChannelImpl);
};

Expand Down

0 comments on commit e1d67a8

Please sign in to comment.