Commit 74722e72 authored by mdempsky@chromium.org's avatar mdempsky@chromium.org

Use RecvMsgWithPid to find zygote PID instead of chrome-sandbox

BUG=357670
NOTRY=true

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@266618 0039d316-1c4b-4281-b951-d872f2087c98
parent 3e552f39
...@@ -4,6 +4,7 @@ ...@@ -4,6 +4,7 @@
#include "content/browser/zygote_host/zygote_host_impl_linux.h" #include "content/browser/zygote_host/zygote_host_impl_linux.h"
#include <string.h>
#include <sys/socket.h> #include <sys/socket.h>
#include <sys/stat.h> #include <sys/stat.h>
#include <sys/types.h> #include <sys/types.h>
...@@ -25,6 +26,7 @@ ...@@ -25,6 +26,7 @@
#include "base/posix/unix_domain_socket_linux.h" #include "base/posix/unix_domain_socket_linux.h"
#include "base/process/launch.h" #include "base/process/launch.h"
#include "base/process/memory.h" #include "base/process/memory.h"
#include "base/process/process_handle.h"
#include "base/strings/string_number_conversions.h" #include "base/strings/string_number_conversions.h"
#include "base/strings/string_util.h" #include "base/strings/string_util.h"
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
...@@ -46,6 +48,26 @@ ...@@ -46,6 +48,26 @@
namespace content { namespace content {
// Returns true if |proc| is the same process as or a descendent process of
// |ancestor|.
static bool SameOrDescendantOf(base::ProcessId proc, base::ProcessId ancestor) {
for (unsigned i = 0; i < 100; i++) {
if (proc == ancestor)
return true;
// Walk up process tree.
base::ProcessHandle handle;
CHECK(base::OpenProcessHandle(proc, &handle));
proc = base::GetParentProcessId(handle);
base::CloseProcessHandle(handle);
if (proc <= 0)
return false;
}
NOTREACHED();
return false;
}
// static // static
ZygoteHost* ZygoteHost::GetInstance() { ZygoteHost* ZygoteHost::GetInstance() {
return ZygoteHostImpl::GetInstance(); return ZygoteHostImpl::GetInstance();
...@@ -83,6 +105,7 @@ void ZygoteHostImpl::Init(const std::string& sandbox_cmd) { ...@@ -83,6 +105,7 @@ void ZygoteHostImpl::Init(const std::string& sandbox_cmd) {
int fds[2]; int fds[2];
CHECK(socketpair(AF_UNIX, SOCK_SEQPACKET, 0, fds) == 0); CHECK(socketpair(AF_UNIX, SOCK_SEQPACKET, 0, fds) == 0);
CHECK(UnixDomainSocket::EnableReceiveProcessId(fds[0]));
base::FileHandleMappingVector fds_to_map; base::FileHandleMappingVector fds_to_map;
fds_to_map.push_back(std::make_pair(fds[1], kZygoteSocketPairFd)); fds_to_map.push_back(std::make_pair(fds[1], kZygoteSocketPairFd));
...@@ -125,58 +148,52 @@ void ZygoteHostImpl::Init(const std::string& sandbox_cmd) { ...@@ -125,58 +148,52 @@ void ZygoteHostImpl::Init(const std::string& sandbox_cmd) {
const int sfd = RenderSandboxHostLinux::GetInstance()->GetRendererSocket(); const int sfd = RenderSandboxHostLinux::GetInstance()->GetRendererSocket();
fds_to_map.push_back(std::make_pair(sfd, GetSandboxFD())); fds_to_map.push_back(std::make_pair(sfd, GetSandboxFD()));
int dummy_fd = -1; base::ScopedFD dummy_fd;
if (using_suid_sandbox_) { if (using_suid_sandbox_) {
scoped_ptr<sandbox::SetuidSandboxClient> scoped_ptr<sandbox::SetuidSandboxClient>
sandbox_client(sandbox::SetuidSandboxClient::Create()); sandbox_client(sandbox::SetuidSandboxClient::Create());
sandbox_client->PrependWrapper(&cmd_line, &options); sandbox_client->PrependWrapper(&cmd_line, &options);
sandbox_client->SetupLaunchEnvironment(); sandbox_client->SetupLaunchEnvironment();
// We no longer need this dummy socket for discovering the zygote's PID,
// but the sandbox is still hard-coded to expect a file descriptor at
// kZygoteIdFd. Fixing this requires a sandbox API change. :(
CHECK_EQ(kZygoteIdFd, sandbox_client->GetUniqueToChildFileDescriptor()); CHECK_EQ(kZygoteIdFd, sandbox_client->GetUniqueToChildFileDescriptor());
dummy_fd = socket(AF_UNIX, SOCK_DGRAM, 0); dummy_fd.reset(socket(AF_UNIX, SOCK_DGRAM, 0));
CHECK(dummy_fd >= 0); CHECK_GE(dummy_fd.get(), 0);
fds_to_map.push_back(std::make_pair(dummy_fd, kZygoteIdFd)); fds_to_map.push_back(std::make_pair(dummy_fd.get(), kZygoteIdFd));
} }
base::ProcessHandle process = -1; base::ProcessHandle process = -1;
options.fds_to_remap = &fds_to_map; options.fds_to_remap = &fds_to_map;
base::LaunchProcess(cmd_line.argv(), options, &process); base::LaunchProcess(cmd_line.argv(), options, &process);
CHECK(process != -1) << "Failed to launch zygote process"; CHECK(process != -1) << "Failed to launch zygote process";
dummy_fd.reset();
if (using_suid_sandbox_) { if (using_suid_sandbox_) {
// In the SUID sandbox, the real zygote is forked from the sandbox. // In the SUID sandbox, the real zygote is forked from the sandbox
// We need to look for it. // and will be executing in another PID namespace.
// But first, wait for the zygote to tell us it's running. // Wait for the zygote to tell us it's running, and receive its PID,
// which the kernel will translate to our PID namespace.
// The sending code is in content/browser/zygote_main_linux.cc. // The sending code is in content/browser/zygote_main_linux.cc.
std::vector<int> fds_vec; std::vector<int> fds_vec;
const int kExpectedLength = sizeof(kZygoteHelloMessage); const size_t kExpectedLength = sizeof(kZygoteHelloMessage);
char buf[kExpectedLength]; char buf[kExpectedLength];
const ssize_t len = UnixDomainSocket::RecvMsg(fds[0], buf, sizeof(buf), const ssize_t len = UnixDomainSocket::RecvMsgWithPid(
&fds_vec); fds[0], buf, sizeof(buf), &fds_vec, &pid_);
CHECK_EQ(kExpectedLength, len) << "Incorrect zygote magic length"; CHECK_EQ(kExpectedLength, static_cast<size_t>(len))
CHECK(0 == strcmp(buf, kZygoteHelloMessage)) << "Incorrect zygote hello"; << "Incorrect zygote magic length";
CHECK_EQ(0, memcmp(buf, kZygoteHelloMessage, kExpectedLength))
std::string inode_output; << "Incorrect zygote hello";
ino_t inode = 0; CHECK_EQ(0U, fds_vec.size())
// Figure out the inode for |dummy_fd|, close |dummy_fd| on our end, << "Zygote hello should not include file descriptors";
// and find the zygote process holding |dummy_fd|.
CHECK(base::FileDescriptorGetInode(&inode, dummy_fd)) if (pid_ <= 0 || !SameOrDescendantOf(pid_, base::GetProcId(process))) {
<< "Cannot get inode for dummy_fd " << dummy_fd; LOG(FATAL)
close(dummy_fd); << "Received invalid process ID for zygote; kernel might be too old? "
"See crbug.com/357670 or try using --"
std::vector<std::string> get_inode_cmdline; << switches::kDisableSetuidSandbox << " to workaround.";
get_inode_cmdline.push_back(sandbox_binary_); }
get_inode_cmdline.push_back(base::kFindInodeSwitch);
get_inode_cmdline.push_back(base::Int64ToString(inode));
CommandLine get_inode_cmd(get_inode_cmdline);
CHECK(base::GetAppOutput(get_inode_cmd, &inode_output))
<< "Find inode command failed for inode " << inode;
base::TrimWhitespaceASCII(inode_output, base::TRIM_ALL, &inode_output);
CHECK(base::StringToInt(inode_output, &pid_))
<< "Invalid find inode output: " << inode_output;
CHECK(pid_ > 0) << "Did not find zygote process (using sandbox binary "
<< sandbox_binary_ << ")";
if (process != pid_) { if (process != pid_) {
// Reap the sandbox. // Reap the sandbox.
......
...@@ -83,10 +83,10 @@ bool Zygote::ProcessRequests() { ...@@ -83,10 +83,10 @@ bool Zygote::ProcessRequests() {
if (UsingSUIDSandbox()) { if (UsingSUIDSandbox()) {
// Let the ZygoteHost know we are ready to go. // Let the ZygoteHost know we are ready to go.
// The receiving code is in content/browser/zygote_host_linux.cc. // The receiving code is in content/browser/zygote_host_linux.cc.
std::vector<int> empty;
bool r = UnixDomainSocket::SendMsg(kZygoteSocketPairFd, bool r = UnixDomainSocket::SendMsg(kZygoteSocketPairFd,
kZygoteHelloMessage, kZygoteHelloMessage,
sizeof(kZygoteHelloMessage), empty); sizeof(kZygoteHelloMessage),
std::vector<int>());
#if defined(OS_CHROMEOS) #if defined(OS_CHROMEOS)
LOG_IF(WARNING, !r) << "Sending zygote magic failed"; LOG_IF(WARNING, !r) << "Sending zygote magic failed";
// Exit normally on chromeos because session manager may send SIGTERM // Exit normally on chromeos because session manager may send SIGTERM
...@@ -489,8 +489,6 @@ base::ProcessId Zygote::ReadArgsAndFork(const Pickle& pickle, ...@@ -489,8 +489,6 @@ base::ProcessId Zygote::ReadArgsAndFork(const Pickle& pickle,
// This is the child process. // This is the child process.
close(kZygoteSocketPairFd); // Our socket from the browser. close(kZygoteSocketPairFd); // Our socket from the browser.
if (UsingSUIDSandbox())
close(kZygoteIdFd); // Another socket from the browser.
base::GlobalDescriptors::GetInstance()->Reset(mapping); base::GlobalDescriptors::GetInstance()->Reset(mapping);
// Reset the process-wide command line to our new command line. // Reset the process-wide command line to our new command line.
......
...@@ -23,6 +23,7 @@ ...@@ -23,6 +23,7 @@
#include "base/linux_util.h" #include "base/linux_util.h"
#include "base/native_library.h" #include "base/native_library.h"
#include "base/pickle.h" #include "base/pickle.h"
#include "base/posix/eintr_wrapper.h"
#include "base/posix/unix_domain_socket_linux.h" #include "base/posix/unix_domain_socket_linux.h"
#include "base/rand_util.h" #include "base/rand_util.h"
#include "base/sys_info.h" #include "base/sys_info.h"
...@@ -344,19 +345,10 @@ static void ZygotePreSandboxInit() { ...@@ -344,19 +345,10 @@ static void ZygotePreSandboxInit() {
new FontConfigIPC(GetSandboxFD()))->unref(); new FontConfigIPC(GetSandboxFD()))->unref();
} }
static void CloseFdAndHandleEintr(int fd) {
close(fd);
}
static bool CreateInitProcessReaper() { static bool CreateInitProcessReaper() {
// This "magic" socket must only appear in one process, so make sure
// it gets closed in the parent after fork().
base::Closure zygoteid_fd_closer =
base::Bind(CloseFdAndHandleEintr, kZygoteIdFd);
// The current process becomes init(1), this function returns from a // The current process becomes init(1), this function returns from a
// newly created process. // newly created process.
const bool init_created = const bool init_created = sandbox::CreateInitProcessReaper(NULL);
sandbox::CreateInitProcessReaper(&zygoteid_fd_closer);
if (!init_created) { if (!init_created) {
LOG(ERROR) << "Error creating an init process to reap zombies"; LOG(ERROR) << "Error creating an init process to reap zombies";
return false; return false;
...@@ -460,6 +452,13 @@ bool ZygoteMain(const MainFunctionParams& params, ...@@ -460,6 +452,13 @@ bool ZygoteMain(const MainFunctionParams& params,
const bool must_enable_setuid_sandbox = const bool must_enable_setuid_sandbox =
linux_sandbox->setuid_sandbox_client()->IsSuidSandboxChild(); linux_sandbox->setuid_sandbox_client()->IsSuidSandboxChild();
if (must_enable_setuid_sandbox) {
// When we're launched through the setuid sandbox, ZygoteHostImpl::Init
// arranges for kZygoteIdFd to be a dummy file descriptor to satisfy an
// ancient setuid sandbox ABI requirement. However, the descriptor is no
// longer needed, so we can simply close it right away now.
CHECK_EQ(0, IGNORE_EINTR(close(kZygoteIdFd)));
}
if (forkdelegate != NULL) { if (forkdelegate != NULL) {
VLOG(1) << "ZygoteMain: initializing fork delegate"; VLOG(1) << "ZygoteMain: initializing fork delegate";
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment