Skip to content

Commit

Permalink
Revert "Non-SFI mode: Use dummy PID for NaCl's IPC channel and IPC ch…
Browse files Browse the repository at this point in the history
…annel on Linux platform."

This reverts commit d3eb0838097cbffb09005db2b66ffcdd7e2b9417.

Reason for revert, it breaks chrome://webrtc_internals on Linux.

BUG=441312,358465

TBR=hidehiko@chromium.org

Review URL: https://codereview.chromium.org/795153002

Cr-Commit-Position: refs/heads/master@{#307922}
  • Loading branch information
perkj authored and Commit bot committed Dec 11, 2014
1 parent 7864167 commit dbcac35
Show file tree
Hide file tree
Showing 6 changed files with 45 additions and 25 deletions.
3 changes: 3 additions & 0 deletions content/zygote/zygote_linux.cc
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,9 @@ int Zygote::ForkWithRealPid(const std::string& process_type,
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);
// Force the real PID so chrome event data have a PID that corresponds
// to system trace event data.
base::debug::TraceLog::GetInstance()->SetProcessID(
Expand Down
7 changes: 0 additions & 7 deletions ipc/ipc_channel.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,7 @@ std::string Channel::GenerateUniqueRandomChannelID() {
// component. The strong random component prevents other processes from
// hijacking or squatting on predictable channel names.

#if defined(OS_LINUX) || defined(OS_NACL_NONSFI)
// On Linux platform, PID does not play any security role.
// In nacl_helper_nonsfi, the seccomp sandbox disallows the use of getpid().
// So, for both cases, we provide a dummy PID.
int process_id = -1;
#else
int process_id = base::GetCurrentProcId();
#endif
return base::StringPrintf("%d.%u.%d",
process_id,
g_last_id.GetNext(),
Expand Down
7 changes: 7 additions & 0 deletions ipc/ipc_channel.h
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,13 @@ class IPC_EXPORT Channel : public Sender {
static std::string GenerateVerifiedChannelID(const std::string& prefix);
#endif

#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

#if defined(OS_ANDROID)
// Most tests are single process and work the same on all platforms. However
// in some cases we want to test multi-process, and Android differs in that it
Expand Down
33 changes: 25 additions & 8 deletions ipc/ipc_channel_posix.cc
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,10 @@ void Channel::NotifyProcessForkedForTesting() {

//------------------------------------------------------------------------------

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

ChannelPosix::ChannelPosix(const IPC::ChannelHandle& channel_handle,
Mode mode, Listener* listener)
: ChannelReader(listener),
Expand Down Expand Up @@ -641,6 +645,13 @@ bool ChannelPosix::IsNamedServerInitialized(
return base::PathExists(base::FilePath(channel_id));
}

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

// Called by libevent when we can read from the pipe without blocking.
void ChannelPosix::OnFileCanReadWithoutBlocking(int fd) {
if (fd == server_listen_pipe_.get()) {
Expand Down Expand Up @@ -760,15 +771,14 @@ void ChannelPosix::ClosePipeOnError() {
}

int ChannelPosix::GetHelloMessageProcId() const {
#if defined(OS_LINUX) || defined(OS_NACL_NONSFI)
// On Linux platform, PID does not play any security role.
// In nacl_helper_nonsfi, getpid() invoked by GetCurrentProcId() is not
// allowed and would cause a SIGSYS crash because of the seccomp sandbox.
// So, for both cases, we provide a dummy PID.
return -1;
#else
return base::GetCurrentProcId();
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 ChannelPosix::QueueHelloMessage() {
Expand Down Expand Up @@ -1097,4 +1107,11 @@ bool Channel::IsNamedServerInitialized(
return ChannelPosix::IsNamedServerInitialized(channel_id);
}

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

} // namespace IPC
8 changes: 8 additions & 0 deletions ipc/ipc_channel_posix.h
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,9 @@ class IPC_EXPORT ChannelPosix : public Channel,
void CloseClientFileDescriptor();

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 Down Expand Up @@ -209,6 +212,11 @@ class IPC_EXPORT ChannelPosix : public Channel,
// 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(ChannelPosix);
};

Expand Down
12 changes: 2 additions & 10 deletions ipc/ipc_sync_channel_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1722,11 +1722,7 @@ class VerifiedServer : public Worker {
VLOG(1) << __FUNCTION__ << " Sending reply: " << reply_text_;
SyncChannelNestedTestMsg_String::WriteReplyParams(reply_msg, reply_text_);
Send(reply_msg);
#if defined(OS_LINUX) || defined(OS_NACL_NONSFI)
ASSERT_EQ(-1, channel()->GetPeerPID());
#else
ASSERT_EQ(base::GetCurrentProcId(), channel()->GetPeerPID());
#endif
ASSERT_EQ(channel()->GetPeerPID(), base::GetCurrentProcId());
Done();
}

Expand Down Expand Up @@ -1755,11 +1751,7 @@ class VerifiedClient : public Worker {
(void)expected_text_;

VLOG(1) << __FUNCTION__ << " Received reply: " << response;
#if defined(OS_LINUX) || defined(OS_NACL_NONSFI)
ASSERT_EQ(-1, channel()->GetPeerPID());
#else
ASSERT_EQ(base::GetCurrentProcId(), channel()->GetPeerPID());
#endif
ASSERT_EQ(channel()->GetPeerPID(), base::GetCurrentProcId());
Done();
}

Expand Down

0 comments on commit dbcac35

Please sign in to comment.