Bug 1767514 - Part 1: Decouple the IPC::Message max handle count and the number of FDs supported by IPC::Channel, r=ipc-reviewers,jld
☠☠ backed out by f138b7007901 ☠ ☠
authorNika Layzell <nika@thelayzells.com>
Fri, 06 May 2022 20:02:36 +0000
changeset 616572 62befea29e738b6d96fdeae6fcfa16b54859a0fc
parent 616571 cdba1d6ffd463aecd18e4f285ee4b64da2ba0326
child 616573 53ebc3f919ba28cd76e19c9440896d4deab257e2
push id162414
push usernlayzell@mozilla.com
push dateFri, 06 May 2022 20:39:15 +0000
treeherderautoland@53ebc3f919ba [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersipc-reviewers, jld
bugs1767514
milestone102.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1767514 - Part 1: Decouple the IPC::Message max handle count and the number of FDs supported by IPC::Channel, r=ipc-reviewers,jld This is done by splitting messages with large numbers of handles into multiple `sendmsg` calls, each of which contains less than the maximum number of transferred handles per-message, and stitching the message back together on the receiving side. Most of the work on the receiving side was already handled by the IPC::Channel code, so the work required was only to ensure we could split the handle list up when sending. Differential Revision: https://phabricator.services.mozilla.com/D145391
ipc/chromium/src/chrome/common/ipc_channel_posix.cc
ipc/chromium/src/chrome/common/ipc_channel_posix.h
--- a/ipc/chromium/src/chrome/common/ipc_channel_posix.cc
+++ b/ipc/chromium/src/chrome/common/ipc_channel_posix.cc
@@ -190,21 +190,23 @@ bool Channel::ChannelImpl::PipeBufHasSpa
   return pipe_buf_len_ == 0 ||
          static_cast<size_t>(pipe_buf_len_) > already_written;
 }
 
 void Channel::ChannelImpl::Init(Mode mode, Listener* listener) {
   // Verify that we fit in a "quantum-spaced" jemalloc bucket.
   static_assert(sizeof(*this) <= 512, "Exceeded expected size class");
 
-  DCHECK(kControlBufferHeaderSize >= CMSG_SPACE(0));
+  MOZ_RELEASE_ASSERT(kControlBufferHeaderSize >= CMSG_SPACE(0));
+  MOZ_RELEASE_ASSERT(kControlBufferSize >=
+                     CMSG_SPACE(sizeof(int) * kControlBufferMaxFds));
 
   mode_ = mode;
   is_blocked_on_write_ = false;
-  partial_write_iter_.reset();
+  partial_write_.reset();
   input_buf_offset_ = 0;
   input_buf_ = mozilla::MakeUnique<char[]>(Channel::kReadBufferSize);
   input_cmsg_buf_ = mozilla::MakeUnique<char[]>(kControlBufferSize);
   server_listen_pipe_ = -1;
   SetPipe(-1);
   client_pipe_ = -1;
   listener_ = listener;
   waiting_connect_ = true;
@@ -566,118 +568,120 @@ bool Channel::ChannelImpl::ProcessOutgoi
   while (!output_queue_.IsEmpty()) {
 #ifdef FUZZING
     mozilla::ipc::Faulty::instance().MaybeCollectAndClosePipe(pipe_);
 #endif
     Message* msg = output_queue_.FirstElement().get();
 
     struct msghdr msgh = {0};
 
-    static const int tmp =
-        CMSG_SPACE(sizeof(int[IPC::Message::MAX_DESCRIPTORS_PER_MESSAGE]));
-    char buf[tmp];
+    char cmsgBuf[kControlBufferSize];
 
-    if (partial_write_iter_.isNothing()) {
+    if (partial_write_.isNothing()) {
 #if defined(OS_MACOSX)
       if (!TransferMachPorts(*msg)) {
         return false;
       }
 #endif
       Pickle::BufferList::IterImpl iter(msg->Buffers());
       MOZ_DIAGNOSTIC_ASSERT(!iter.Done(), "empty message");
-      partial_write_iter_.emplace(iter);
+      partial_write_.emplace(PartialWrite{iter, msg->attached_handles_});
     }
 
-    if (partial_write_iter_.ref().Done()) {
-      MOZ_DIAGNOSTIC_ASSERT(false, "partial_write_iter_ should not be null");
+    if (partial_write_->iter_.Done()) {
+      MOZ_DIAGNOSTIC_ASSERT(false, "partial_write_->iter_ should not be done");
       // report a send error to our caller, which will close the channel.
       return false;
     }
 
-    if (partial_write_iter_.value().Data() == msg->Buffers().Start()) {
+    if (partial_write_->iter_.Data() == msg->Buffers().Start()) {
       AddIPCProfilerMarker(*msg, other_pid_, MessageDirection::eSending,
                            MessagePhase::TransferStart);
 
       if (!msg->attached_handles_.IsEmpty()) {
         // This is the first chunk of a message which has descriptors to send
-        struct cmsghdr* cmsg;
-        const unsigned num_fds = msg->attached_handles_.Length();
-
-        if (num_fds > IPC::Message::MAX_DESCRIPTORS_PER_MESSAGE) {
+        if (msg->attached_handles_.Length() >
+            IPC::Message::MAX_DESCRIPTORS_PER_MESSAGE) {
           MOZ_DIAGNOSTIC_ASSERT(false, "Too many file descriptors!");
           CHROMIUM_LOG(FATAL) << "Too many file descriptors!";
           // This should not be reached.
           return false;
         }
 
-        msgh.msg_control = buf;
-        msgh.msg_controllen = CMSG_SPACE(sizeof(int) * num_fds);
-        cmsg = CMSG_FIRSTHDR(&msgh);
-        cmsg->cmsg_level = SOL_SOCKET;
-        cmsg->cmsg_type = SCM_RIGHTS;
-        cmsg->cmsg_len = CMSG_LEN(sizeof(int) * num_fds);
-        for (unsigned i = 0; i < num_fds; ++i) {
-          reinterpret_cast<int*>(CMSG_DATA(cmsg))[i] =
-              msg->attached_handles_[i].get();
-        }
-        msgh.msg_controllen = cmsg->cmsg_len;
-
-        msg->header()->num_handles = num_fds;
+        msg->header()->num_handles = msg->attached_handles_.Length();
 #if defined(OS_MACOSX)
         msg->set_fd_cookie(++last_pending_fd_id_);
 #endif
       }
     }
 
     struct iovec iov[kMaxIOVecSize];
     size_t iov_count = 0;
     size_t amt_to_write = 0;
 
     // How much of this message have we written so far?
-    Pickle::BufferList::IterImpl iter = partial_write_iter_.value();
+    Pickle::BufferList::IterImpl iter = partial_write_->iter_;
+    auto handles = partial_write_->handles_;
+
+    size_t max_amt_to_write = iter.TotalBytesAvailable(msg->Buffers());
+    if (!handles.IsEmpty()) {
+      // We can only send at most kControlBufferMaxFds files per sendmsg call.
+      const size_t num_fds = std::min(handles.Length(), kControlBufferMaxFds);
 
-    // Store the unwritten part of the first segment to write into the iovec.
-    iov[0].iov_base = const_cast<char*>(iter.Data());
-    iov[0].iov_len = iter.RemainingInSegment();
-    amt_to_write += iov[0].iov_len;
-    iter.Advance(msg->Buffers(), iov[0].iov_len);
-    iov_count++;
+      // Populate the cmsg header for this call
+      msgh.msg_control = cmsgBuf;
+      msgh.msg_controllen = CMSG_LEN(sizeof(int) * num_fds);
+      struct cmsghdr* cmsg = CMSG_FIRSTHDR(&msgh);
+      cmsg->cmsg_level = SOL_SOCKET;
+      cmsg->cmsg_type = SCM_RIGHTS;
+      cmsg->cmsg_len = msgh.msg_controllen;
+      for (size_t i = 0; i < num_fds; ++i) {
+        reinterpret_cast<int*>(CMSG_DATA(cmsg))[i] = handles[i].get();
+      }
+
+      // Update partial_write_ to record which handles were written.
+      auto remaining = handles.From(num_fds);
+      partial_write_->handles_ = remaining;
+
+      // Avoid writing one byte per remaining handle in excess of
+      // kControlBufferMaxFds.  Each handle written will consume a minimum of 4
+      // bytes in the message (to store it's index), so we can depend on there
+      // being enough data to send every handle.
+      MOZ_ASSERT(max_amt_to_write > remaining.Length(),
+                 "must be at least one byte in the message for each handle");
+      max_amt_to_write -= remaining.Length();
+    }
 
     // Store remaining segments to write into iovec.
     //
     // Don't add more than kMaxIOVecSize iovecs so that we avoid
     // OS-dependent limits.  Also, stop adding iovecs if we've already
     // prepared to write at least the full buffer size.
     while (!iter.Done() && iov_count < kMaxIOVecSize &&
-           PipeBufHasSpaceAfter(amt_to_write)) {
+           PipeBufHasSpaceAfter(amt_to_write) &&
+           amt_to_write < max_amt_to_write) {
       char* data = iter.Data();
-      size_t size = iter.RemainingInSegment();
+      size_t size =
+          std::min(iter.RemainingInSegment(), max_amt_to_write - amt_to_write);
 
       iov[iov_count].iov_base = data;
       iov[iov_count].iov_len = size;
       iov_count++;
       amt_to_write += size;
       iter.Advance(msg->Buffers(), size);
     }
+    MOZ_ASSERT(amt_to_write <= max_amt_to_write);
 
     const bool intentional_short_write = !iter.Done();
     msgh.msg_iov = iov;
     msgh.msg_iovlen = iov_count;
 
     ssize_t bytes_written =
         HANDLE_EINTR(corrected_sendmsg(pipe_, &msgh, MSG_DONTWAIT));
 
-#if !defined(OS_MACOSX)
-    // On OSX the attached_handles_ array gets cleared later, once we get the
-    // RECEIVED_FDS_MESSAGE_TYPE message.
-    if (bytes_written > 0) {
-      msg->attached_handles_.Clear();
-    }
-#endif
-
     if (bytes_written < 0) {
       switch (errno) {
         case EAGAIN:
           // Not an error; the sendmsg would have blocked, so return to the
           // event loop and try again later.
           break;
 #if defined(OS_MACOSX) || defined(OS_NETBSD)
           // (Note: this comment is copied from https://crrev.com/86c3d9ef4fdf6;
@@ -713,37 +717,41 @@ bool Channel::ChannelImpl::ProcessOutgoi
 
     if (intentional_short_write ||
         static_cast<size_t>(bytes_written) != amt_to_write) {
       // If write() fails with EAGAIN then bytes_written will be -1.
       if (bytes_written > 0) {
         MOZ_DIAGNOSTIC_ASSERT(intentional_short_write ||
                               static_cast<size_t>(bytes_written) <
                                   amt_to_write);
-        partial_write_iter_.ref().AdvanceAcrossSegments(msg->Buffers(),
-                                                        bytes_written);
+        partial_write_->iter_.AdvanceAcrossSegments(msg->Buffers(),
+                                                    bytes_written);
         // We should not hit the end of the buffer.
-        MOZ_DIAGNOSTIC_ASSERT(!partial_write_iter_.ref().Done());
+        MOZ_DIAGNOSTIC_ASSERT(!partial_write_->iter_.Done());
       }
 
       // Tell libevent to call us back once things are unblocked.
       is_blocked_on_write_ = true;
       MessageLoopForIO::current()->WatchFileDescriptor(
           pipe_,
           false,  // One shot
           MessageLoopForIO::WATCH_WRITE, &write_watcher_, this);
       return true;
     } else {
-      partial_write_iter_.reset();
+      partial_write_.reset();
 
 #if defined(OS_MACOSX)
       if (!msg->attached_handles_.IsEmpty()) {
         pending_fds_.push_back(PendingDescriptors{
             msg->fd_cookie(), std::move(msg->attached_handles_)});
       }
+#else
+      if (bytes_written > 0) {
+        msg->attached_handles_.Clear();
+      }
 #endif
 
       // Message sent OK!
 
       AddIPCProfilerMarker(*msg, other_pid_, MessageDirection::eSending,
                            MessagePhase::TransferEnd);
 
 #ifdef IPC_MESSAGE_DEBUG_EXTRA
@@ -838,17 +846,17 @@ void Channel::ChannelImpl::OutputQueuePu
 
   MOZ_DIAGNOSTIC_ASSERT(!closed_);
   msg->AssertAsLargeAsHeader();
   output_queue_.Push(std::move(msg));
 }
 
 void Channel::ChannelImpl::OutputQueuePop() {
   // Clear any reference to the front of output_queue_ before we destroy it.
-  partial_write_iter_.reset();
+  partial_write_.reset();
 
   mozilla::UniquePtr<Message> message = output_queue_.Pop();
 }
 
 // Called by libevent when we can write to the pipe without blocking.
 void Channel::ChannelImpl::OnFileCanWriteWithoutBlocking(int fd) {
   if (!ProcessOutgoingMessages()) {
     Close();
--- a/ipc/chromium/src/chrome/common/ipc_channel_posix.h
+++ b/ipc/chromium/src/chrome/common/ipc_channel_posix.h
@@ -95,17 +95,21 @@ class Channel::ChannelImpl : public Mess
   MessageLoopForIO::FileDescriptorWatcher write_watcher_;
 
   // Indicates whether we're currently blocked waiting for a write to complete.
   bool is_blocked_on_write_;
 
   // If sending a message blocks then we use this iterator to keep track of
   // where in the message we are. It gets reset when the message is finished
   // sending.
-  mozilla::Maybe<Pickle::BufferList::IterImpl> partial_write_iter_;
+  struct PartialWrite {
+    Pickle::BufferList::IterImpl iter_;
+    mozilla::Span<const mozilla::UniqueFileHandle> handles_;
+  };
+  mozilla::Maybe<PartialWrite> partial_write_;
 
   int server_listen_pipe_;
   int pipe_;
   int client_pipe_;        // The client end of our socketpair().
   unsigned pipe_buf_len_;  // The SO_SNDBUF value of pipe_, or 0 if unknown.
 
   Listener* listener_;
 
@@ -115,28 +119,29 @@ class Channel::ChannelImpl : public Mess
   // We read from the pipe into these buffers.
   size_t input_buf_offset_;
   mozilla::UniquePtr<char[]> input_buf_;
   mozilla::UniquePtr<char[]> input_cmsg_buf_;
 
   // The control message buffer will hold all of the file descriptors that will
   // be read in during a single recvmsg call. Message::WriteFileDescriptor
   // always writes one word of data for every file descriptor added to the
-  // message, and the number of file descriptors per message will not exceed
-  // MAX_DESCRIPTORS_PER_MESSAGE.
+  // message, and the number of file descriptors per recvmsg will not exceed
+  // kControlBufferMaxFds. This is based on the true maximum SCM_RIGHTS
+  // descriptor count, which is just over 250 on both Linux and macOS.
   //
   // This buffer also holds a control message header of size CMSG_SPACE(0)
   // bytes. However, CMSG_SPACE is not a constant on Macs, so we can't use it
   // here. Consequently, we pick a number here that is at least CMSG_SPACE(0) on
   // all platforms. We assert at runtime, in Channel::ChannelImpl::Init, that
   // it's big enough.
+  static constexpr size_t kControlBufferMaxFds = 200;
   static constexpr size_t kControlBufferHeaderSize = 32;
   static constexpr size_t kControlBufferSize =
-      IPC::Message::MAX_DESCRIPTORS_PER_MESSAGE * sizeof(int) +
-      kControlBufferHeaderSize;
+      kControlBufferMaxFds * sizeof(int) + kControlBufferHeaderSize;
 
   // Large incoming messages that span multiple pipe buffers get built-up in the
   // buffers of this message.
   mozilla::Maybe<Message> incoming_message_;
   std::vector<int> input_overflow_fds_;
 
   // In server-mode, we have to wait for the client to connect before we
   // can begin reading.  We make use of the input_state_ when performing