diff --git a/chrome/nacl/nacl_helper_linux.cc b/chrome/nacl/nacl_helper_linux.cc index 90417936de5793..ab63788c2bc581 100644 --- a/chrome/nacl/nacl_helper_linux.cc +++ b/chrome/nacl/nacl_helper_linux.cc @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -20,12 +21,12 @@ #include "base/at_exit.h" #include "base/command_line.h" -#include "base/json/string_escape.h" #include "base/logging.h" #include "base/message_loop/message_loop.h" #include "base/posix/eintr_wrapper.h" #include "base/posix/global_descriptors.h" #include "base/posix/unix_domain_socket_linux.h" +#include "base/process/kill.h" #include "base/rand_util.h" #include "components/nacl/loader/nacl_listener.h" #include "components/nacl/loader/nacl_sandbox_linux.h" @@ -36,12 +37,16 @@ namespace { +struct NaClLoaderSystemInfo { + size_t prereserved_sandbox_size; + int number_of_cores; +}; + // The child must mimic the behavior of zygote_main_linux.cc on the child // side of the fork. See zygote_main_linux.cc:HandleForkRequest from // if (!child) { void BecomeNaClLoader(const std::vector& child_fds, - size_t prereserved_sandbox_size, - int number_of_cores) { + const NaClLoaderSystemInfo& system_info) { VLOG(1) << "NaCl loader: setting up IPC descriptor"; // don't need zygote FD any more if (HANDLE_EINTR(close(kNaClZygoteDescriptor)) != 0) @@ -56,61 +61,75 @@ void BecomeNaClLoader(const std::vector& child_fds, base::MessageLoopForIO main_message_loop; NaClListener listener; - listener.set_prereserved_sandbox_size(prereserved_sandbox_size); - listener.set_number_of_cores(number_of_cores); + listener.set_prereserved_sandbox_size(system_info.prereserved_sandbox_size); + listener.set_number_of_cores(system_info.number_of_cores); listener.Listen(); _exit(0); } +// Start the NaCl loader in a child created by the NaCl loader Zygote. +void ChildNaClLoaderInit(const std::vector& child_fds, + const NaClLoaderSystemInfo& system_info) { + bool validack = false; + const size_t kMaxReadSize = 1024; + char buffer[kMaxReadSize]; + // 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. + const int nread = HANDLE_EINTR(read(child_fds[kNaClParentFDIndex], buffer, + kMaxReadSize)); + const std::string switch_prefix = std::string("--") + + switches::kProcessChannelID + std::string("="); + const size_t len = switch_prefix.length(); + + if (nread < 0) { + perror("read"); + LOG(ERROR) << "read returned " << nread; + } else if (nread > static_cast(len)) { + if (switch_prefix.compare(0, len, buffer, 0, len) == 0) { + VLOG(1) << "NaCl loader is synchronised with Chrome zygote"; + CommandLine::ForCurrentProcess()->AppendSwitchASCII( + switches::kProcessChannelID, + std::string(&buffer[len], nread - len)); + validack = true; + } + } + if (HANDLE_EINTR(close(child_fds[kNaClDummyFDIndex])) != 0) + LOG(ERROR) << "close(child_fds[kNaClDummyFDIndex]) failed"; + if (HANDLE_EINTR(close(child_fds[kNaClParentFDIndex])) != 0) + LOG(ERROR) << "close(child_fds[kNaClParentFDIndex]) failed"; + if (validack) { + BecomeNaClLoader(child_fds, system_info); + } else { + LOG(ERROR) << "Failed to synch with zygote"; + } + _exit(1); +} + +// Handle a fork request from the Zygote. // Some of this code was lifted from // content/browser/zygote_main_linux.cc:ForkWithRealPid() -void HandleForkRequest(const std::vector& child_fds, - size_t prereserved_sandbox_size, - int number_of_cores) { +bool HandleForkRequest(const std::vector& child_fds, + const NaClLoaderSystemInfo& system_info, + Pickle* output_pickle) { + if (kNaClParentFDIndex + 1 != child_fds.size()) { + LOG(ERROR) << "nacl_helper: unexpected number of fds, got " + << child_fds.size(); + return false; + } + VLOG(1) << "nacl_helper: forking"; - pid_t childpid = fork(); - if (childpid < 0) { - perror("fork"); - LOG(ERROR) << "*** HandleForkRequest failed\n"; - // fall through to parent case below - } else if (childpid == 0) { // In the child process. - bool validack = false; - const size_t kMaxReadSize = 1024; - char buffer[kMaxReadSize]; - // 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. - const int nread = HANDLE_EINTR(read(child_fds[kNaClParentFDIndex], buffer, - kMaxReadSize)); - const std::string switch_prefix = std::string("--") + - switches::kProcessChannelID + std::string("="); - const size_t len = switch_prefix.length(); - - if (nread < 0) { - perror("read"); - LOG(ERROR) << "read returned " << nread; - } else if (nread > static_cast(len)) { - if (switch_prefix.compare(0, len, buffer, 0, len) == 0) { - VLOG(1) << "NaCl loader is synchronised with Chrome zygote"; - CommandLine::ForCurrentProcess()->AppendSwitchASCII( - switches::kProcessChannelID, - std::string(&buffer[len], nread - len)); - validack = true; - } - } - if (HANDLE_EINTR(close(child_fds[kNaClDummyFDIndex])) != 0) - LOG(ERROR) << "close(child_fds[kNaClDummyFDIndex]) failed"; - if (HANDLE_EINTR(close(child_fds[kNaClParentFDIndex])) != 0) - LOG(ERROR) << "close(child_fds[kNaClParentFDIndex]) failed"; - if (validack) { - BecomeNaClLoader(child_fds, prereserved_sandbox_size, number_of_cores); - } else { - LOG(ERROR) << "Failed to synch with zygote"; - } - // NOTREACHED - return; + pid_t child_pid = fork(); + if (child_pid < 0) { + PLOG(ERROR) << "*** fork() failed."; } + + if (child_pid == 0) { + ChildNaClLoaderInit(child_fds, system_info); + NOTREACHED(); + } + // I am the parent. // First, close the dummy_fd so the sandbox won't find me when // looking for the child's pid in /proc. Also close other fds. @@ -118,13 +137,47 @@ void HandleForkRequest(const std::vector& child_fds, if (HANDLE_EINTR(close(child_fds[i])) != 0) LOG(ERROR) << "close(child_fds[i]) failed"; } - VLOG(1) << "nacl_helper: childpid is " << childpid; - // Now tell childpid to the Chrome zygote. - if (HANDLE_EINTR(send(kNaClZygoteDescriptor, - &childpid, sizeof(childpid), MSG_EOR)) - != sizeof(childpid)) { - LOG(ERROR) << "*** send() to zygote failed"; + VLOG(1) << "nacl_helper: child_pid is " << child_pid; + + // Now send child_pid (eventually -1 if fork failed) to the Chrome Zygote. + output_pickle->WriteInt(child_pid); + return true; +} + +bool HandleGetTerminationStatusRequest(PickleIterator* input_iter, + Pickle* output_pickle) { + pid_t child_to_wait; + if (!input_iter->ReadInt(&child_to_wait)) { + LOG(ERROR) << "Could not read pid to wait for"; + return false; + } + + bool known_dead; + if (!input_iter->ReadBool(&known_dead)) { + LOG(ERROR) << "Could not read known_dead status"; + return false; + } + // TODO(jln): With NaCl, known_dead seems to never be set to true (unless + // called from the Zygote's kZygoteCommandReap command). This means that we + // will sometimes detect the process as still running when it's not. Fix + // this! + + int exit_code; + base::TerminationStatus status; + // See the comment in the Zygote about known_dead. + if (known_dead) { + // Make sure to not perform a blocking wait on something that + // could still be alive. + if (kill(child_to_wait, SIGKILL)) { + PLOG(ERROR) << "kill (" << child_to_wait << ")"; + } + status = base::WaitForTerminationStatus(child_to_wait, &exit_code); + } else { + status = base::GetTerminationStatus(child_to_wait, &exit_code); } + output_pickle->WriteInt(static_cast(status)); + output_pickle->WriteInt(exit_code); + return true; } // This is a poor man's check on whether we are sandboxed. @@ -137,7 +190,76 @@ bool IsSandboxed() { return true; } -} // namespace +// Honor a command |command_type|. Eventual command parameters are +// available in |input_iter| and eventual file descriptors attached to +// the command are in |attached_fds|. +// Reply to the command on |reply_fds|. +bool HonorRequestAndReply(int reply_fd, + int command_type, + const std::vector& attached_fds, + const NaClLoaderSystemInfo& system_info, + PickleIterator* input_iter) { + Pickle write_pickle; + bool have_to_reply = false; + // Commands must write anything to send back to |write_pickle|. + switch (command_type) { + case kNaClForkRequest: + have_to_reply = HandleForkRequest(attached_fds, system_info, + &write_pickle); + break; + case kNaClGetTerminationStatusRequest: + have_to_reply = + HandleGetTerminationStatusRequest(input_iter, &write_pickle); + break; + default: + LOG(ERROR) << "Unsupported command from Zygote"; + return false; + } + if (!have_to_reply) + return false; + const std::vector empty; // We never send file descriptors back. + if (!UnixDomainSocket::SendMsg(reply_fd, write_pickle.data(), + write_pickle.size(), empty)) { + LOG(ERROR) << "*** send() to zygote failed"; + return false; + } + return true; +} + +// Read a request from the Zygote from |zygote_ipc_fd| and handle it. +// Die on EOF from |zygote_ipc_fd|. +bool HandleZygoteRequest(int zygote_ipc_fd, + const NaClLoaderSystemInfo& system_info) { + std::vector fds; + char buf[kNaClMaxIPCMessageLength]; + const ssize_t msglen = UnixDomainSocket::RecvMsg(zygote_ipc_fd, + &buf, sizeof(buf), &fds); + // If the Zygote has started handling requests, we should be sandboxed via + // the setuid sandbox. + if (!IsSandboxed()) { + LOG(ERROR) << "NaCl helper process running without a sandbox!\n" + << "Most likely you need to configure your SUID sandbox " + << "correctly"; + } + if (msglen == 0 || (msglen == -1 && errno == ECONNRESET)) { + // EOF from the browser. Goodbye! + _exit(0); + } + if (msglen < 0) { + PLOG(ERROR) << "nacl_helper: receive from zygote failed"; + return false; + } + + Pickle read_pickle(buf, msglen); + PickleIterator read_iter(read_pickle); + int command_type; + if (!read_iter.ReadInt(&command_type)) { + LOG(ERROR) << "Unable to read command from Zygote"; + return false; + } + return HonorRequestAndReply(zygote_ipc_fd, command_type, fds, system_info, + &read_iter); +} static const char kNaClHelperReservedAtZero[] = "reserved_at_zero"; static const char kNaClHelperRDebug[] = "r_debug"; @@ -206,6 +328,8 @@ static size_t CheckReservedAtZero() { return prereserved_sandbox_size; } +} // namespace + #if defined(ADDRESS_SANITIZER) // Do not install the SIGSEGV handler in ASan. This should make the NaCl // platform qualification test pass. @@ -240,15 +364,17 @@ int main(int argc, char* argv[]) { // NSS is needed to perform hashing for validation caching. crypto::LoadNSSLibraries(); #endif - std::vector empty; // for SendMsg() calls - size_t prereserved_sandbox_size = CheckReservedAtZero(); - int number_of_cores = sysconf(_SC_NPROCESSORS_ONLN); + const NaClLoaderSystemInfo system_info = { + CheckReservedAtZero(), + sysconf(_SC_NPROCESSORS_ONLN) + }; CheckRDebug(argv[0]); // Check that IsSandboxed() works. We should not be sandboxed at this point. CHECK(!IsSandboxed()) << "Unexpectedly sandboxed!"; + const std::vector empty; // Send the zygote a message to let it know we are ready to help if (!UnixDomainSocket::SendMsg(kNaClZygoteDescriptor, kNaClHelperStartupAck, @@ -256,45 +382,13 @@ int main(int argc, char* argv[]) { LOG(ERROR) << "*** send() to zygote failed"; } + // Now handle requests from the Zygote. while (true) { - int badpid = -1; - std::vector fds; - static const unsigned kMaxMessageLength = 2048; - char buf[kMaxMessageLength]; - const ssize_t msglen = UnixDomainSocket::RecvMsg(kNaClZygoteDescriptor, - &buf, sizeof(buf), &fds); - // If the Zygote has started handling requests, we should be sandboxed via - // the setuid sandbox. - if (!IsSandboxed()) { - LOG(ERROR) << "NaCl helper process running without a sandbox!\n" - << "Most likely you need to configure your SUID sandbox " - << "correctly"; - } - if (msglen == 0 || (msglen == -1 && errno == ECONNRESET)) { - // EOF from the browser. Goodbye! - _exit(0); - } else if (msglen < 0) { - LOG(ERROR) << "nacl_helper: receive from zygote failed, errno = " - << errno; - } else if (msglen == sizeof(kNaClForkRequest) - 1 && - memcmp(buf, kNaClForkRequest, msglen) == 0) { - if (kNaClParentFDIndex + 1 == fds.size()) { - HandleForkRequest(fds, prereserved_sandbox_size, number_of_cores); - continue; // fork succeeded. Note: child does not return - } else { - LOG(ERROR) << "nacl_helper: unexpected number of fds, got " - << fds.size(); - } - } else { - LOG(ERROR) << "nacl_helper unrecognized request: " - << base::GetDoubleQuotedJson(std::string(buf, buf + msglen)); - _exit(-1); - } - // if fork fails, send PID=-1 to zygote - if (!UnixDomainSocket::SendMsg(kNaClZygoteDescriptor, &badpid, - sizeof(badpid), empty)) { - LOG(ERROR) << "*** send() to zygote failed"; - } + bool request_handled = HandleZygoteRequest(kNaClZygoteDescriptor, + system_info); + // Do not turn this into a CHECK() without thinking about robustness + // against malicious IPC requests. + DCHECK(request_handled); } - CHECK(false); // This routine must not return + NOTREACHED(); } diff --git a/components/nacl/common/nacl_helper_linux.h b/components/nacl/common/nacl_helper_linux.h index 732b21570a9aab..a9324b3fa1d786 100644 --- a/components/nacl/common/nacl_helper_linux.h +++ b/components/nacl/common/nacl_helper_linux.h @@ -9,10 +9,15 @@ // constants used to implement communication between the nacl_helper // process and the Chrome zygote. +#define kNaClMaxIPCMessageLength 2048 + // Used by Helper to tell Zygote it has started successfully. #define kNaClHelperStartupAck "NACLHELPER_OK" -// Used by Zygote to ask Helper to fork a new NaCl loader. -#define kNaClForkRequest "NACLFORK" + +enum NaClZygoteIPCCommand { + kNaClForkRequest, + kNaClGetTerminationStatusRequest, +}; // The next set of constants define global Linux file descriptors. // For communications between NaCl loader and browser. diff --git a/components/nacl/zygote/nacl_fork_delegate_linux.cc b/components/nacl/zygote/nacl_fork_delegate_linux.cc index 8445342fdaac39..d94ad6b9fdfe52 100644 --- a/components/nacl/zygote/nacl_fork_delegate_linux.cc +++ b/components/nacl/zygote/nacl_fork_delegate_linux.cc @@ -17,8 +17,10 @@ #include "base/files/file_path.h" #include "base/logging.h" #include "base/path_service.h" +#include "base/pickle.h" #include "base/posix/eintr_wrapper.h" #include "base/posix/unix_domain_socket_linux.h" +#include "base/process/kill.h" #include "base/process/launch.h" #include "base/third_party/dynamic_annotations/dynamic_annotations.h" #include "components/nacl/common/nacl_helper_linux.h" @@ -63,6 +65,41 @@ bool NonZeroSegmentBaseIsSlow() { } #endif +// Send an IPC request on |ipc_channel|. The request is contained in +// |request_pickle| and can have file descriptors attached in |attached_fds|. +// |reply_data_buffer| must be allocated by the caller and will contain the +// reply. The size of the reply will be written to |reply_size|. +// This code assumes that only one thread can write to |ipc_channel| to make +// requests. +bool SendIPCRequestAndReadReply(int ipc_channel, + const std::vector& attached_fds, + const Pickle& request_pickle, + char* reply_data_buffer, + size_t reply_data_buffer_size, + ssize_t* reply_size) { + DCHECK_LE(static_cast(kNaClMaxIPCMessageLength), + reply_data_buffer_size); + DCHECK(reply_size); + + if (!UnixDomainSocket::SendMsg(ipc_channel, request_pickle.data(), + request_pickle.size(), attached_fds)) { + LOG(ERROR) << "SendIPCRequestAndReadReply: SendMsg failed"; + return false; + } + + // Then read the remote reply. + std::vector received_fds; + const ssize_t msg_len = + UnixDomainSocket::RecvMsg(ipc_channel, reply_data_buffer, + reply_data_buffer_size, &received_fds); + if (msg_len <= 0) { + LOG(ERROR) << "SendIPCRequestAndReadReply: RecvMsg failed"; + return false; + } + *reply_size = msg_len; + return true; +} + } // namespace. NaClForkDelegate::NaClForkDelegate() @@ -219,22 +256,34 @@ bool NaClForkDelegate::CanHelp(const std::string& process_type, } pid_t NaClForkDelegate::Fork(const std::vector& fds) { - base::ProcessId naclchild; VLOG(1) << "NaClForkDelegate::Fork"; DCHECK(fds.size() == kNaClParentFDIndex + 1); - if (!UnixDomainSocket::SendMsg(fd_, kNaClForkRequest, - strlen(kNaClForkRequest), fds)) { - LOG(ERROR) << "NaClForkDelegate::Fork: SendMsg failed"; + + // First, send a remote fork request. + Pickle write_pickle; + write_pickle.WriteInt(kNaClForkRequest); + + char reply_buf[kNaClMaxIPCMessageLength]; + ssize_t reply_size = 0; + bool got_reply = + SendIPCRequestAndReadReply(fd_, fds, write_pickle, + reply_buf, sizeof(reply_buf), &reply_size); + if (!got_reply) { + LOG(ERROR) << "Could not perform remote fork."; return -1; } - int nread = HANDLE_EINTR(read(fd_, &naclchild, sizeof(naclchild))); - if (nread != sizeof(naclchild)) { - LOG(ERROR) << "NaClForkDelegate::Fork: read failed"; + + // Now see if the other end managed to fork. + Pickle reply_pickle(reply_buf, reply_size); + PickleIterator iter(reply_pickle); + pid_t nacl_child; + if (!iter.ReadInt(&nacl_child)) { + LOG(ERROR) << "NaClForkDelegate::Fork: pickle failed"; return -1; } - VLOG(1) << "nacl_child is " << naclchild << " (" << nread << " bytes)"; - return naclchild; + VLOG(1) << "nacl_child is " << nacl_child; + return nacl_child; } bool NaClForkDelegate::AckChild(const int fd, @@ -246,3 +295,47 @@ bool NaClForkDelegate::AckChild(const int fd, } return true; } + +bool NaClForkDelegate::GetTerminationStatus(pid_t pid, bool known_dead, + base::TerminationStatus* status, + int* exit_code) { + VLOG(1) << "NaClForkDelegate::GetTerminationStatus"; + DCHECK(status); + DCHECK(exit_code); + + Pickle write_pickle; + write_pickle.WriteInt(kNaClGetTerminationStatusRequest); + write_pickle.WriteInt(pid); + write_pickle.WriteBool(known_dead); + + const std::vector empty_fds; + char reply_buf[kNaClMaxIPCMessageLength]; + ssize_t reply_size = 0; + bool got_reply = + SendIPCRequestAndReadReply(fd_, empty_fds, write_pickle, + reply_buf, sizeof(reply_buf), &reply_size); + if (!got_reply) { + LOG(ERROR) << "Could not perform remote GetTerminationStatus."; + return false; + } + + Pickle reply_pickle(reply_buf, reply_size); + PickleIterator iter(reply_pickle); + int termination_status; + if (!iter.ReadInt(&termination_status) || + termination_status < 0 || + termination_status >= base::TERMINATION_STATUS_MAX_ENUM) { + LOG(ERROR) << "GetTerminationStatus: pickle failed"; + return false; + } + + int remote_exit_code; + if (!iter.ReadInt(&remote_exit_code)) { + LOG(ERROR) << "GetTerminationStatus: pickle failed"; + return false; + } + + *status = static_cast(termination_status); + *exit_code = remote_exit_code; + return true; +} diff --git a/components/nacl/zygote/nacl_fork_delegate_linux.h b/components/nacl/zygote/nacl_fork_delegate_linux.h index 5812f33fe26307..a5ec534aa21ebc 100644 --- a/components/nacl/zygote/nacl_fork_delegate_linux.h +++ b/components/nacl/zygote/nacl_fork_delegate_linux.h @@ -31,6 +31,9 @@ class NaClForkDelegate : public content::ZygoteForkDelegate { virtual pid_t Fork(const std::vector& fds) OVERRIDE; virtual bool AckChild(int fd, const std::string& channel_switch) OVERRIDE; + virtual bool GetTerminationStatus(pid_t pid, bool known_dead, + base::TerminationStatus* status, + int* exit_code) OVERRIDE; private: // These values are reported via UMA and hence they become permanent diff --git a/content/public/common/zygote_fork_delegate_linux.h b/content/public/common/zygote_fork_delegate_linux.h index 7beda6d3393154..a5ea9f2952f6ee 100644 --- a/content/public/common/zygote_fork_delegate_linux.h +++ b/content/public/common/zygote_fork_delegate_linux.h @@ -10,6 +10,10 @@ #include #include +// TODO(jln) base::TerminationStatus should be forward declared when switching +// to C++11. +#include "base/process/kill.h" + namespace content { // The ZygoteForkDelegate allows the Chrome Linux zygote to delegate @@ -40,15 +44,26 @@ class ZygoteForkDelegate { int* uma_sample, int* uma_boundary_value) = 0; // Delegate forks, returning a -1 on failure. Outside the - // suid sandbox, Fork() returns the Linux process ID. Inside - // the sandbox, returns a positive integer, with PID discovery - // handled by the sandbox. + // suid sandbox, Fork() returns the Linux process ID. + // This method is not aware of any potential pid namespaces, so it'll + // return a raw pid just like fork() would. virtual pid_t Fork(const std::vector& fds) = 0; - // After a successful for, signal the child to indicate that + // After a successful fork, signal the child to indicate that // the child's PID has been received. Also communicate the // channel switch as a part of acknowledgement message. virtual bool AckChild(int fd, const std::string& channel_switch) = 0; + + // The fork delegate must also assume the role of waiting for its children + // since the caller will not be their parents and cannot do it. |pid| here + // should be a pid that has been returned by the Fork() method. i.e. This + // method is completely unaware of eventual PID namespaces due to sandboxing. + // |known_dead| indicates that the process is already dead and that a + // blocking wait() should be performed. In this case, GetTerminationStatus() + // will send a SIGKILL to the target process first. + virtual bool GetTerminationStatus(pid_t pid, bool known_dead, + base::TerminationStatus* status, + int* exit_code) = 0; }; } // namespace content diff --git a/content/zygote/zygote_linux.cc b/content/zygote/zygote_linux.cc index 6e42a9d045a4d1..65ed4357a4a9dc 100644 --- a/content/zygote/zygote_linux.cc +++ b/content/zygote/zygote_linux.cc @@ -155,37 +155,99 @@ bool Zygote::HandleRequestFromBrowser(int fd) { return false; } +// TODO(jln): remove callers to this broken API. See crbug.com/274855. void Zygote::HandleReapRequest(int fd, const Pickle& pickle, PickleIterator iter) { base::ProcessId child; - base::ProcessId actual_child; if (!pickle.ReadInt(&iter, &child)) { LOG(WARNING) << "Error parsing reap request from browser"; return; } - if (UsingSUIDSandbox()) { - actual_child = real_pids_to_sandbox_pids[child]; - if (!actual_child) - return; - real_pids_to_sandbox_pids.erase(child); + if (process_info_map_.find(child) == process_info_map_.end()) { + // TODO(jln): test on more bots and add a DCHECK. + LOG(ERROR) << "Child not found!"; + return; + } + const base::ProcessId actual_child = process_info_map_[child].internal_pid; + const bool started_from_helper = + process_info_map_[child].started_from_helper; + if (!started_from_helper) { + // TODO(jln): this old code is completely broken. See crbug.com/274855. + base::EnsureProcessTerminated(actual_child); } else { - actual_child = child; + // For processes from the helper, send a GetTerminationStatus request + // with known_dead set to true. + // This is not perfect, as the process may be killed instantly, but is + // better than ignoring the request. + base::TerminationStatus status; + int exit_code; + bool got_termination_status = + GetTerminationStatus(child, true /* known_dead */, &status, &exit_code); + DCHECK(got_termination_status); } + process_info_map_.erase(child); +} - base::EnsureProcessTerminated(actual_child); +bool Zygote::GetTerminationStatus(base::ProcessHandle real_pid, + bool known_dead, + base::TerminationStatus* status, + int* exit_code) { + // Do we know about this child? + if (process_info_map_.find(real_pid) == process_info_map_.end()) { + // TODO(jln): test on more bots and add a DCHECK. + LOG(ERROR) << "Zygote::GetTerminationStatus for unknown PID " + << real_pid; + return false; + } + // We know about |real_pid|. + const base::ProcessHandle child = + process_info_map_[real_pid].internal_pid; + const bool started_from_helper = + process_info_map_[real_pid].started_from_helper; + if (started_from_helper) { + // Let the helper handle the request. + DCHECK(helper_); + if (!helper_->GetTerminationStatus(child, known_dead, status, exit_code)) { + return false; + } + } else { + // Handle the request directly. + if (known_dead) { + // If we know that the process is already dead and the kernel is cleaning + // it up, we do want to wait until it becomes a zombie and not risk + // returning eroneously that it is still running. However, we do not + // want to risk a bug where we're told a process is dead when it's not. + // By sending SIGKILL, we make sure that WaitForTerminationStatus will + // return quickly even in this case. + if (kill(child, SIGKILL)) { + PLOG(ERROR) << "kill (" << child << ")"; + } + *status = base::WaitForTerminationStatus(child, exit_code); + } else { + // We don't know if the process is dying, so get its status but don't + // wait. + *status = base::GetTerminationStatus(child, exit_code); + } + } + // Successfully got a status for |real_pid|. + if (*status != base::TERMINATION_STATUS_STILL_RUNNING) { + // Time to forget about this process. + process_info_map_.erase(real_pid); + } + return true; } void Zygote::HandleGetTerminationStatus(int fd, const Pickle& pickle, PickleIterator iter) { bool known_dead; - base::ProcessHandle child; + base::ProcessHandle child_requested; if (!pickle.ReadBool(&iter, &known_dead) || - !pickle.ReadInt(&iter, &child)) { + !pickle.ReadInt(&iter, &child_requested)) { LOG(WARNING) << "Error parsing GetTerminationStatus request " << "from browser"; return; @@ -193,26 +255,13 @@ void Zygote::HandleGetTerminationStatus(int fd, base::TerminationStatus status; int exit_code; - if (UsingSUIDSandbox()) - child = real_pids_to_sandbox_pids[child]; - if (child) { - if (known_dead) { - // If we know that the process is already dead and the kernel is cleaning - // it up, we do want to wait until it becomes a zombie and not risk - // returning eroneously that it is still running. However, we do not - // want to risk a bug where we're told a process is dead when it's not. - // By sending SIGKILL, we make sure that WaitForTerminationStatus will - // return quickly even in this case. - if (kill(child, SIGKILL)) { - PLOG(ERROR) << "kill (" << child << ")"; - } - status = base::WaitForTerminationStatus(child, &exit_code); - } else { - status = base::GetTerminationStatus(child, &exit_code); - } - } else { + + bool got_termination_status = + GetTerminationStatus(child_requested, known_dead, &status, &exit_code); + if (!got_termination_status) { // Assume that if we can't find the child in the sandbox, then // it terminated normally. + // TODO(jln): add a DCHECK. status = base::TERMINATION_STATUS_NORMAL_TERMINATION; exit_code = RESULT_CODE_NORMAL_EXIT; } @@ -236,8 +285,15 @@ int Zygote::ForkWithRealPid(const std::string& process_type, uma_name, uma_sample, uma_boundary_value)); + // TODO(jln): this shortcut is silly and does nothing but make the code + // harder to follow and to test. Get rid of it. if (!(use_helper || UsingSUIDSandbox())) { - return fork(); + pid_t pid = fork(); + if (pid > 0) { + process_info_map_[pid].internal_pid = pid; + process_info_map_[pid].started_from_helper = use_helper; + } + return pid; } int dummy_fd; @@ -326,13 +382,20 @@ int Zygote::ForkWithRealPid(const std::string& process_type, LOG(ERROR) << "METHOD_GET_CHILD_WITH_INODE failed"; goto error; } - - real_pids_to_sandbox_pids[real_pid] = pid; } else { // If no SUID sandbox is involved then no pid translation is // necessary. real_pid = pid; } + + // Now set-up this process to be tracked by the Zygote. + if (process_info_map_.find(real_pid) != process_info_map_.end()) { + // TODO(jln): add DCHECK. + LOG(ERROR) << "Already tracking PID " << real_pid; + } + process_info_map_[real_pid].internal_pid = pid; + process_info_map_[real_pid].started_from_helper = use_helper; + if (use_helper) { if (!helper_->AckChild(pipe_fds[1], channel_switch)) { LOG(ERROR) << "Failed to synchronise with zygote fork helper"; diff --git a/content/zygote/zygote_linux.h b/content/zygote/zygote_linux.h index 3b175acd8a74e2..327f917958a70f 100644 --- a/content/zygote/zygote_linux.h +++ b/content/zygote/zygote_linux.h @@ -8,7 +8,8 @@ #include #include -#include "base/containers/hash_tables.h" +#include "base/containers/small_map.h" +#include "base/process/kill.h" #include "base/process/process.h" class Pickle; @@ -33,6 +34,16 @@ class Zygote { static const int kMagicSandboxIPCDescriptor = 5; private: + struct ZygoteProcessInfo { + // Pid from inside the Zygote's PID namespace. + base::ProcessHandle internal_pid; + // Keeps track of whether or not a process was started from a fork + // delegate helper. + bool started_from_helper; + }; + typedef base::SmallMap< std::map > + ZygoteProcessMap; + // Returns true if the SUID sandbox is active. bool UsingSUIDSandbox() const; @@ -45,6 +56,14 @@ class Zygote { void HandleReapRequest(int fd, const Pickle& pickle, PickleIterator iter); + // Get the termination status of |real_pid|. |real_pid| is the PID as it + // appears outside of the sandbox. + // Return true if it managed to get the termination status and return the + // status in |status| and the exit code in |exit_code|. + bool GetTerminationStatus(base::ProcessHandle real_pid, bool known_dead, + base::TerminationStatus* status, + int* exit_code); + void HandleGetTerminationStatus(int fd, const Pickle& pickle, PickleIterator iter); @@ -84,11 +103,11 @@ class Zygote { const Pickle& pickle, PickleIterator iter); - // In the SUID sandbox, we try to use a new PID namespace. Thus the PIDs - // fork() returns are not the real PIDs, so we need to map the Real PIDS - // into the sandbox PID namespace. - typedef base::hash_map ProcessMap; - ProcessMap real_pids_to_sandbox_pids; + // The Zygote needs to keep some information about each process. Most + // notably what the PID of the process is inside the PID namespace of + // the Zygote and whether or not a process was started by the + // ZygoteForkDelegate helper. + ZygoteProcessMap process_info_map_; const int sandbox_flags_; ZygoteForkDelegate* helper_;