Skip to content

Commit

Permalink
Change UnixDomainSocket::RecvMsg to return ScopedVector<base::ScopedFD>
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
mdempsky@chromium.org committed Apr 30, 2014
1 parent 12fad44 commit 8feaa67
Show file tree
Hide file tree
Showing 12 changed files with 189 additions and 160 deletions.
66 changes: 40 additions & 26 deletions base/posix/unix_domain_socket_linux.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,29 @@
#include <sys/uio.h>
#include <unistd.h>

#include <vector>

#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;
Expand Down Expand Up @@ -63,15 +79,15 @@ bool UnixDomainSocket::SendMsg(int fd,
ssize_t UnixDomainSocket::RecvMsg(int fd,
void* buf,
size_t length,
std::vector<int>* fds) {
ScopedVector<base::ScopedFD>* fds) {
return UnixDomainSocket::RecvMsgWithPid(fd, buf, length, fds, NULL);
}

// static
ssize_t UnixDomainSocket::RecvMsgWithPid(int fd,
void* buf,
size_t length,
std::vector<int>* fds,
ScopedVector<base::ScopedFD>* fds,
base::ProcessId* pid) {
return UnixDomainSocket::RecvMsgWithFlags(fd, buf, length, 0, fds, pid);
}
Expand All @@ -81,7 +97,7 @@ ssize_t UnixDomainSocket::RecvMsgWithFlags(int fd,
void* buf,
size_t length,
int flags,
std::vector<int>* fds,
ScopedVector<base::ScopedFD>* fds,
base::ProcessId* out_pid) {
fds->clear();

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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<int> 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<int> 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<base::ScopedFD> 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<int>::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;
}
8 changes: 5 additions & 3 deletions base/posix/unix_domain_socket_linux.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
#include <vector>

#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;
Expand All @@ -36,7 +38,7 @@ class BASE_EXPORT UnixDomainSocket {
static ssize_t RecvMsg(int fd,
void* msg,
size_t length,
std::vector<int>* fds);
ScopedVector<base::ScopedFD>* 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
Expand All @@ -45,7 +47,7 @@ class BASE_EXPORT UnixDomainSocket {
static ssize_t RecvMsgWithPid(int fd,
void* msg,
size_t length,
std::vector<int>* fds,
ScopedVector<base::ScopedFD>* fds,
base::ProcessId* pid);

// Perform a sendmsg/recvmsg pair.
Expand Down Expand Up @@ -86,7 +88,7 @@ class BASE_EXPORT UnixDomainSocket {
void* msg,
size_t length,
int flags,
std::vector<int>* fds,
ScopedVector<base::ScopedFD>* fds,
base::ProcessId* pid);
};

Expand Down
21 changes: 10 additions & 11 deletions base/posix/unix_domain_socket_linux_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -38,15 +39,15 @@ TEST(UnixDomainSocketTest, SendRecvMsgAbortOnReplyFDClose) {
request));

// Receive the message.
std::vector<int> message_fds;
ScopedVector<base::ScopedFD> message_fds;
uint8_t buffer[16];
ASSERT_EQ(static_cast<int>(request.size()),
UnixDomainSocket::RecvMsg(fds[0], buffer, sizeof(buffer),
&message_fds));
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);
Expand Down Expand Up @@ -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<int> fd_vec;
ScopedVector<base::ScopedFD> fd_vec;
const ssize_t nread = UnixDomainSocket::RecvMsgWithPid(
recv_sock.get(), buf, sizeof(buf), &fd_vec, &sender_pid);
ASSERT_EQ(sizeof(kHello), static_cast<size_t>(nread));
Expand All @@ -114,23 +115,21 @@ TEST(UnixDomainSocketTest, RecvPidWithMaxDescriptors) {
ASSERT_TRUE(UnixDomainSocket::EnableReceiveProcessId(recv_sock.get()));

static const char kHello[] = "hello";
std::vector<int> fd_vec(UnixDomainSocket::kMaxFileDescriptors,
send_sock.get());
std::vector<int> 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<base::ScopedFD> 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<size_t>(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);
}
Expand Down
64 changes: 36 additions & 28 deletions components/nacl/loader/nacl_helper_linux.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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<int>& child_fds,
void BecomeNaClLoader(base::ScopedFD browser_fd,
const NaClLoaderSystemInfo& system_info,
bool uses_nonsfi_mode,
nacl::NaClSandbox* nacl_sandbox) {
Expand Down Expand Up @@ -94,9 +96,8 @@ void BecomeNaClLoader(const std::vector<int>& 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;
Expand All @@ -108,22 +109,21 @@ void BecomeNaClLoader(const std::vector<int>& child_fds,
}

// Start the NaCl loader in a child created by the NaCl loader Zygote.
void ChildNaClLoaderInit(const std::vector<int>& child_fds,
void ChildNaClLoaderInit(ScopedVector<base::ScopedFD> 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<size_t>(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
Expand All @@ -139,12 +139,13 @@ void ChildNaClLoaderInit(const std::vector<int>& 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";
}
Expand All @@ -154,7 +155,7 @@ void ChildNaClLoaderInit(const std::vector<int>& 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<int>& child_fds,
bool HandleForkRequest(ScopedVector<base::ScopedFD> child_fds,
const NaClLoaderSystemInfo& system_info,
nacl::NaClSandbox* nacl_sandbox,
PickleIterator* input_iter,
Expand Down Expand Up @@ -184,18 +185,18 @@ bool HandleForkRequest(const std::vector<int>& 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.
Expand Down Expand Up @@ -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<int>& attached_fds,
ScopedVector<base::ScopedFD> attached_fds,
const NaClLoaderSystemInfo& system_info,
nacl::NaClSandbox* nacl_sandbox,
PickleIterator* input_iter) {
Expand All @@ -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 =
Expand All @@ -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<int> fds;
ScopedVector<base::ScopedFD> fds;
char buf[kNaClMaxIPCMessageLength];
const ssize_t msglen = UnixDomainSocket::RecvMsg(zygote_ipc_fd,
&buf, sizeof(buf), &fds);
Expand All @@ -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";
Expand Down
Loading

0 comments on commit 8feaa67

Please sign in to comment.