workaround for mac kernel bug

ipc/ipc_channel_posix.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "ipc/ipc_channel_posix.h"
 
 #include <errno.h>
 #include <fcntl.h>
 #include <stddef.h>
 #include <sys/socket.h>
@@ skipping 329 matching lines @@
         true,
         base::MessageLoopForIO::WATCH_READ,
         &server_listen_connection_watcher_,
         this);
   } else {
     did_connect = AcceptConnection();
   }
   return did_connect;
 }
 
+void Channel::ChannelImpl::CloseFileDescriptors(Message* msg) {
+#if 0 // defined(OS_MACOSX)  // Disable fix to make sure test works
+  // There is a bug on OSX which makes it dangerous to close
+  // a file descriptor while it is in transit. So instead we
+  // store the file descriptor in a set and send a message to
+  // the recipient, which is queued AFTER the message that
+  // sent the FD. The recipient will reply to the message,
+  // letting us know that it is now safe to close the file
+  // descptor. For more information, see:

[Scott Hess - ex-Googler, 2013/10/09 20:18:33]

descriptor

[hubbe, 2013/10/09 21:34:12]

Done.

+  // http://crbug.com/298276
+  std::vector<int> to_close;
+  msg->file_descriptor_set()->GetFDsToCloseAndClear(&to_close);
+  for (size_t i = 0; i < to_close.size(); i++) {
+    fds_to_close_.insert(to_close[i]);
+    QueueCloseFDMessage(to_close[i], 2);
+  }
+#else
+  msg->file_descriptor_set()->CommitAll();
+#endif
+}
+
+
 bool Channel::ChannelImpl::ProcessOutgoingMessages() {
   DCHECK(!waiting_connect_);  // Why are we trying to send messages if there's
                               // no connection?
   if (output_queue_.empty())
     return true;
 
   if (pipe_ == -1)
     return false;
 
   // Write out all the messages we can till the write blocks or there are no
@@ skipping 52 matching lines @@
         // 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_;
         bytes_written = HANDLE_EINTR(sendmsg(fd_pipe_, &msgh, MSG_DONTWAIT));
         msgh.msg_iov = &iov;
         msgh.msg_controllen = 0;
         if (bytes_written > 0) {
-          msg->file_descriptor_set()->CommitAll();
+          CloseFileDescriptors(msg);
         }
       }
 #endif  // IPC_USES_READWRITE
     }
 
     if (bytes_written == 1) {
       fd_written = pipe_;
 #if defined(IPC_USES_READWRITE)
       if ((mode_ & MODE_CLIENT_FLAG) && IsHelloMessage(*msg)) {
         DCHECK_EQ(msg->file_descriptor_set()->size(), 1U);
       }
       if (!msgh.msg_controllen) {
         bytes_written = HANDLE_EINTR(write(pipe_, out_bytes, amt_to_write));
       } else
 #endif  // IPC_USES_READWRITE
       {
         bytes_written = HANDLE_EINTR(sendmsg(pipe_, &msgh, MSG_DONTWAIT));
       }
     }
     if (bytes_written > 0)
-      msg->file_descriptor_set()->CommitAll();
+      CloseFileDescriptors(msg);
 
     if (bytes_written < 0 && !SocketWriteErrorIsRecoverable()) {
 #if defined(OS_MACOSX)
       // On OSX writing to a pipe with no listener returns EPERM.
       if (errno == EPERM) {
         Close();
         return false;
       }
 #endif  // OS_MACOSX
       if (errno == EPIPE) {
@@ skipping 114 matching lines @@
 #endif  // IPC_USES_READWRITE
 
   while (!output_queue_.empty()) {
     Message* m = output_queue_.front();
     output_queue_.pop();
     delete m;
   }
 
   // Close any outstanding, received file descriptors.
   ClearInputFDs();
+
+#if defined(OS_MACOSX)
+  // Clear any outstanding, sent file descriptors.
+  for (std::set<int>::iterator i = fds_to_close_.begin();
+       i != fds_to_close_.end();
+       ++i) {
+    if (HANDLE_EINTR(close(*i)) < 0)
+      PLOG(ERROR) << "close";
+  }
+#endif
 }
 
 // static
 bool Channel::ChannelImpl::IsNamedServerInitialized(
     const std::string& channel_id) {
   return base::PathExists(base::FilePath(channel_id));
 }
 
 #if defined(OS_LINUX)
 // static
 void Channel::ChannelImpl::SetGlobalPid(int pid) {
   global_pid_ = pid;
 }
 #endif  // OS_LINUX
 
 // Called by libevent when we can read from the pipe without blocking.
 void Channel::ChannelImpl::OnFileCanReadWithoutBlocking(int fd) {
-  bool send_server_hello_msg = false;
   if (fd == server_listen_pipe_) {
     int new_pipe = 0;
     if (!ServerAcceptConnection(server_listen_pipe_, &new_pipe) ||
         new_pipe < 0) {
       Close();
       listener()->OnChannelListenError();
     }
 
     if (pipe_ != -1) {
       // We already have a connection. We only handle one at a time.
@@ skipping 18 matching lines @@
       if (client_euid != geteuid()) {
         DLOG(WARNING) << "Client euid is not authorised";
         ResetToAcceptingConnectionState();
         return;
       }
     }
 
     if (!AcceptConnection()) {
       NOTREACHED() << "AcceptConnection should not fail on server";
     }
-    send_server_hello_msg = true;
     waiting_connect_ = false;
   } else if (fd == pipe_) {
     if (waiting_connect_ && (mode_ & MODE_SERVER_FLAG)) {
-      send_server_hello_msg = true;
       waiting_connect_ = false;
     }
     if (!ProcessIncomingMessages()) {
       // ClosePipeOnError may delete this object, so we mustn't call
       // ProcessOutgoingMessages.
-      send_server_hello_msg = false;
       ClosePipeOnError();
+      return;
     }
   } else {
     NOTREACHED() << "Unknown pipe " << fd;
   }
 
   // If we're a server and handshaking, then we want to make sure that we
   // only send our handshake message after we've processed the client's.
   // This gives us a chance to kill the client if the incoming handshake
-  // is invalid.
-  if (send_server_hello_msg) {
+  // is invalid. This also flushes any closefd messagse.
+  if (!is_blocked_on_write_) {

[Scott Hess - ex-Googler, 2013/10/09 20:18:33]

I am nowhere close to understanding whether the changes in this function
replicate the original behavior.

[hubbe, 2013/10/09 21:34:12]

It's fairly easy to reason out:

When we enter OnFileCanReadWithoutBlocking, we can be in one of two states:

1. is_blocked_on_write_ is true
2. output_queue_ is empty

When we get to the end of OnFileCanReadWithoutBlocking, it is possible that some
data has been queued (a hello message or a closefd message)  if so, we should
make sure that those messages get sent at some point.  If is_blocked_on_write_
is true, then there is already a callback pending to ProcessOutgoingMessage()
and we don't need to do anything. If not, we call ProcessOutgoingMessage() which
will either empty the queue or set is_blocked_on_write_ to true, thus returning
us to a good state.

(Note that ProcessOutgoingMessage() will do nothing if the queue is empty, so we
don't really need to test if there is a message to process or not.)

There is one caveat, in the previous code send_server_hello_msg was set to false
in one case to avoid calling ProcessOutgoingMessage() if the object has already
been destroyed. That case is now taken care of with a return.

     ProcessOutgoingMessages();
   }
 }
 
 // Called by libevent when we can write to the pipe without blocking.
 void Channel::ChannelImpl::OnFileCanWriteWithoutBlocking(int fd) {
   DCHECK_EQ(pipe_, fd);
   is_blocked_on_write_ = false;
   if (!ProcessOutgoingMessages()) {
     ClosePipeOnError();
@@ skipping 229 matching lines @@
 }
 
 void Channel::ChannelImpl::ClearInputFDs() {
   for (size_t i = 0; i < input_fds_.size(); ++i) {
     if (HANDLE_EINTR(close(input_fds_[i])) < 0)
       PLOG(ERROR) << "close ";
   }
   input_fds_.clear();
 }
 
-void Channel::ChannelImpl::HandleHelloMessage(const Message& msg) {
+void Channel::ChannelImpl::QueueCloseFDMessage(int fd, int hops) {
+  switch (hops) {
+    case 1:
+    case 2: {
+      // Create the message
+      scoped_ptr<Message> msg(new Message(MSG_ROUTING_NONE,
+                                          CLOSE_FD_MESSAGE_TYPE,
+                                          IPC::Message::PRIORITY_NORMAL));
+      if (!msg->WriteInt(hops - 1) || !msg->WriteInt(fd)) {
+        NOTREACHED() << "Unable to pickle close fd.";
+      }
+      // Send(msg.release());
+      output_queue_.push(msg.release());
+      break;
+    }
+
+    default:
+      NOTREACHED();
+      break;
+  }
+}
+
+void Channel::ChannelImpl::HandleInternalMessage(const Message& msg) {
   // The Hello message contains only the process id.
   PickleIterator iter(msg);
-  int pid;
-  if (!msg.ReadInt(&iter, &pid))
-    NOTREACHED();
+
+  switch (msg.type()) {
+    default:
+      NOTREACHED();
+      break;
+
+    case Channel::HELLO_MESSAGE_TYPE:
+      int pid;
+      if (!msg.ReadInt(&iter, &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.file_descriptor_set()->size(), 1U);
-    base::FileDescriptor descriptor;
-    if (!msg.ReadFileDescriptor(&iter, &descriptor)) {
-      NOTREACHED();
-    }
-    fd_pipe_ = descriptor.fd;
-    CHECK(descriptor.auto_close);
+      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.file_descriptor_set()->size(), 1U);
+        base::FileDescriptor descriptor;
+        if (!msg.ReadFileDescriptor(&iter, &descriptor)) {
+          NOTREACHED();
+        }
+        fd_pipe_ = descriptor.fd;
+        CHECK(descriptor.auto_close);
+      }
+#endif  // IPC_USES_READWRITE
+      peer_pid_ = pid;
+      listener()->OnChannelConnected(pid);
+      break;
+
+#if defined(OS_MACOSX)
+    case Channel::CLOSE_FD_MESSAGE_TYPE:
+      int fd, hops;
+      if (!msg.ReadInt(&iter, &hops))
+        NOTREACHED();
+      if (!msg.ReadInt(&iter, &fd))
+        NOTREACHED();
+      std::set<int>::iterator i = fds_to_close_.find(fd);

[Scott Hess - ex-Googler, 2013/10/09 20:18:33]

You only make use of i in one case, which leads to erase().  You could probably
put this in the hops == 0 case, like:
  if (fds_to_close_.erase(fd) > 0) {
    if (HANDLE_EINTR(close(fd)) < 0)
      PLOG(ERROR) << "close";
  } else {
    NOTREACHED();
  }

[hubbe, 2013/10/09 21:34:12]

Yep, much better, done.

+      if (hops == 0) {
+        if (i != fds_to_close_.end()) {
+          fds_to_close_.erase(i);
+          if (HANDLE_EINTR(close(fd)) < 0)
+            PLOG(ERROR) << "close";
+        } else {
+          NOTREACHED();
+        }
+      } else {
+        QueueCloseFDMessage(fd, hops);
+      }

[Scott Hess - ex-Googler, 2013/10/09 20:18:33]

break;

[hubbe, 2013/10/09 21:34:12]

Done.

+#endif
   }
-#endif  // IPC_USES_READWRITE
-  peer_pid_ = pid;
-  listener()->OnChannelConnected(pid);
 }
 
 void Channel::ChannelImpl::Close() {
   // Close can be called multiple time, so we need to make sure we're
   // idempotent.
 
   ResetToAcceptingConnectionState();
 
   if (must_unlink_) {
     unlink(pipe_name_.c_str());
@@ skipping 81 matching lines @@
 
 
 #if defined(OS_LINUX)
 // static
 void Channel::SetGlobalPid(int pid) {
   ChannelImpl::SetGlobalPid(pid);
 }
 #endif  // OS_LINUX
 
 }  // namespace IPC