ipc: remove IPC_USES_READWRITE

The Linux sandbox performance implications that made using
recvmsg()/sendmsg() slower than read()/write() have not applied for a
long time, since we switched from seccomp (v1) to seccomp-bpf.

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

Cr-Commit-Position: refs/heads/master@{#330414}

2 files changed, 5 insertions, 165 deletions

diff --git a/ipc/ipc_channel_posix.cc b/ipc/ipc_channel_posix.cc
index aac7e79..a76ef8a 100644
--- a/ipc/ipc_channel_posix.cc
+++ b/ipc/ipc_channel_posix.cc
@@ -247,19 +247,6 @@ bool ChannelPosix::CreatePipe(
   if (channel_handle.socket.fd != -1) {
     // Case 1 from comment above.
     local_pipe.reset(channel_handle.socket.fd);
-#if defined(IPC_USES_READWRITE)
-    // Test the socket passed into us to make sure it is nonblocking.
-    // We don't want to call read/write on a blocking socket.
-    int value = fcntl(local_pipe.get(), F_GETFL);
-    if (value == -1) {
-      PLOG(ERROR) << "fcntl(F_GETFL) " << pipe_name_;
-      return false;
-    }
-    if (!(value & O_NONBLOCK)) {
-      LOG(ERROR) << "Socket " << pipe_name_ << " must be O_NONBLOCK";
-      return false;
-    }
-#endif   // IPC_USES_READWRITE
   } else if (mode_ & MODE_NAMED_FLAG) {
 #if defined(OS_NACL_NONSFI)
     LOG(FATAL)
@@ -333,20 +320,6 @@ bool ChannelPosix::CreatePipe(
     }
   }
 
-#if defined(IPC_USES_READWRITE)
-  // Create a dedicated socketpair() for exchanging file descriptors.
-  // See comments for IPC_USES_READWRITE for details.
-  if (mode_ & MODE_CLIENT_FLAG) {
-    int fd_pipe_fd = 1, remote_fd_pipe_fd = -1;
-    if (!SocketPair(&fd_pipe_fd, &remote_fd_pipe_fd)) {
-      return false;
-    }
-
-    fd_pipe_.reset(fd_pipe_fd);
-    remote_fd_pipe_.reset(remote_fd_pipe_fd);
-  }
-#endif  // IPC_USES_READWRITE
-
   if ((mode_ & MODE_SERVER_FLAG) && (mode_ & MODE_NAMED_FLAG)) {
 #if defined(OS_NACL_NONSFI)
     LOG(FATAL) << "IPC channels in nacl_helper_nonsfi "
@@ -466,40 +439,11 @@ bool ChannelPosix::ProcessOutgoingMessages() {
       // DCHECK_LE above already checks that
       // num_fds < kMaxDescriptorsPerMessage so no danger of overflow.
       msg->header()->num_fds = static_cast<uint16>(num_fds);
-
-#if defined(IPC_USES_READWRITE)
-      if (!IsHelloMessage(*msg)) {
-        // Only the Hello message sends the file descriptor with the message.
-        // Subsequently, we can send file descriptors on the dedicated
-        // fd_pipe_ which makes Seccomp sandbox operation more efficient.
-        struct iovec fd_pipe_iov = { const_cast<char *>(""), 1 };
-        msgh.msg_iov = &fd_pipe_iov;
-        fd_written = fd_pipe_.get();
-        bytes_written =
-            HANDLE_EINTR(sendmsg(fd_pipe_.get(), &msgh, MSG_DONTWAIT));
-        msgh.msg_iov = &iov;
-        msgh.msg_controllen = 0;
-        if (bytes_written > 0) {
-          CloseFileDescriptors(msg);
-        }
-      }
-#endif  // IPC_USES_READWRITE
     }
 
     if (bytes_written == 1) {
       fd_written = pipe_.get();
-#if defined(IPC_USES_READWRITE)
-      if ((mode_ & MODE_CLIENT_FLAG) && IsHelloMessage(*msg)) {
-        DCHECK_EQ(msg->attachment_set()->size(), 1U);
-      }
-      if (!msgh.msg_controllen) {
-        bytes_written =
-            HANDLE_EINTR(write(pipe_.get(), out_bytes, amt_to_write));
-      } else
-#endif  // IPC_USES_READWRITE
-      {
-        bytes_written = HANDLE_EINTR(sendmsg(pipe_.get(), &msgh, MSG_DONTWAIT));
-      }
+      bytes_written = HANDLE_EINTR(sendmsg(pipe_.get(), &msgh, MSG_DONTWAIT));
     }
     if (bytes_written > 0)
       CloseFileDescriptors(msg);
@@ -616,10 +560,6 @@ void ChannelPosix::ResetToAcceptingConnectionState() {
   read_watcher_.StopWatchingFileDescriptor();
   write_watcher_.StopWatchingFileDescriptor();
   ResetSafely(&pipe_);
-#if defined(IPC_USES_READWRITE)
-  fd_pipe_.reset();
-  remote_fd_pipe_.reset();
-#endif  // IPC_USES_READWRITE
 
   while (!output_queue_.empty()) {
     Message* m = output_queue_.front();
@@ -798,16 +738,6 @@ void ChannelPosix::QueueHelloMessage() {
   if (!msg->WriteInt(GetHelloMessageProcId())) {
     NOTREACHED() << "Unable to pickle hello message proc id";
   }
-#if defined(IPC_USES_READWRITE)
-  scoped_ptr<Message> hello;
-  if (remote_fd_pipe_.is_valid()) {
-    if (!msg->WriteAttachment(
-            new internal::PlatformFileAttachment(remote_fd_pipe_.get()))) {
-      NOTREACHED() << "Unable to pickle hello message file descriptors";
-    }
-    DCHECK_EQ(msg->attachment_set()->size(), 1U);
-  }
-#endif  // IPC_USES_READWRITE
   output_queue_.push(msg.release());
 }
 
@@ -828,16 +758,9 @@ ChannelPosix::ReadState ChannelPosix::ReadData(
 
   // recvmsg() returns 0 if the connection has closed or EAGAIN if no data
   // is waiting on the pipe.
-#if defined(IPC_USES_READWRITE)
-  if (fd_pipe_.is_valid()) {
-    *bytes_read = HANDLE_EINTR(read(pipe_.get(), buffer, buffer_len));
-    msg.msg_controllen = 0;
-  } else
-#endif  // IPC_USES_READWRITE
-  {
-    msg.msg_controllen = sizeof(input_cmsg_buf_);
-    *bytes_read = HANDLE_EINTR(recvmsg(pipe_.get(), &msg, MSG_DONTWAIT));
-  }
+  msg.msg_controllen = sizeof(input_cmsg_buf_);
+  *bytes_read = HANDLE_EINTR(recvmsg(pipe_.get(), &msg, MSG_DONTWAIT));
+
   if (*bytes_read < 0) {
     if (errno == EAGAIN) {
       return READ_PENDING;
@@ -868,28 +791,6 @@ ChannelPosix::ReadState ChannelPosix::ReadData(
   return READ_SUCCEEDED;
 }
 
-#if defined(IPC_USES_READWRITE)
-bool ChannelPosix::ReadFileDescriptorsFromFDPipe() {
-  char dummy;
-  struct iovec fd_pipe_iov = { &dummy, 1 };
-
-  struct msghdr msg = { 0 };
-  msg.msg_iov = &fd_pipe_iov;
-  msg.msg_iovlen = 1;
-  msg.msg_control = input_cmsg_buf_;
-  msg.msg_controllen = sizeof(input_cmsg_buf_);
-  ssize_t bytes_received =
-      HANDLE_EINTR(recvmsg(fd_pipe_.get(), &msg, MSG_DONTWAIT));
-
-  if (bytes_received != 1)
-    return true;  // No message waiting.
-
-  if (!ExtractFileDescriptorsFromMsghdr(&msg))
-    return false;
-  return true;
-}
-#endif
-
 // On Posix, we need to fix up the file descriptors before the input message
 // is dispatched.
 //
@@ -905,12 +806,7 @@ bool ChannelPosix::WillDispatchInputMessage(Message* msg) {
   if (header_fds > input_fds_.size()) {
     // The message has been completely received, but we didn't get
     // enough file descriptors.
-#if defined(IPC_USES_READWRITE)
-    if (!ReadFileDescriptorsFromFDPipe())
-      return false;
-    if (header_fds > input_fds_.size())
-#endif  // IPC_USES_READWRITE
-      error = "Message needs unreceived descriptors";
+    error = "Message needs unreceived descriptors";
   }
 
   if (header_fds > MessageAttachmentSet::kMaxDescriptorsPerMessage)
@@ -1017,19 +913,6 @@ void ChannelPosix::HandleInternalMessage(const Message& msg) {
       if (!iter.ReadInt(&pid))
         NOTREACHED();
 
-#if defined(IPC_USES_READWRITE)
-      if (mode_ & MODE_SERVER_FLAG) {
-        // With IPC_USES_READWRITE, the Hello message from the client to the
-        // server also contains the fd_pipe_, which  will be used for all
-        // subsequent file descriptor passing.
-        DCHECK_EQ(msg.attachment_set()->size(), 1U);
-        scoped_refptr<MessageAttachment> attachment;
-        if (!msg.ReadAttachment(&iter, &attachment)) {
-          NOTREACHED();
-        }
-        fd_pipe_.reset(attachment->TakePlatformFile());
-      }
-#endif  // IPC_USES_READWRITE
       peer_pid_ = pid;
       listener()->OnChannelConnected(pid);
       break;
@@ -1129,7 +1012,6 @@ std::string Channel::GenerateVerifiedChannelID(const std::string& prefix) {
   return id.append(GenerateUniqueRandomChannelID());
 }
 
-
 bool Channel::IsNamedServerInitialized(
     const std::string& channel_id) {
   return ChannelPosix::IsNamedServerInitialized(channel_id);
diff --git a/ipc/ipc_channel_posix.h b/ipc/ipc_channel_posix.h
index a65283d..4edb6a0 100644
--- a/ipc/ipc_channel_posix.h
+++ b/ipc/ipc_channel_posix.h
@@ -20,34 +20,6 @@
 #include "ipc/ipc_channel_reader.h"
 #include "ipc/ipc_message_attachment_set.h"
 
-#if !defined(OS_MACOSX)
-// On Linux, the seccomp sandbox makes it very expensive to call
-// recvmsg() and sendmsg(). The restriction on calling read() and write(), which
-// are cheap, is that we can't pass file descriptors over them.
-//
-// As we cannot anticipate when the sender will provide us with file
-// descriptors, we have to make the decision about whether we call read() or
-// recvmsg() before we actually make the call. The easiest option is to
-// create a dedicated socketpair() for exchanging file descriptors. Any file
-// descriptors are split out of a message, with the non-file-descriptor payload
-// going over the normal connection, and the file descriptors being sent
-// separately over the other channel. When read()ing from a channel, we'll
-// notice if the message was supposed to have come with file descriptors and
-// use recvmsg on the other socketpair to retrieve them and combine them
-// back with the rest of the message.
-//
-// Mac can also run in IPC_USES_READWRITE mode if necessary, but at this time
-// doesn't take a performance hit from recvmsg and sendmsg, so it doesn't
-// make sense to waste resources on having the separate dedicated socketpair.
-// It is however useful for debugging between Linux and Mac to be able to turn
-// this switch 'on' on the Mac as well.
-//
-// The HELLO message from the client to the server is always sent using
-// sendmsg because it will contain the file descriptor that the server
-// needs to send file descriptors in later messages.
-#define IPC_USES_READWRITE 1
-#endif
-
 namespace IPC {
 
 class IPC_EXPORT ChannelPosix : public Channel,
@@ -107,14 +79,6 @@ class IPC_EXPORT ChannelPosix : public Channel,
   bool DidEmptyInputBuffers() override;
   void HandleInternalMessage(const Message& msg) override;
 
-#if defined(IPC_USES_READWRITE)
-  // Reads the next message from the fd_pipe_ and appends them to the
-  // input_fds_ queue. Returns false if there was a message receiving error.
-  // True means there was a message and it was processed properly, or there was
-  // no messages.
-  bool ReadFileDescriptorsFromFDPipe();
-#endif
-
   // Finds the set of file descriptors in the given message.  On success,
   // appends the descriptors to the input_fds_ member and returns true
   //
@@ -161,12 +125,6 @@ class IPC_EXPORT ChannelPosix : public Channel,
   base::ScopedFD client_pipe_;
   mutable base::Lock client_pipe_lock_;  // Lock that protects |client_pipe_|.
 
-#if defined(IPC_USES_READWRITE)
-  // Linux/BSD use a dedicated socketpair() for passing file descriptors.
-  base::ScopedFD fd_pipe_;
-  base::ScopedFD remote_fd_pipe_;
-#endif
-
   // The "name" of our pipe.  On Windows this is the global identifier for
   // the pipe.  On POSIX it's used as a key in a local map of file descriptors.
   std::string pipe_name_;