Skip to content

Commit

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

We do not want to expose PID from process on Linux, because it does not play any security role.
Specifically, in NaCl processes, now although getpid() syscall is prohibited by seccomp sandbox, it looks working, probably because of the cache in libc layer.
By this CL, Linux IPC, including nacl_helper_nonsfi, uses dummy PID (-1).
Note; as for nacl_helper process, currently, the process is under PID namespace, so "dummy-like-" PID is already used.

BUG=358465
TEST=Ran trybot.

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

Cr-Commit-Position: refs/heads/master@{#307853}
  • Loading branch information
hidehiko authored and Commit bot committed Dec 11, 2014
1 parent 2ff1341 commit 20a9a3a
Show file tree
Hide file tree
Showing 6 changed files with 25 additions and 45 deletions.
3 changes: 0 additions & 3 deletions content/zygote/zygote_linux.cc
Original file line number Diff line number Diff line change
Expand Up @@ -395,9 +395,6 @@ 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: 7 additions & 0 deletions ipc/ipc_channel.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,14 @@ 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: 0 additions & 7 deletions ipc/ipc_channel.h
Original file line number Diff line number Diff line change
Expand Up @@ -211,13 +211,6 @@ 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: 8 additions & 25 deletions ipc/ipc_channel_posix.cc
Original file line number Diff line number Diff line change
Expand Up @@ -176,10 +176,6 @@ 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 @@ -645,13 +641,6 @@ 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 @@ -771,14 +760,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)
// 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();
#endif
return pid;
}

void ChannelPosix::QueueHelloMessage() {
Expand Down Expand Up @@ -1107,11 +1097,4 @@ 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: 0 additions & 8 deletions ipc/ipc_channel_posix.h
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,6 @@ 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 @@ -212,11 +209,6 @@ 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: 10 additions & 2 deletions ipc/ipc_sync_channel_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1722,7 +1722,11 @@ class VerifiedServer : public Worker {
VLOG(1) << __FUNCTION__ << " Sending reply: " << reply_text_;
SyncChannelNestedTestMsg_String::WriteReplyParams(reply_msg, reply_text_);
Send(reply_msg);
ASSERT_EQ(channel()->GetPeerPID(), base::GetCurrentProcId());
#if defined(OS_LINUX) || defined(OS_NACL_NONSFI)
ASSERT_EQ(-1, channel()->GetPeerPID());
#else
ASSERT_EQ(base::GetCurrentProcId(), channel()->GetPeerPID());
#endif
Done();
}

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

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

Expand Down

0 comments on commit 20a9a3a

Please sign in to comment.