summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorderat@chromium.org <derat@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2012-10-12 19:59:39 +0000
committerderat@chromium.org <derat@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2012-10-12 19:59:39 +0000
commit375a8463858537aa295b904a71073784dfa4de6a (patch)
treeffda46a1d09ae53d657c12ea4b22f194b5f3f2fa
parentffabb1ea75e662308da91e916a0ffc922525aa18 (diff)
downloadchromium_src-375a8463858537aa295b904a71073784dfa4de6a.zip
chromium_src-375a8463858537aa295b904a71073784dfa4de6a.tar.gz
chromium_src-375a8463858537aa295b904a71073784dfa4de6a.tar.bz2
linux: Crash browser on too-big messages to zygote.
This adds CHECKs to the browser if it attempts to send a message to the zygote that exceeds the maximum message size (which causes an EMSGSIZE error in the zygote) or that contains too many file descriptors. I'm hoping that this will help make the source of the problem more apparent when we hit the message size limit, which we appear to have done multiple times (it was originally 1 KB and is now 2 KB). BUG=154409 Review URL: https://chromiumcodereview.appspot.com/11108019 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@161645 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r--base/posix/unix_domain_socket.cc6
-rw-r--r--base/posix/unix_domain_socket.h5
-rw-r--r--content/browser/zygote_host/zygote_host_impl_linux.cc36
-rw-r--r--content/browser/zygote_host/zygote_host_impl_linux.h7
-rw-r--r--content/common/zygote_commands_linux.h3
-rw-r--r--content/zygote/zygote_linux.cc3
6 files changed, 40 insertions, 20 deletions
diff --git a/base/posix/unix_domain_socket.cc b/base/posix/unix_domain_socket.cc
index da8be63..bd11292 100644
--- a/base/posix/unix_domain_socket.cc
+++ b/base/posix/unix_domain_socket.cc
@@ -14,6 +14,8 @@
#include "base/pickle.h"
#include "base/stl_util.h"
+const size_t UnixDomainSocket::kMaxFileDescriptors = 16;
+
// static
bool UnixDomainSocket::SendMsg(int fd,
const void* buf,
@@ -52,8 +54,6 @@ ssize_t UnixDomainSocket::RecvMsg(int fd,
void* buf,
size_t length,
std::vector<int>* fds) {
- static const unsigned kMaxDescriptors = 16;
-
fds->clear();
struct msghdr msg;
@@ -62,7 +62,7 @@ ssize_t UnixDomainSocket::RecvMsg(int fd,
msg.msg_iov = &iov;
msg.msg_iovlen = 1;
- char control_buffer[CMSG_SPACE(sizeof(int) * kMaxDescriptors)];
+ char control_buffer[CMSG_SPACE(sizeof(int) * kMaxFileDescriptors)];
msg.msg_control = control_buffer;
msg.msg_controllen = sizeof(control_buffer);
diff --git a/base/posix/unix_domain_socket.h b/base/posix/unix_domain_socket.h
index d08d170..cb2a0b8 100644
--- a/base/posix/unix_domain_socket.h
+++ b/base/posix/unix_domain_socket.h
@@ -15,6 +15,9 @@ class Pickle;
class BASE_EXPORT UnixDomainSocket {
public:
+ // Maximum number of file descriptors that can be read by RecvMsg().
+ static const size_t kMaxFileDescriptors;
+
// Use sendmsg to write the given msg and include a vector of file
// descriptors. Returns true if successful.
static bool SendMsg(int fd,
@@ -23,7 +26,7 @@ class BASE_EXPORT UnixDomainSocket {
const std::vector<int>& fds);
// Use recvmsg to read a message and an array of file descriptors. Returns
- // -1 on failure. Note: will read, at most, 16 descriptors.
+ // -1 on failure. Note: will read, at most, |kMaxFileDescriptors| descriptors.
static ssize_t RecvMsg(int fd,
void* msg,
size_t length,
diff --git a/content/browser/zygote_host/zygote_host_impl_linux.cc b/content/browser/zygote_host/zygote_host_impl_linux.cc
index d4b4fb2..647d902 100644
--- a/content/browser/zygote_host/zygote_host_impl_linux.cc
+++ b/content/browser/zygote_host/zygote_host_impl_linux.cc
@@ -20,7 +20,6 @@
#include "base/memory/scoped_ptr.h"
#include "base/metrics/histogram.h"
#include "base/path_service.h"
-#include "base/pickle.h"
#include "base/posix/unix_domain_socket.h"
#include "base/process_util.h"
#include "base/string_number_conversions.h"
@@ -161,8 +160,7 @@ void ZygoteHostImpl::Init(const std::string& sandbox_cmd) {
if (using_suid_sandbox_) {
dummy_fd = socket(PF_UNIX, SOCK_DGRAM, 0);
CHECK(dummy_fd >= 0);
- fds_to_map.push_back(std::make_pair(dummy_fd,
- content::kZygoteIdFd));
+ fds_to_map.push_back(std::make_pair(dummy_fd, content::kZygoteIdFd));
}
base::ProcessHandle process = -1;
@@ -217,13 +215,26 @@ void ZygoteHostImpl::Init(const std::string& sandbox_cmd) {
Pickle pickle;
pickle.WriteInt(content::kZygoteCommandGetSandboxStatus);
- std::vector<int> empty_fds;
- if (!UnixDomainSocket::SendMsg(control_fd_, pickle.data(), pickle.size(),
- empty_fds))
+ if (!SendMessage(pickle, NULL))
LOG(FATAL) << "Cannot communicate with zygote";
// We don't wait for the reply. We'll read it in ReadReply.
}
+bool ZygoteHostImpl::SendMessage(const Pickle& data,
+ const std::vector<int>* fds) {
+ CHECK(data.size() <= content::kZygoteMaxMessageLength)
+ << "Trying to send too-large message to zygote (sending " << data.size()
+ << " bytes, max is " << content::kZygoteMaxMessageLength << ")";
+ CHECK(!fds || fds->size() <= UnixDomainSocket::kMaxFileDescriptors)
+ << "Trying to send message with too many file descriptors to zygote "
+ << "(sending " << fds->size() << ", max is "
+ << UnixDomainSocket::kMaxFileDescriptors << ")";
+
+ return UnixDomainSocket::SendMsg(control_fd_,
+ data.data(), data.size(),
+ fds ? *fds : std::vector<int>());
+}
+
ssize_t ZygoteHostImpl::ReadReply(void* buf, size_t buf_len) {
// At startup we send a kZygoteCommandGetSandboxStatus request to the zygote,
// but don't wait for the reply. Thus, the first time that we read from the
@@ -276,8 +287,7 @@ pid_t ZygoteHostImpl::ForkRequest(
pid_t pid;
{
base::AutoLock lock(control_lock_);
- if (!UnixDomainSocket::SendMsg(control_fd_, pickle.data(), pickle.size(),
- fds))
+ if (!SendMessage(pickle, &fds))
return base::kNullProcessHandle;
// Read the reply, which pickles the PID and an optional UMA enumeration.
@@ -438,9 +448,8 @@ void ZygoteHostImpl::EnsureProcessTerminated(pid_t process) {
pickle.WriteInt(content::kZygoteCommandReap);
pickle.WriteInt(process);
-
- if (HANDLE_EINTR(write(control_fd_, pickle.data(), pickle.size())) < 0)
- PLOG(ERROR) << "write";
+ if (!SendMessage(pickle, NULL))
+ LOG(ERROR) << "Failed to send Reap message to zygote";
}
base::TerminationStatus ZygoteHostImpl::GetTerminationStatus(
@@ -460,9 +469,8 @@ base::TerminationStatus ZygoteHostImpl::GetTerminationStatus(
ssize_t len;
{
base::AutoLock lock(control_lock_);
- if (HANDLE_EINTR(write(control_fd_, pickle.data(), pickle.size())) < 0)
- PLOG(ERROR) << "write";
-
+ if (!SendMessage(pickle, NULL))
+ LOG(ERROR) << "Failed to send GetTerminationStatus message to zygote";
len = ReadReply(buf, sizeof(buf));
}
diff --git a/content/browser/zygote_host/zygote_host_impl_linux.h b/content/browser/zygote_host/zygote_host_impl_linux.h
index 460abc5..8548a2b 100644
--- a/content/browser/zygote_host/zygote_host_impl_linux.h
+++ b/content/browser/zygote_host/zygote_host_impl_linux.h
@@ -8,6 +8,7 @@
#include <string>
#include <vector>
+#include "base/pickle.h"
#include "base/process_util.h"
#include "base/synchronization/lock.h"
#include "content/public/browser/file_descriptor_info.h"
@@ -47,9 +48,15 @@ class CONTENT_EXPORT ZygoteHostImpl : public content::ZygoteHost {
private:
friend struct DefaultSingletonTraits<ZygoteHostImpl>;
+
ZygoteHostImpl();
virtual ~ZygoteHostImpl();
+ // Sends |data| to the zygote via |control_fd_|. If |fds| is non-NULL, the
+ // included file descriptors will also be passed. The caller is responsible
+ // for acquiring |control_lock_|.
+ bool SendMessage(const Pickle& data, const std::vector<int>* fds);
+
ssize_t ReadReply(void* buf, size_t buflen);
int control_fd_; // the socket to the zygote
diff --git a/content/common/zygote_commands_linux.h b/content/common/zygote_commands_linux.h
index c46eec4..f718504 100644
--- a/content/common/zygote_commands_linux.h
+++ b/content/common/zygote_commands_linux.h
@@ -11,6 +11,9 @@ namespace content {
// is ready to go.
static const char kZygoteHelloMessage[] = "ZYGOTE_OK";
+// Maximum allowable length for messages sent to the zygote.
+const size_t kZygoteMaxMessageLength = 2048;
+
// File descriptors initialized by the Zygote Host
const int kZygoteSocketPairFd = 3;
const int kZygoteRendererSocketFd = 5;
diff --git a/content/zygote/zygote_linux.cc b/content/zygote/zygote_linux.cc
index 101ea7f..aad5697 100644
--- a/content/zygote/zygote_linux.cc
+++ b/content/zygote/zygote_linux.cc
@@ -128,8 +128,7 @@ bool Zygote::UsingSUIDSandbox() const {
bool Zygote::HandleRequestFromBrowser(int fd) {
std::vector<int> fds;
- static const unsigned kMaxMessageLength = 2048;
- char buf[kMaxMessageLength];
+ char buf[kZygoteMaxMessageLength];
const ssize_t len = UnixDomainSocket::RecvMsg(fd, buf, sizeof(buf), &fds);
if (len == 0 || (len == -1 && errno == ECONNRESET)) {