From 8feaa6723381a1b30ea3afcd3b90faf6f3138ddd Mon Sep 17 00:00:00 2001 From: "mdempsky@chromium.org" Date: Wed, 30 Apr 2014 21:57:10 +0000 Subject: [PATCH] Change UnixDomainSocket::RecvMsg to return ScopedVector This is slightly suboptimal because ScopedVector forces each ScopedFD to be individually heap allocated, but it's the simplest solution until C++11 is available. BUG=360274 NOTRY=true Review URL: https://codereview.chromium.org/258543006 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@267350 0039d316-1c4b-4281-b951-d872f2087c98 --- base/posix/unix_domain_socket_linux.cc | 66 ++++++++----- base/posix/unix_domain_socket_linux.h | 8 +- base/posix/unix_domain_socket_linux_unittest.cc | 21 ++-- components/nacl/loader/nacl_helper_linux.cc | 64 ++++++------ components/nacl/zygote/nacl_fork_delegate_linux.cc | 3 +- content/browser/renderer_host/sandbox_ipc_linux.cc | 108 +++++++++++---------- content/browser/renderer_host/sandbox_ipc_linux.h | 19 ++-- .../browser/zygote_host/zygote_host_impl_linux.cc | 3 +- content/zygote/zygote_linux.cc | 30 +++--- content/zygote/zygote_linux.h | 7 +- sandbox/linux/services/broker_process.cc | 14 +-- .../linux/services/unix_domain_socket_unittest.cc | 6 +- 12 files changed, 189 insertions(+), 160 deletions(-) diff --git a/base/posix/unix_domain_socket_linux.cc b/base/posix/unix_domain_socket_linux.cc index 8bfc8cea7fcf..aaa7067f76fe 100644 --- a/base/posix/unix_domain_socket_linux.cc +++ b/base/posix/unix_domain_socket_linux.cc @@ -9,13 +9,29 @@ #include #include +#include + +#include "base/files/scoped_file.h" #include "base/logging.h" +#include "base/memory/scoped_vector.h" #include "base/pickle.h" #include "base/posix/eintr_wrapper.h" #include "base/stl_util.h" const size_t UnixDomainSocket::kMaxFileDescriptors = 16; +// Creates a connected pair of UNIX-domain SOCK_SEQPACKET sockets, and passes +// ownership of the newly allocated file descriptors to |one| and |two|. +// Returns true on success. +static bool CreateSocketPair(base::ScopedFD* one, base::ScopedFD* two) { + int raw_socks[2]; + if (socketpair(AF_UNIX, SOCK_SEQPACKET, 0, raw_socks) == -1) + return false; + one->reset(raw_socks[0]); + two->reset(raw_socks[1]); + return true; +} + // static bool UnixDomainSocket::EnableReceiveProcessId(int fd) { const int enable = 1; @@ -63,7 +79,7 @@ bool UnixDomainSocket::SendMsg(int fd, ssize_t UnixDomainSocket::RecvMsg(int fd, void* buf, size_t length, - std::vector* fds) { + ScopedVector* fds) { return UnixDomainSocket::RecvMsgWithPid(fd, buf, length, fds, NULL); } @@ -71,7 +87,7 @@ ssize_t UnixDomainSocket::RecvMsg(int fd, ssize_t UnixDomainSocket::RecvMsgWithPid(int fd, void* buf, size_t length, - std::vector* fds, + ScopedVector* fds, base::ProcessId* pid) { return UnixDomainSocket::RecvMsgWithFlags(fd, buf, length, 0, fds, pid); } @@ -81,7 +97,7 @@ ssize_t UnixDomainSocket::RecvMsgWithFlags(int fd, void* buf, size_t length, int flags, - std::vector* fds, + ScopedVector* fds, base::ProcessId* out_pid) { fds->clear(); @@ -131,8 +147,8 @@ ssize_t UnixDomainSocket::RecvMsgWithFlags(int fd, } if (wire_fds) { - fds->resize(wire_fds_len); - memcpy(vector_as_array(fds), wire_fds, sizeof(int) * wire_fds_len); + for (unsigned i = 0; i < wire_fds_len; ++i) + fds->push_back(new base::ScopedFD(wire_fds[i])); } if (out_pid) { @@ -161,44 +177,42 @@ ssize_t UnixDomainSocket::SendRecvMsgWithFlags(int fd, int recvmsg_flags, int* result_fd, const Pickle& request) { - int fds[2]; - // This socketpair is only used for the IPC and is cleaned up before // returning. - if (socketpair(AF_UNIX, SOCK_SEQPACKET, 0, fds) == -1) + base::ScopedFD recv_sock, send_sock; + if (!CreateSocketPair(&recv_sock, &send_sock)) return -1; - std::vector fd_vector; - fd_vector.push_back(fds[1]); - if (!SendMsg(fd, request.data(), request.size(), fd_vector)) { - close(fds[0]); - close(fds[1]); - return -1; + { + std::vector send_fds; + send_fds.push_back(send_sock.get()); + if (!SendMsg(fd, request.data(), request.size(), send_fds)) + return -1; } - close(fds[1]); - fd_vector.clear(); + // Close the sending end of the socket right away so that if our peer closes + // it before sending a response (e.g., from exiting), RecvMsgWithFlags() will + // return EOF instead of hanging. + send_sock.reset(); + + ScopedVector recv_fds; // When porting to OSX keep in mind it doesn't support MSG_NOSIGNAL, so the // sender might get a SIGPIPE. const ssize_t reply_len = RecvMsgWithFlags( - fds[0], reply, max_reply_len, recvmsg_flags, &fd_vector, NULL); - close(fds[0]); + recv_sock.get(), reply, max_reply_len, recvmsg_flags, &recv_fds, NULL); + recv_sock.reset(); if (reply_len == -1) return -1; - if ((!fd_vector.empty() && result_fd == NULL) || fd_vector.size() > 1) { - for (std::vector::const_iterator - i = fd_vector.begin(); i != fd_vector.end(); ++i) { - close(*i); - } - + // If we received more file descriptors than caller expected, then we treat + // that as an error. + if (recv_fds.size() > (result_fd != NULL ? 1 : 0)) { NOTREACHED(); - return -1; } if (result_fd) - *result_fd = fd_vector.empty() ? -1 : fd_vector[0]; + *result_fd = recv_fds.empty() ? -1 : recv_fds[0]->release(); return reply_len; } diff --git a/base/posix/unix_domain_socket_linux.h b/base/posix/unix_domain_socket_linux.h index 1b30ef5e5e45..59bb8840b9df 100644 --- a/base/posix/unix_domain_socket_linux.h +++ b/base/posix/unix_domain_socket_linux.h @@ -10,6 +10,8 @@ #include #include "base/base_export.h" +#include "base/files/scoped_file.h" +#include "base/memory/scoped_vector.h" #include "base/process/process_handle.h" class Pickle; @@ -36,7 +38,7 @@ class BASE_EXPORT UnixDomainSocket { static ssize_t RecvMsg(int fd, void* msg, size_t length, - std::vector* fds); + ScopedVector* fds); // Same as RecvMsg above, but also returns the sender's process ID (as seen // from the caller's namespace). However, before using this function to @@ -45,7 +47,7 @@ class BASE_EXPORT UnixDomainSocket { static ssize_t RecvMsgWithPid(int fd, void* msg, size_t length, - std::vector* fds, + ScopedVector* fds, base::ProcessId* pid); // Perform a sendmsg/recvmsg pair. @@ -86,7 +88,7 @@ class BASE_EXPORT UnixDomainSocket { void* msg, size_t length, int flags, - std::vector* fds, + ScopedVector* fds, base::ProcessId* pid); }; diff --git a/base/posix/unix_domain_socket_linux_unittest.cc b/base/posix/unix_domain_socket_linux_unittest.cc index 5a14b41e8db6..d3082d315713 100644 --- a/base/posix/unix_domain_socket_linux_unittest.cc +++ b/base/posix/unix_domain_socket_linux_unittest.cc @@ -10,6 +10,7 @@ #include "base/bind_helpers.h" #include "base/file_util.h" #include "base/files/scoped_file.h" +#include "base/memory/scoped_vector.h" #include "base/pickle.h" #include "base/posix/unix_domain_socket_linux.h" #include "base/synchronization/waitable_event.h" @@ -38,7 +39,7 @@ TEST(UnixDomainSocketTest, SendRecvMsgAbortOnReplyFDClose) { request)); // Receive the message. - std::vector message_fds; + ScopedVector message_fds; uint8_t buffer[16]; ASSERT_EQ(static_cast(request.size()), UnixDomainSocket::RecvMsg(fds[0], buffer, sizeof(buffer), @@ -46,7 +47,7 @@ TEST(UnixDomainSocketTest, SendRecvMsgAbortOnReplyFDClose) { ASSERT_EQ(1U, message_fds.size()); // Close the reply FD. - ASSERT_EQ(0, IGNORE_EINTR(close(message_fds.front()))); + message_fds.clear(); // Check that the thread didn't get blocked. WaitableEvent event(false, false); @@ -94,7 +95,7 @@ TEST(UnixDomainSocketTest, RecvPid) { // sizeof(kHello) bytes and it wasn't just truncated to fit the buffer. char buf[sizeof(kHello) + 1]; base::ProcessId sender_pid; - std::vector fd_vec; + ScopedVector fd_vec; const ssize_t nread = UnixDomainSocket::RecvMsgWithPid( recv_sock.get(), buf, sizeof(buf), &fd_vec, &sender_pid); ASSERT_EQ(sizeof(kHello), static_cast(nread)); @@ -114,23 +115,21 @@ TEST(UnixDomainSocketTest, RecvPidWithMaxDescriptors) { ASSERT_TRUE(UnixDomainSocket::EnableReceiveProcessId(recv_sock.get())); static const char kHello[] = "hello"; - std::vector fd_vec(UnixDomainSocket::kMaxFileDescriptors, - send_sock.get()); + std::vector send_fds(UnixDomainSocket::kMaxFileDescriptors, + send_sock.get()); ASSERT_TRUE(UnixDomainSocket::SendMsg( - send_sock.get(), kHello, sizeof(kHello), fd_vec)); + send_sock.get(), kHello, sizeof(kHello), send_fds)); // Extra receiving buffer space to make sure we really received only // sizeof(kHello) bytes and it wasn't just truncated to fit the buffer. char buf[sizeof(kHello) + 1]; base::ProcessId sender_pid; + ScopedVector recv_fds; const ssize_t nread = UnixDomainSocket::RecvMsgWithPid( - recv_sock.get(), buf, sizeof(buf), &fd_vec, &sender_pid); + recv_sock.get(), buf, sizeof(buf), &recv_fds, &sender_pid); ASSERT_EQ(sizeof(kHello), static_cast(nread)); ASSERT_EQ(0, memcmp(buf, kHello, sizeof(kHello))); - ASSERT_EQ(UnixDomainSocket::kMaxFileDescriptors, fd_vec.size()); - for (size_t i = 0; i < UnixDomainSocket::kMaxFileDescriptors; i++) { - ASSERT_EQ(0, IGNORE_EINTR(close(fd_vec[i]))); - } + ASSERT_EQ(UnixDomainSocket::kMaxFileDescriptors, recv_fds.size()); ASSERT_EQ(getpid(), sender_pid); } diff --git a/components/nacl/loader/nacl_helper_linux.cc b/components/nacl/loader/nacl_helper_linux.cc index 1fbfdad510db..eaf6204d6967 100644 --- a/components/nacl/loader/nacl_helper_linux.cc +++ b/components/nacl/loader/nacl_helper_linux.cc @@ -21,8 +21,10 @@ #include "base/at_exit.h" #include "base/command_line.h" +#include "base/files/scoped_file.h" #include "base/logging.h" #include "base/memory/scoped_ptr.h" +#include "base/memory/scoped_vector.h" #include "base/message_loop/message_loop.h" #include "base/posix/eintr_wrapper.h" #include "base/posix/global_descriptors.h" @@ -62,7 +64,7 @@ void ReplaceFDWithDummy(int file_descriptor) { // 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, +void BecomeNaClLoader(base::ScopedFD browser_fd, const NaClLoaderSystemInfo& system_info, bool uses_nonsfi_mode, nacl::NaClSandbox* nacl_sandbox) { @@ -94,9 +96,8 @@ void BecomeNaClLoader(const std::vector& child_fds, nacl_sandbox->SealLayerOneSandbox(); nacl_sandbox->CheckSandboxingStateWithPolicy(); - base::GlobalDescriptors::GetInstance()->Set( - kPrimaryIPCChannel, - child_fds[content::ZygoteForkDelegate::kBrowserFDIndex]); + base::GlobalDescriptors::GetInstance()->Set(kPrimaryIPCChannel, + browser_fd.release()); base::MessageLoopForIO main_message_loop; NaClListener listener; @@ -108,22 +109,21 @@ void BecomeNaClLoader(const std::vector& child_fds, } // Start the NaCl loader in a child created by the NaCl loader Zygote. -void ChildNaClLoaderInit(const std::vector& child_fds, +void ChildNaClLoaderInit(ScopedVector child_fds, const NaClLoaderSystemInfo& system_info, bool uses_nonsfi_mode, nacl::NaClSandbox* nacl_sandbox, const std::string& channel_id) { - const int parent_fd = child_fds[content::ZygoteForkDelegate::kParentFDIndex]; - const int dummy_fd = child_fds[content::ZygoteForkDelegate::kDummyFDIndex]; - bool validack = false; 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. - const ssize_t nread = - HANDLE_EINTR(read(parent_fd, &real_pid, sizeof(real_pid))); + const ssize_t nread = HANDLE_EINTR( + read(child_fds[content::ZygoteForkDelegate::kParentFDIndex]->get(), + &real_pid, + sizeof(real_pid))); if (static_cast(nread) == sizeof(real_pid)) { // Make sure the parent didn't accidentally send us our real PID. // We don't want it to be discoverable anywhere in our address space @@ -139,12 +139,13 @@ void ChildNaClLoaderInit(const std::vector& child_fds, LOG(ERROR) << "read returned " << nread; } - if (IGNORE_EINTR(close(dummy_fd)) != 0) - LOG(ERROR) << "close(dummy_fd) failed"; - if (IGNORE_EINTR(close(parent_fd)) != 0) - LOG(ERROR) << "close(parent_fd) failed"; + base::ScopedFD browser_fd( + child_fds[content::ZygoteForkDelegate::kBrowserFDIndex]->Pass()); + child_fds.clear(); + if (validack) { - BecomeNaClLoader(child_fds, system_info, uses_nonsfi_mode, nacl_sandbox); + BecomeNaClLoader( + browser_fd.Pass(), system_info, uses_nonsfi_mode, nacl_sandbox); } else { LOG(ERROR) << "Failed to synch with zygote"; } @@ -154,7 +155,7 @@ void ChildNaClLoaderInit(const std::vector& child_fds, // Handle a fork request from the Zygote. // Some of this code was lifted from // content/browser/zygote_main_linux.cc:ForkWithRealPid() -bool HandleForkRequest(const std::vector& child_fds, +bool HandleForkRequest(ScopedVector child_fds, const NaClLoaderSystemInfo& system_info, nacl::NaClSandbox* nacl_sandbox, PickleIterator* input_iter, @@ -184,18 +185,18 @@ bool HandleForkRequest(const std::vector& child_fds, } if (child_pid == 0) { - ChildNaClLoaderInit( - child_fds, system_info, uses_nonsfi_mode, nacl_sandbox, channel_id); + ChildNaClLoaderInit(child_fds.Pass(), + system_info, + uses_nonsfi_mode, + nacl_sandbox, + channel_id); 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. - for (size_t i = 0; i < child_fds.size(); i++) { - if (IGNORE_EINTR(close(child_fds[i])) != 0) - LOG(ERROR) << "close(child_fds[i]) failed"; - } + child_fds.clear(); VLOG(1) << "nacl_helper: child_pid is " << child_pid; // Now send child_pid (eventually -1 if fork failed) to the Chrome Zygote. @@ -238,7 +239,7 @@ bool HandleGetTerminationStatusRequest(PickleIterator* input_iter, // Reply to the command on |reply_fds|. bool HonorRequestAndReply(int reply_fd, int command_type, - const std::vector& attached_fds, + ScopedVector attached_fds, const NaClLoaderSystemInfo& system_info, nacl::NaClSandbox* nacl_sandbox, PickleIterator* input_iter) { @@ -247,8 +248,11 @@ bool HonorRequestAndReply(int reply_fd, // Commands must write anything to send back to |write_pickle|. switch (command_type) { case nacl::kNaClForkRequest: - have_to_reply = HandleForkRequest( - attached_fds, system_info, nacl_sandbox, input_iter, &write_pickle); + have_to_reply = HandleForkRequest(attached_fds.Pass(), + system_info, + nacl_sandbox, + input_iter, + &write_pickle); break; case nacl::kNaClGetTerminationStatusRequest: have_to_reply = @@ -274,7 +278,7 @@ bool HonorRequestAndReply(int reply_fd, bool HandleZygoteRequest(int zygote_ipc_fd, const NaClLoaderSystemInfo& system_info, nacl::NaClSandbox* nacl_sandbox) { - std::vector fds; + ScopedVector fds; char buf[kNaClMaxIPCMessageLength]; const ssize_t msglen = UnixDomainSocket::RecvMsg(zygote_ipc_fd, &buf, sizeof(buf), &fds); @@ -301,8 +305,12 @@ bool HandleZygoteRequest(int zygote_ipc_fd, LOG(ERROR) << "Unable to read command from Zygote"; return false; } - return HonorRequestAndReply( - zygote_ipc_fd, command_type, fds, system_info, nacl_sandbox, &read_iter); + return HonorRequestAndReply(zygote_ipc_fd, + command_type, + fds.Pass(), + system_info, + nacl_sandbox, + &read_iter); } static const char kNaClHelperReservedAtZero[] = "reserved_at_zero"; diff --git a/components/nacl/zygote/nacl_fork_delegate_linux.cc b/components/nacl/zygote/nacl_fork_delegate_linux.cc index e6bb77e9b8b0..ca373dfe6c99 100644 --- a/components/nacl/zygote/nacl_fork_delegate_linux.cc +++ b/components/nacl/zygote/nacl_fork_delegate_linux.cc @@ -18,6 +18,7 @@ #include "base/files/scoped_file.h" #include "base/logging.h" #include "base/memory/scoped_ptr.h" +#include "base/memory/scoped_vector.h" #include "base/path_service.h" #include "base/pickle.h" #include "base/posix/eintr_wrapper.h" @@ -93,7 +94,7 @@ bool SendIPCRequestAndReadReply(int ipc_channel, } // Then read the remote reply. - std::vector received_fds; + ScopedVector received_fds; const ssize_t msg_len = UnixDomainSocket::RecvMsg(ipc_channel, reply_data_buffer, reply_data_buffer_size, &received_fds); diff --git a/content/browser/renderer_host/sandbox_ipc_linux.cc b/content/browser/renderer_host/sandbox_ipc_linux.cc index ad1747e01736..f988ae942dc0 100644 --- a/content/browser/renderer_host/sandbox_ipc_linux.cc +++ b/content/browser/renderer_host/sandbox_ipc_linux.cc @@ -11,7 +11,9 @@ #include #include "base/command_line.h" +#include "base/files/scoped_file.h" #include "base/linux_util.h" +#include "base/memory/scoped_vector.h" #include "base/memory/shared_memory.h" #include "base/posix/eintr_wrapper.h" #include "base/posix/unix_domain_socket_linux.h" @@ -186,7 +188,7 @@ void SandboxIPCProcess::Run() { } void SandboxIPCProcess::HandleRequestFromRenderer(int fd) { - std::vector fds; + ScopedVector fds; // A FontConfigIPC::METHOD_MATCH message could be kMaxFontFamilyLength // bytes long (this is the largest message type). @@ -208,29 +210,24 @@ void SandboxIPCProcess::HandleRequestFromRenderer(int fd) { int kind; if (!pickle.ReadInt(&iter, &kind)) - goto error; + return; if (kind == FontConfigIPC::METHOD_MATCH) { - HandleFontMatchRequest(fd, pickle, iter, fds); + HandleFontMatchRequest(fd, pickle, iter, fds.get()); } else if (kind == FontConfigIPC::METHOD_OPEN) { - HandleFontOpenRequest(fd, pickle, iter, fds); + HandleFontOpenRequest(fd, pickle, iter, fds.get()); } else if (kind == LinuxSandbox::METHOD_GET_FONT_FAMILY_FOR_CHAR) { - HandleGetFontFamilyForChar(fd, pickle, iter, fds); + HandleGetFontFamilyForChar(fd, pickle, iter, fds.get()); } else if (kind == LinuxSandbox::METHOD_LOCALTIME) { - HandleLocaltime(fd, pickle, iter, fds); + HandleLocaltime(fd, pickle, iter, fds.get()); } else if (kind == LinuxSandbox::METHOD_GET_CHILD_WITH_INODE) { - HandleGetChildWithInode(fd, pickle, iter, fds); + HandleGetChildWithInode(fd, pickle, iter, fds.get()); } else if (kind == LinuxSandbox::METHOD_GET_STYLE_FOR_STRIKE) { - HandleGetStyleForStrike(fd, pickle, iter, fds); + HandleGetStyleForStrike(fd, pickle, iter, fds.get()); } else if (kind == LinuxSandbox::METHOD_MAKE_SHARED_MEMORY_SEGMENT) { - HandleMakeSharedMemorySegment(fd, pickle, iter, fds); + HandleMakeSharedMemorySegment(fd, pickle, iter, fds.get()); } else if (kind == LinuxSandbox::METHOD_MATCH_WITH_FALLBACK) { - HandleMatchWithFallback(fd, pickle, iter, fds); - } - -error: - for (std::vector::const_iterator i = fds.begin(); i != fds.end(); ++i) { - close(*i); + HandleMatchWithFallback(fd, pickle, iter, fds.get()); } } @@ -244,10 +241,11 @@ int SandboxIPCProcess::FindOrAddPath(const SkString& path) { return count; } -void SandboxIPCProcess::HandleFontMatchRequest(int fd, - const Pickle& pickle, - PickleIterator iter, - std::vector& fds) { +void SandboxIPCProcess::HandleFontMatchRequest( + int fd, + const Pickle& pickle, + PickleIterator iter, + const std::vector& fds) { uint32_t requested_style; std::string family; if (!pickle.ReadString(&iter, &family) || @@ -283,10 +281,11 @@ void SandboxIPCProcess::HandleFontMatchRequest(int fd, SendRendererReply(fds, reply, -1); } -void SandboxIPCProcess::HandleFontOpenRequest(int fd, - const Pickle& pickle, - PickleIterator iter, - std::vector& fds) { +void SandboxIPCProcess::HandleFontOpenRequest( + int fd, + const Pickle& pickle, + PickleIterator iter, + const std::vector& fds) { uint32_t index; if (!pickle.ReadUInt32(&iter, &index)) return; @@ -311,10 +310,11 @@ void SandboxIPCProcess::HandleFontOpenRequest(int fd, } } -void SandboxIPCProcess::HandleGetFontFamilyForChar(int fd, - const Pickle& pickle, - PickleIterator iter, - std::vector& fds) { +void SandboxIPCProcess::HandleGetFontFamilyForChar( + int fd, + const Pickle& pickle, + PickleIterator iter, + const std::vector& fds) { // The other side of this call is // chrome/renderer/renderer_sandbox_support_linux.cc @@ -341,10 +341,11 @@ void SandboxIPCProcess::HandleGetFontFamilyForChar(int fd, SendRendererReply(fds, reply, -1); } -void SandboxIPCProcess::HandleGetStyleForStrike(int fd, - const Pickle& pickle, - PickleIterator iter, - std::vector& fds) { +void SandboxIPCProcess::HandleGetStyleForStrike( + int fd, + const Pickle& pickle, + PickleIterator iter, + const std::vector& fds) { std::string family; int sizeAndStyle; @@ -369,10 +370,11 @@ void SandboxIPCProcess::HandleGetStyleForStrike(int fd, SendRendererReply(fds, reply, -1); } -void SandboxIPCProcess::HandleLocaltime(int fd, - const Pickle& pickle, - PickleIterator iter, - std::vector& fds) { +void SandboxIPCProcess::HandleLocaltime( + int fd, + const Pickle& pickle, + PickleIterator iter, + const std::vector& fds) { // The other side of this call is in zygote_main_linux.cc std::string time_string; @@ -401,10 +403,11 @@ void SandboxIPCProcess::HandleLocaltime(int fd, SendRendererReply(fds, reply, -1); } -void SandboxIPCProcess::HandleGetChildWithInode(int fd, - const Pickle& pickle, - PickleIterator iter, - std::vector& fds) { +void SandboxIPCProcess::HandleGetChildWithInode( + int fd, + const Pickle& pickle, + PickleIterator iter, + const std::vector& fds) { // The other side of this call is in zygote_main_linux.cc if (sandbox_cmd_.empty()) { LOG(ERROR) << "Not in the sandbox, this should not be called"; @@ -435,10 +438,11 @@ void SandboxIPCProcess::HandleGetChildWithInode(int fd, SendRendererReply(fds, reply, -1); } -void SandboxIPCProcess::HandleMakeSharedMemorySegment(int fd, - const Pickle& pickle, - PickleIterator iter, - std::vector& fds) { +void SandboxIPCProcess::HandleMakeSharedMemorySegment( + int fd, + const Pickle& pickle, + PickleIterator iter, + const std::vector& fds) { base::SharedMemoryCreateOptions options; uint32_t size; if (!pickle.ReadUInt32(&iter, &size)) @@ -454,10 +458,11 @@ void SandboxIPCProcess::HandleMakeSharedMemorySegment(int fd, SendRendererReply(fds, reply, shm_fd); } -void SandboxIPCProcess::HandleMatchWithFallback(int fd, - const Pickle& pickle, - PickleIterator iter, - std::vector& fds) { +void SandboxIPCProcess::HandleMatchWithFallback( + int fd, + const Pickle& pickle, + PickleIterator iter, + const std::vector& fds) { // Unlike the other calls, for which we are an indirection in front of // WebKit or Skia, this call is always made via this sandbox helper // process. Therefore the fontconfig code goes in here directly. @@ -613,9 +618,10 @@ void SandboxIPCProcess::HandleMatchWithFallback(int fd, } } -void SandboxIPCProcess::SendRendererReply(const std::vector& fds, - const Pickle& reply, - int reply_fd) { +void SandboxIPCProcess::SendRendererReply( + const std::vector& fds, + const Pickle& reply, + int reply_fd) { struct msghdr msg; memset(&msg, 0, sizeof(msg)); struct iovec iov = {const_cast(reply.data()), reply.size()}; @@ -644,7 +650,7 @@ void SandboxIPCProcess::SendRendererReply(const std::vector& fds, msg.msg_controllen = cmsg->cmsg_len; } - if (HANDLE_EINTR(sendmsg(fds[0], &msg, MSG_DONTWAIT)) < 0) + if (HANDLE_EINTR(sendmsg(fds[0]->get(), &msg, MSG_DONTWAIT)) < 0) PLOG(ERROR) << "sendmsg"; } diff --git a/content/browser/renderer_host/sandbox_ipc_linux.h b/content/browser/renderer_host/sandbox_ipc_linux.h index 2ebefc2e84df..e9009546dea3 100644 --- a/content/browser/renderer_host/sandbox_ipc_linux.h +++ b/content/browser/renderer_host/sandbox_ipc_linux.h @@ -9,6 +9,7 @@ #include +#include "base/files/scoped_file.h" #include "base/memory/scoped_ptr.h" #include "base/pickle.h" #include "content/child/blink_platform_impl.h" @@ -42,44 +43,44 @@ class SandboxIPCProcess { void HandleFontMatchRequest(int fd, const Pickle& pickle, PickleIterator iter, - std::vector& fds); + const std::vector& fds); void HandleFontOpenRequest(int fd, const Pickle& pickle, PickleIterator iter, - std::vector& fds); + const std::vector& fds); void HandleGetFontFamilyForChar(int fd, const Pickle& pickle, PickleIterator iter, - std::vector& fds); + const std::vector& fds); void HandleGetStyleForStrike(int fd, const Pickle& pickle, PickleIterator iter, - std::vector& fds); + const std::vector& fds); void HandleLocaltime(int fd, const Pickle& pickle, PickleIterator iter, - std::vector& fds); + const std::vector& fds); void HandleGetChildWithInode(int fd, const Pickle& pickle, PickleIterator iter, - std::vector& fds); + const std::vector& fds); void HandleMakeSharedMemorySegment(int fd, const Pickle& pickle, PickleIterator iter, - std::vector& fds); + const std::vector& fds); void HandleMatchWithFallback(int fd, const Pickle& pickle, PickleIterator iter, - std::vector& fds); + const std::vector& fds); - void SendRendererReply(const std::vector& fds, + void SendRendererReply(const std::vector& fds, const Pickle& reply, int reply_fd); diff --git a/content/browser/zygote_host/zygote_host_impl_linux.cc b/content/browser/zygote_host/zygote_host_impl_linux.cc index a56d646f7efb..cb6545b4527f 100644 --- a/content/browser/zygote_host/zygote_host_impl_linux.cc +++ b/content/browser/zygote_host/zygote_host_impl_linux.cc @@ -20,6 +20,7 @@ #include "base/logging.h" #include "base/memory/linked_ptr.h" #include "base/memory/scoped_ptr.h" +#include "base/memory/scoped_vector.h" #include "base/metrics/histogram.h" #include "base/path_service.h" #include "base/posix/eintr_wrapper.h" @@ -176,7 +177,7 @@ void ZygoteHostImpl::Init(const std::string& sandbox_cmd) { // 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. - std::vector fds_vec; + ScopedVector fds_vec; const size_t kExpectedLength = sizeof(kZygoteHelloMessage); char buf[kExpectedLength]; const ssize_t len = UnixDomainSocket::RecvMsgWithPid( diff --git a/content/zygote/zygote_linux.cc b/content/zygote/zygote_linux.cc index d2dc66a0d1ca..67b071c2fd08 100644 --- a/content/zygote/zygote_linux.cc +++ b/content/zygote/zygote_linux.cc @@ -15,6 +15,7 @@ #include "base/file_util.h" #include "base/linux_util.h" #include "base/logging.h" +#include "base/macros.h" #include "base/pickle.h" #include "base/posix/eintr_wrapper.h" #include "base/posix/global_descriptors.h" @@ -122,7 +123,7 @@ bool Zygote::UsingSUIDSandbox() const { } bool Zygote::HandleRequestFromBrowser(int fd) { - std::vector fds; + ScopedVector fds; char buf[kZygoteMaxMessageLength]; const ssize_t len = UnixDomainSocket::RecvMsg(fd, buf, sizeof(buf), &fds); @@ -145,7 +146,7 @@ bool Zygote::HandleRequestFromBrowser(int fd) { switch (kind) { case kZygoteCommandFork: // This function call can return multiple times, once per fork(). - return HandleForkRequest(fd, pickle, iter, fds); + return HandleForkRequest(fd, pickle, iter, fds.Pass()); case kZygoteCommandReap: if (!fds.empty()) @@ -167,9 +168,6 @@ bool Zygote::HandleRequestFromBrowser(int fd) { } LOG(WARNING) << "Error parsing message from browser"; - for (std::vector::const_iterator - i = fds.begin(); i != fds.end(); ++i) - close(*i); return false; } @@ -439,7 +437,7 @@ int Zygote::ForkWithRealPid(const std::string& process_type, base::ProcessId Zygote::ReadArgsAndFork(const Pickle& pickle, PickleIterator iter, - std::vector& fds, + ScopedVector fds, std::string* uma_name, int* uma_sample, int* uma_boundary_value) { @@ -475,7 +473,7 @@ base::ProcessId Zygote::ReadArgsAndFork(const Pickle& pickle, base::GlobalDescriptors::Key key; if (!pickle.ReadUInt32(&iter, &key)) return -1; - mapping.push_back(std::make_pair(key, fds[i])); + mapping.push_back(std::make_pair(key, fds[i]->get())); } mapping.push_back(std::make_pair( @@ -488,7 +486,13 @@ base::ProcessId Zygote::ReadArgsAndFork(const Pickle& pickle, if (!child_pid) { // This is the child process. - close(kZygoteSocketPairFd); // Our socket from the browser. + // Our socket from the browser. + PCHECK(0 == IGNORE_EINTR(close(kZygoteSocketPairFd))); + + // Pass ownership of file descriptors from fds to GlobalDescriptors. + for (ScopedVector::iterator i = fds.begin(); i != fds.end(); + ++i) + ignore_result((*i)->release()); base::GlobalDescriptors::GetInstance()->Reset(mapping); // Reset the process-wide command line to our new command line. @@ -510,18 +514,14 @@ base::ProcessId Zygote::ReadArgsAndFork(const Pickle& pickle, bool Zygote::HandleForkRequest(int fd, const Pickle& pickle, PickleIterator iter, - std::vector& fds) { + ScopedVector fds) { std::string uma_name; int uma_sample; int uma_boundary_value; - base::ProcessId child_pid = ReadArgsAndFork(pickle, iter, fds, - &uma_name, &uma_sample, - &uma_boundary_value); + base::ProcessId child_pid = ReadArgsAndFork( + pickle, iter, fds.Pass(), &uma_name, &uma_sample, &uma_boundary_value); if (child_pid == 0) return true; - for (std::vector::const_iterator - i = fds.begin(); i != fds.end(); ++i) - close(*i); if (uma_name.empty()) { // There is no UMA report from this particular fork. // Use the initial UMA report if any, and clear that record for next time. diff --git a/content/zygote/zygote_linux.h b/content/zygote/zygote_linux.h index 140dfa6735aa..4aa2f03d3bfb 100644 --- a/content/zygote/zygote_linux.h +++ b/content/zygote/zygote_linux.h @@ -6,9 +6,10 @@ #define CONTENT_ZYGOTE_ZYGOTE_H_ #include -#include #include "base/containers/small_map.h" +#include "base/files/scoped_file.h" +#include "base/memory/scoped_vector.h" #include "base/posix/global_descriptors.h" #include "base/process/kill.h" #include "base/process/process.h" @@ -89,7 +90,7 @@ class Zygote { // process and the child process ID to the parent process, like fork(). base::ProcessId ReadArgsAndFork(const Pickle& pickle, PickleIterator iter, - std::vector& fds, + ScopedVector fds, std::string* uma_name, int* uma_sample, int* uma_boundary_value); @@ -101,7 +102,7 @@ class Zygote { bool HandleForkRequest(int fd, const Pickle& pickle, PickleIterator iter, - std::vector& fds); + ScopedVector fds); bool HandleGetSandboxStatus(int fd, const Pickle& pickle, diff --git a/sandbox/linux/services/broker_process.cc b/sandbox/linux/services/broker_process.cc index e91df52b709f..ef916f223a86 100644 --- a/sandbox/linux/services/broker_process.cc +++ b/sandbox/linux/services/broker_process.cc @@ -22,6 +22,7 @@ #include "base/compiler_specific.h" #include "base/files/scoped_file.h" #include "base/logging.h" +#include "base/memory/scoped_vector.h" #include "base/pickle.h" #include "base/posix/eintr_wrapper.h" #include "base/posix/unix_domain_socket_linux.h" @@ -320,8 +321,7 @@ int BrokerProcess::PathAndFlagsSyscall(enum IPCCommands syscall_type, // that we will then close. // A request should start with an int that will be used as the command type. bool BrokerProcess::HandleRequest() const { - - std::vector fds; + ScopedVector fds; char buf[kMaxMessageLength]; errno = 0; const ssize_t msg_len = UnixDomainSocket::RecvMsg(ipc_socketpair_, buf, @@ -334,17 +334,13 @@ bool BrokerProcess::HandleRequest() const { // The parent should send exactly one file descriptor, on which we // will write the reply. - if (msg_len < 0 || fds.size() != 1 || fds.at(0) < 0) { + // TODO(mdempsky): ScopedVector doesn't have 'at()', only 'operator[]'. + if (msg_len < 0 || fds.size() != 1 || fds[0]->get() < 0) { PLOG(ERROR) << "Error reading message from the client"; - // The client could try to DoS us by sending more file descriptors, so - // make sure we close them. - for (std::vector::iterator it = fds.begin(); it != fds.end(); ++it) { - PCHECK(0 == IGNORE_EINTR(close(*it))); - } return false; } - base::ScopedFD temporary_ipc(fds.at(0)); + base::ScopedFD temporary_ipc(fds[0]->Pass()); Pickle pickle(buf, msg_len); PickleIterator iter(pickle); diff --git a/sandbox/linux/services/unix_domain_socket_unittest.cc b/sandbox/linux/services/unix_domain_socket_unittest.cc index ed9c401cd208..17208a829ddc 100644 --- a/sandbox/linux/services/unix_domain_socket_unittest.cc +++ b/sandbox/linux/services/unix_domain_socket_unittest.cc @@ -14,6 +14,7 @@ #include "base/files/scoped_file.h" #include "base/logging.h" +#include "base/memory/scoped_vector.h" #include "base/posix/eintr_wrapper.h" #include "base/posix/unix_domain_socket_linux.h" #include "base/process/process_handle.h" @@ -94,15 +95,14 @@ void RecvHello(int fd, // Extra receiving buffer space to make sure we really received only // sizeof(kHello) bytes and it wasn't just truncated to fit the buffer. char buf[sizeof(kHello) + 1]; - std::vector message_fds; + ScopedVector message_fds; ssize_t n = UnixDomainSocket::RecvMsgWithPid( fd, buf, sizeof(buf), &message_fds, sender_pid); CHECK_EQ(sizeof(kHello), static_cast(n)); CHECK_EQ(0, memcmp(buf, kHello, sizeof(kHello))); CHECK_EQ(1U, message_fds.size()); - base::ScopedFD message_fd(message_fds[0]); if (write_pipe) - write_pipe->swap(message_fd); + write_pipe->swap(*message_fds[0]); } // Check that receiving PIDs works across a fork(). -- 2.11.4.GIT