mac: ChildPortHandshake: allow receive rights to be sent

The intended use is to flip the client-server relationship in
CrashpadClient so that the initial client (parent process) furnishes the
handler process with a receive right. The parent can optionally receive
a port-destroyed notification allowing it to restart the handler if it
exits prematurely.

R=rsesek@chromium.org

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

client/crashpad_client_mac.cc

@@ -97,7 +97,7 @@ bool CrashpadClient::StartHandler(
   // Set up the arguments for execve() first. These aren’t needed until execve()
   // is called, but it’s dangerous to do this in a child process after fork().
   ChildPortHandshake child_port_handshake;
-  int handshake_fd = child_port_handshake.ReadPipeFD();
+  base::ScopedFD client_read_fd = child_port_handshake.ClientReadFD();
 
   // Use handler as argv[0], followed by arguments directed by this method’s
   // parameters and a --handshake-fd argument. |arguments| are added first so
@@ -119,7 +119,7 @@ bool CrashpadClient::StartHandler(
     argv.push_back(
         FormatArgumentString("annotation", kv.first + '=' + kv.second));
   }
-  argv.push_back(FormatArgumentInt("handshake-fd", handshake_fd));
+  argv.push_back(FormatArgumentInt("handshake-fd", client_read_fd.get()));
 
   // argv_c contains const char* pointers and is terminated by nullptr. argv
   // is required because the pointers in argv_c need to point somewhere, and
@@ -181,7 +181,7 @@ bool CrashpadClient::StartHandler(
 
     // Grandchild process.
 
-    CloseMultipleNowOrOnExec(STDERR_FILENO + 1, handshake_fd);
+    CloseMultipleNowOrOnExec(STDERR_FILENO + 1, client_read_fd.get());
 
     // &argv_c[0] is a pointer to a pointer to const char data, but because of
     // how C (not C++) works, execvp() wants a pointer to a const pointer to
@@ -193,6 +193,8 @@ bool CrashpadClient::StartHandler(
 
   // Parent process.
 
+  client_read_fd.reset();
+
   // waitpid() for the child, so that it does not become a zombie process. The
   // child normally exits quickly.
   int status;
@@ -209,7 +211,8 @@ bool CrashpadClient::StartHandler(
   }
 
   // Rendezvous with the handler running in the grandchild process.
-  exception_port_.reset(child_port_handshake.RunServer());
+  exception_port_.reset(child_port_handshake.RunServer(
+      ChildPortHandshake::PortRightType::kSendRight));
 
   return exception_port_.is_valid();
 }

handler/main.cc

@@ -19,6 +19,7 @@
 #include <string>
 
 #include "base/files/file_path.h"
+#include "base/files/scoped_file.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
 #include "build/build_config.h"
@@ -219,9 +220,12 @@ int HandlerMain(int argc, char* argv[]) {
 
 #if defined(OS_MACOSX)
   CloseStdinAndStdout();
-  ChildPortHandshake::RunClient(options.handshake_fd,
+  if (!ChildPortHandshake::RunClientForFD(
-                                exception_handler_server.receive_port(),
+          base::ScopedFD(options.handshake_fd),
-                                MACH_MSG_TYPE_MAKE_SEND);
+          exception_handler_server.receive_port(),
+          MACH_MSG_TYPE_MAKE_SEND)) {
+    return EXIT_FAILURE;
+  }
 #endif  // OS_MACOSX
 
   scoped_ptr<CrashReportDatabase> database(CrashReportDatabase::Initialize(

util/mach/child_port.defs

@@ -16,10 +16,10 @@
 #include <mach/std_types.defs>
 
 // child_port provides an interface for port rights to be transferred between
-// tasks. Its expected usage is for child processes to be able to pass port
+// tasks. Its expected usage is for processes to be able to pass port rights
-// rights to their parent processes. A child may wish to give its parent a copy
+// across IPC boundaries. A child process may wish to give its parent a copy of
-// of a send right to its own task port, or a child may hold a receive right for
+// of a send right to its own task port, or a parent process may wish to give a
-// a server and wish to furnish its parent with a send right to that server.
+// receive right to a child process that implements a server.
 //
 // This Mach subsystem defines the lowest-level interface for these rights to
 // be transferred. Most users will not user this interface directly, but will
@@ -48,9 +48,7 @@ import "util/mach/child_port_types.h";
 //     secret allowing the server to verify that it has received a request from
 //     the intended client. |server| will reject requests with an invalid
 //     |token|.
-// port[in]: A port right to transfer to the server. In expected usage, this may
+// port[in]: A port right to transfer to the server.
-//     be a send or send-once right, and the |server| will reject a receive
-//     right. It is permissible to specify make-send for a receive right.
 //
 // Return value: As this is a “simpleroutine”, the server does not respond to
 //     the client request, and the client does not block waiting for a response

util/mach/child_port_handshake.cc

@@ -32,60 +32,60 @@
 #include "base/strings/stringprintf.h"
 #include "util/file/file_io.h"
 #include "util/mach/child_port.h"
+#include "util/mach/child_port_server.h"
 #include "util/mach/mach_extensions.h"
 #include "util/mach/mach_message.h"
 #include "util/mach/mach_message_server.h"
 #include "util/misc/implicit_cast.h"
 
 namespace crashpad {
+namespace {
 
-ChildPortHandshake::ChildPortHandshake()
+class ChildPortHandshakeServer final : public ChildPortServer::Interface {
+ public:
+  ChildPortHandshakeServer();
+  ~ChildPortHandshakeServer();
+
+  mach_port_t RunServer(base::ScopedFD server_write_fd,
+                        ChildPortHandshake::PortRightType port_right_type);
+
+ private:
+  // ChildPortServer::Interface:
+  kern_return_t HandleChildPortCheckIn(child_port_server_t server,
+                                       child_port_token_t token,
+                                       mach_port_t port,
+                                       mach_msg_type_name_t right_type,
+                                       const mach_msg_trailer_t* trailer,
+                                       bool* destroy_request) override;
+
+  child_port_token_t token_;
+  mach_port_t port_;
+  mach_msg_type_name_t right_type_;
+  bool checked_in_;
+
+  DISALLOW_COPY_AND_ASSIGN(ChildPortHandshakeServer);
+};
+
+ChildPortHandshakeServer::ChildPortHandshakeServer()
     : token_(0),
-      pipe_read_(),
+      port_(MACH_PORT_NULL),
-      pipe_write_(),
+      right_type_(MACH_MSG_TYPE_PORT_NONE),
-      child_port_(MACH_PORT_NULL),
       checked_in_(false) {
-  // Use socketpair() instead of pipe(). There is no way to suppress SIGPIPE on
-  // pipes in Mac OS X 10.6, because the F_SETNOSIGPIPE fcntl() command was not
-  // introduced until 10.7.
-  int pipe_fds[2];
-  PCHECK(socketpair(AF_UNIX, SOCK_STREAM, PF_UNSPEC, pipe_fds) == 0)
-      << "socketpair";
-
-  pipe_read_.reset(pipe_fds[0]);
-  pipe_write_.reset(pipe_fds[1]);
-
-  // Simulate pipe() semantics by shutting down the “wrong” sides of the socket.
-  PCHECK(shutdown(pipe_write_.get(), SHUT_RD) == 0) << "shutdown";
-  PCHECK(shutdown(pipe_read_.get(), SHUT_WR) == 0) << "shutdown";
-
-  // SIGPIPE is undesirable when writing to this pipe. Allow broken-pipe writes
-  // to fail with EPIPE instead.
-  const int value = 1;
-  PCHECK(setsockopt(
-      pipe_write_.get(), SOL_SOCKET, SO_NOSIGPIPE, &value, sizeof(value)) == 0)
-      << "setsockopt";
 }
 
-ChildPortHandshake::~ChildPortHandshake() {
+ChildPortHandshakeServer::~ChildPortHandshakeServer() {
 }
 
-int ChildPortHandshake::ReadPipeFD() const {
+mach_port_t ChildPortHandshakeServer::RunServer(
-  DCHECK_NE(pipe_read_.get(), -1);
+    base::ScopedFD server_write_fd,
-  return pipe_read_.get();
+    ChildPortHandshake::PortRightType port_right_type) {
-}
+  DCHECK_EQ(port_, kMachPortNull);
-
+  DCHECK(!checked_in_);
-mach_port_t ChildPortHandshake::RunServer() {
+  DCHECK(server_write_fd.is_valid());
-  DCHECK_NE(pipe_read_.get(), -1);
-  pipe_read_.reset();
-
-  // Transfer ownership of the write pipe into this method’s scope.
-  base::ScopedFD pipe_write_owner = pipe_write_.Pass();
 
   // Initialize the token and share it with the client via the pipe.
   token_ = base::RandUint64();
-  int pipe_write = pipe_write_owner.get();
+  if (!LoggingWriteFile(server_write_fd.get(), &token_, sizeof(token_))) {
-  if (!LoggingWriteFile(pipe_write, &token_, sizeof(token_))) {
     LOG(WARNING) << "no client check-in";
     return MACH_PORT_NULL;
   }
@@ -109,14 +109,15 @@ mach_port_t ChildPortHandshake::RunServer() {
 
   // Share the service name with the client via the pipe.
   uint32_t service_name_length = service_name.size();
-  if (!LoggingWriteFile(
+  if (!LoggingWriteFile(server_write_fd.get(),
-          pipe_write, &service_name_length, sizeof(service_name_length))) {
+                        &service_name_length,
+                        sizeof(service_name_length))) {
     LOG(WARNING) << "no client check-in";
     return MACH_PORT_NULL;
   }
 
   if (!LoggingWriteFile(
-          pipe_write, service_name.c_str(), service_name_length)) {
+          server_write_fd.get(), service_name.c_str(), service_name_length)) {
     LOG(WARNING) << "no client check-in";
     return MACH_PORT_NULL;
   }
@@ -147,7 +148,7 @@ mach_port_t ChildPortHandshake::RunServer() {
          0,
          nullptr);
   EV_SET(&changelist[1],
-         pipe_write,
+         server_write_fd.get(),
          EVFILT_WRITE,
          EV_ADD | EV_CLEAR,
          0,
@@ -162,7 +163,7 @@ mach_port_t ChildPortHandshake::RunServer() {
   bool blocking = true;
   DCHECK(!checked_in_);
   while (!checked_in_) {
-    DCHECK_EQ(child_port_, kMachPortNull);
+    DCHECK_EQ(port_, kMachPortNull);
 
     // Get a kevent from the kqueue. Block while waiting for an event unless the
     // write pipe has arrived at EOF, in which case the kevent() should be
@@ -230,7 +231,7 @@ mach_port_t ChildPortHandshake::RunServer() {
         // pipe. Ignore that case. Multiple notifications for that situation
         // will not be generated because edge triggering (EV_CLEAR) is used
         // above.
-        DCHECK_EQ(implicit_cast<int>(event.ident), pipe_write);
+        DCHECK_EQ(implicit_cast<int>(event.ident), server_write_fd.get());
         if (event.flags & EV_EOF) {
           // There are no readers attached to the write pipe. The client has
           // closed its side of the pipe. There can be one last shot at
@@ -246,19 +247,48 @@ mach_port_t ChildPortHandshake::RunServer() {
     }
   }
 
-  mach_port_t child_port = MACH_PORT_NULL;
+  if (port_ == MACH_PORT_NULL) {
-  std::swap(child_port_, child_port);
+    return MACH_PORT_NULL;
-  return child_port;
+  }
+
+  bool mismatch = false;
+  switch (port_right_type) {
+    case ChildPortHandshake::PortRightType::kReceiveRight:
+      if (right_type_ != MACH_MSG_TYPE_PORT_RECEIVE) {
+        LOG(ERROR) << "expected receive right, observed " << right_type_;
+        mismatch = true;
+      }
+      break;
+    case ChildPortHandshake::PortRightType::kSendRight:
+      if (right_type_ != MACH_MSG_TYPE_PORT_SEND &&
+          right_type_ != MACH_MSG_TYPE_PORT_SEND_ONCE) {
+        LOG(ERROR) << "expected send or send-once right, observed "
+                   << right_type_;
+        mismatch = true;
+      }
+      break;
+  }
+
+  if (mismatch) {
+    MachMessageDestroyReceivedPort(port_, right_type_);
+    port_ = MACH_PORT_NULL;
+    return MACH_PORT_NULL;
+  }
+
+  mach_port_t port = MACH_PORT_NULL;
+  std::swap(port_, port);
+  return port;
 }
 
-kern_return_t ChildPortHandshake::HandleChildPortCheckIn(
+kern_return_t ChildPortHandshakeServer::HandleChildPortCheckIn(
     child_port_server_t server,
     const child_port_token_t token,
     mach_port_t port,
     mach_msg_type_name_t right_type,
     const mach_msg_trailer_t* trailer,
     bool* destroy_request) {
-  DCHECK_EQ(child_port_, kMachPortNull);
+  DCHECK_EQ(port_, kMachPortNull);
+  DCHECK(!checked_in_);
 
   if (token != token_) {
     // If the token’s not correct, someone’s attempting to spoof the legitimate
@@ -268,19 +298,18 @@ kern_return_t ChildPortHandshake::HandleChildPortCheckIn(
   } else {
     checked_in_ = true;
 
-    if (right_type == MACH_MSG_TYPE_PORT_RECEIVE) {
+    if (right_type != MACH_MSG_TYPE_PORT_RECEIVE &&
-      // The message needs to carry a send right or a send-once right. This
+        right_type != MACH_MSG_TYPE_PORT_SEND &&
-      // isn’t a strict requirement of the protocol, but users of this class
+        right_type != MACH_MSG_TYPE_PORT_SEND_ONCE) {
-      // expect a send right or a send-once right, both of which can be managed
+      // The message needs to carry a receive, send, or send-once right.
-      // by base::mac::ScopedMachSendRight. It is invalid to store a receive
+      LOG(ERROR) << "invalid right type " << right_type;
-      // right in that scoper.
-      LOG(WARNING) << "ignoring MACH_MSG_TYPE_PORT_RECEIVE";
       *destroy_request = true;
     } else {
-      // Communicate the child port back to the RunServer().
+      // Communicate the child port and right type back to the RunServer().
       // *destroy_request is left at false, because RunServer() needs the right
       // to remain intact. It gives ownership of the right to its caller.
-      child_port_ = port;
+      port_ = port;
+      right_type_ = right_type;
     }
   }
 
@@ -288,40 +317,112 @@ kern_return_t ChildPortHandshake::HandleChildPortCheckIn(
   return MIG_NO_REPLY;
 }
 
-// static
+}  // namespace
-void ChildPortHandshake::RunClient(int pipe_read,
+
-                                   mach_port_t port,
+ChildPortHandshake::ChildPortHandshake()
+    : client_read_fd_(),
+      server_write_fd_() {
+  // Use socketpair() instead of pipe(). There is no way to suppress SIGPIPE on
+  // pipes in Mac OS X 10.6, because the F_SETNOSIGPIPE fcntl() command was not
+  // introduced until 10.7.
+  int pipe_fds[2];
+  PCHECK(socketpair(AF_UNIX, SOCK_STREAM, PF_UNSPEC, pipe_fds) == 0)
+      << "socketpair";
+
+  client_read_fd_.reset(pipe_fds[0]);
+  server_write_fd_.reset(pipe_fds[1]);
+
+  // Simulate pipe() semantics by shutting down the “wrong” sides of the socket.
+  PCHECK(shutdown(server_write_fd_.get(), SHUT_RD) == 0) << "shutdown SHUT_RD";
+  PCHECK(shutdown(client_read_fd_.get(), SHUT_WR) == 0) << "shutdown SHUT_WR";
+
+  // SIGPIPE is undesirable when writing to this pipe. Allow broken-pipe writes
+  // to fail with EPIPE instead.
+  const int value = 1;
+  PCHECK(setsockopt(server_write_fd_.get(),
+                    SOL_SOCKET,
+                    SO_NOSIGPIPE,
+                    &value,
+                    sizeof(value)) == 0) << "setsockopt";
+}
+
+ChildPortHandshake::~ChildPortHandshake() {
+}
+
+base::ScopedFD ChildPortHandshake::ClientReadFD() {
+  DCHECK(client_read_fd_.is_valid());
+  return client_read_fd_.Pass();
+}
+
+base::ScopedFD ChildPortHandshake::ServerWriteFD() {
+  DCHECK(server_write_fd_.is_valid());
+  return server_write_fd_.Pass();
+}
+
+mach_port_t ChildPortHandshake::RunServer(PortRightType port_right_type) {
+  client_read_fd_.reset();
+  return RunServerForFD(server_write_fd_.Pass(), port_right_type);
+}
+
+bool ChildPortHandshake::RunClient(mach_port_t port,
                                    mach_msg_type_name_t right_type) {
-  base::ScopedFD pipe_read_owner(pipe_read);
+  server_write_fd_.reset();
+  return RunClientForFD(client_read_fd_.Pass(), port, right_type);
+}
+
+// static
+mach_port_t ChildPortHandshake::RunServerForFD(base::ScopedFD server_write_fd,
+                                               PortRightType port_right_type) {
+  ChildPortHandshakeServer server;
+  return server.RunServer(server_write_fd.Pass(), port_right_type);
+}
+
+// static
+bool ChildPortHandshake::RunClientForFD(base::ScopedFD client_read_fd,
+                                        mach_port_t port,
+                                        mach_msg_type_name_t right_type) {
+  DCHECK(client_read_fd.is_valid());
 
   // Read the token and the service name from the read side of the pipe.
   child_port_token_t token;
   std::string service_name;
-  RunClientInternal_ReadPipe(pipe_read, &token, &service_name);
+  if (!RunClientInternal_ReadPipe(
+          client_read_fd.get(), &token, &service_name)) {
+    return false;
+  }
 
   // Look up the server and check in with it by providing the token and port.
-  RunClientInternal_SendCheckIn(service_name, token, port, right_type);
+  return RunClientInternal_SendCheckIn(service_name, token, port, right_type);
 }
 
 // static
-void ChildPortHandshake::RunClientInternal_ReadPipe(int pipe_read,
+bool ChildPortHandshake::RunClientInternal_ReadPipe(int client_read_fd,
                                                     child_port_token_t* token,
                                                     std::string* service_name) {
   // Read the token from the pipe.
-  CheckedReadFile(pipe_read, token, sizeof(*token));
+  if (!LoggingReadFile(client_read_fd, token, sizeof(*token))) {
+    return false;
+  }
 
   // Read the service name from the pipe.
   uint32_t service_name_length;
-  CheckedReadFile(pipe_read, &service_name_length, sizeof(service_name_length));
+  if (!LoggingReadFile(
+      client_read_fd, &service_name_length, sizeof(service_name_length))) {
+    return false;
+  }
 
   service_name->resize(service_name_length);
-  if (!service_name->empty()) {
+  if (!service_name->empty() &&
-    CheckedReadFile(pipe_read, &(*service_name)[0], service_name_length);
+      !LoggingReadFile(
+          client_read_fd, &(*service_name)[0], service_name_length)) {
+    return false;
   }
+
+  return true;
 }
 
 // static
-void ChildPortHandshake::RunClientInternal_SendCheckIn(
+bool ChildPortHandshake::RunClientInternal_SendCheckIn(
     const std::string& service_name,
     child_port_token_t token,
     mach_port_t port,
@@ -329,12 +430,19 @@ void ChildPortHandshake::RunClientInternal_SendCheckIn(
   // Get a send right to the server by looking up the service with the bootstrap
   // server by name.
   base::mac::ScopedMachSendRight server_port(BootstrapLookUp(service_name));
-  CHECK(server_port.is_valid());
+  if (server_port == kMachPortNull) {
+    return false;
+  }
 
   // Check in with the server.
-  kern_return_t kr =
+  kern_return_t kr = child_port_check_in(
-      child_port_check_in(server_port.get(), token, port, right_type);
+      server_port.get(), token, port, right_type);
-  MACH_CHECK(kr == KERN_SUCCESS, kr) << "child_port_check_in";
+  if (kr != KERN_SUCCESS) {
+    MACH_LOG(ERROR, kr) << "child_port_check_in";
+    return false;
+  }
+
+  return true;
 }
 
 }  // namespace crashpad

util/mach/child_port_handshake.h

@@ -21,7 +21,7 @@
 
 #include "base/basictypes.h"
 #include "base/files/scoped_file.h"
-#include "util/mach/child_port_server.h"
+#include "util/mach/child_port_types.h"
 
 namespace crashpad {
 
@@ -31,80 +31,177 @@ class ChildPortHandshakeTest;
 }  // namespace
 }  // namespace test
 
-//! \brief Implements a handshake protocol that allows a parent process to
+//! \brief Implements a handshake protocol that allows processes to exchange
-//!     obtain a Mach port right from a child process.
+//!     port rights.
 //!
 //! Ordinarily, there is no way for parent and child processes to exchange port
 //! rights, outside of the rights that children inherit from their parents.
 //! These include task-special ports and exception ports, but all of these have
 //! system-defined uses, and cannot reliably be replaced: in a multi-threaded
-//! parent, it is impossible to temporarily change one an inheritable port while
+//! parent, it is impossible to temporarily change an inheritable port while
 //! maintaining a guarantee that another thread will not attempt to use it, and
 //! in children, it difficult to guarantee that nothing will attempt to use an
 //! inheritable port before it can be replaced with the correct one. This latter
 //! concern is becoming increasingly more pronounced as system libraries perform
-//! more operations that rely on an inheritable port in module initializers.
+//! more operations that rely on an inherited port in module initializers.
 //!
-//! The protocol implemented by this class involves a server that runs in the
+//! The protocol implemented by this class involves a server that runs in one
-//! parent process. The server is published with the bootstrap server, which the
+//! process. The server is published with the bootstrap server, which the other
-//! child has access to because the bootstrap port is one of the inherited
+//! process has access to because the bootstrap port is one of the inherited
-//! task-special ports. The parent and child also share a pipe, which the parent
+//! task-special ports. The two processes also share a pipe, which the server
-//! can write to and the child can read from. After launching a child process,
+//! can write to and the client can read from. The server will write a random
-//! the parent will write a random token to this pipe, along with the name under
+//! token to this pipe, along with the name under which its service has been
-//! which its server has been registered with the bootstrap server. The child
+//! registered with the bootstrap server. The client can then obtain a send
-//! can then obtain a send right to this server with `bootstrap_look_up()`, and
+//! right to this service with `bootstrap_look_up()`, and send a check-in
-//! send a check-in message containing the token value and the port right of its
+//! message containing the token value and the port right of its choice by
-//! choice by calling `child_port_check_in()`.
+//! calling `child_port_check_in()`.
 //!
-//! The inclusion of the token authenticates the child to its parent. This is
+//! The inclusion of the token authenticates the client to the server. This is
 //! necessary because the service is published with the bootstrap server, which
-//! opens up access to it to more than the child process. Because the token is
+//! opens up access to it to more than the intended client. Because the token is
-//! passed to the child by a shared pipe, it constitutes a shared secret not
+//! passed to the client by a shared pipe, it constitutes a shared secret not
 //! known by other processes that may have incidental access to the server. The
 //! ChildPortHandshake server considers its randomly-generated token valid until
 //! a client checks in with it. This mechanism is used instead of examining the
 //! request message’s audit trailer to verify the sender’s process ID because in
-//! some process architectures, it may be impossible to verify the child’s
+//! some process architectures, it may be impossible to verify the client’s
-//! process ID. This may happen when the child disassociates from the parent
+//! process ID.
-//! with a double fork(), and the actual client is the parent’s grandchild. In
-//! this case, the child would not check in, but the grandchild, in possession
-//! of the token, would check in.
 //!
 //! The shared pipe serves another purpose: the server monitors it for an
 //! end-of-file (no readers) condition. Once detected, it will stop its blocking
-//! wait for a client to check in. This mechanism was chosen over monitoring a
+//! wait for a client to check in. This mechanism was also chosen for its
-//! child process directly for exit to account for the possibility that the
+//! ability to function properly in diverse process architectures.
-//! child might disassociate with a double fork().
 //!
-//! This class can be used to allow a child process to provide its parent with
+//! This class can be used to allow a child process to provide its parent with a
-//! a send right to its task port, in cases where it is desirable for the parent
+//! send right to its task port, in cases where it is desirable for the parent
-//! to have such access. It can also be used to allow a child process to
+//! to have such access. It can also be used to allow a parent process to
-//! establish its own server and provide its parent with a send right to that
+//! transfer a receive right to a child process that implements the server for
-//! server, for cases where a service is provided and it is undesirable or
+//! that right, or for a child process to establish its own server and provide
-//! impossible to provide it via the bootstrap or launchd interfaces.
+//! its parent with a send right to that server, for cases where a service is
-class ChildPortHandshake : public ChildPortServer::Interface {
+//! provided and it is undesirable or impossible to provide it via the bootstrap
+//! or launchd interfaces.
+//!
+//! Example parent process, running a client that sends a receive right to its
+//! child:
+//! \code
+//!   ChildPortHandshake child_port_handshake;
+//!   base::ScopedFD server_write_fd = child_port_handshake.ServerWriteFD();
+//!   std::string server_write_fd_string =
+//!       base::StringPrintf("%d", server_write_fd);
+//!
+//!   pid_t pid = fork();
+//!   if (pid == 0) {
+//!     // Child
+//!
+//!     // Close all file descriptors above STDERR_FILENO except for
+//!     // server_write_fd. Let the child know what file descriptor to use for
+//!     // server_write_fd by passing it as argv[1]. Example code for the child
+//!     // process is below.
+//!     CloseMultipleNowOrOnExec(STDERR_FILENO + 1, server_write_fd);
+//!     execlp("child", "child", server_write_fd_string.c_str(), nullptr);
+//!   }
+//!
+//!   // Parent
+//!
+//!   // Close the child’s end of the pipe.
+//!   server_write_fd.reset();
+//!
+//!   // Make a new Mach receive right.
+//!   base::mac::ScopedMachReceiveRight
+//!       receive_right(NewMachPort(MACH_PORT_RIGHT_RECEIVE));
+//!
+//!   // Make a send right corresponding to the receive right.
+//!   mach_port_t send_right;
+//!   mach_msg_type_name_t send_right_type;
+//!   mach_port_extract_right(mach_task_self(),
+//!                           receive_right.get(),
+//!                           MACH_MSG_TYPE_MAKE_SEND,
+//!                           &send_right,
+//!                           &send_right_type);
+//!   base::mac::ScopedMachSendRight send_right_owner(send_right);
+//!
+//!   // Send the receive right to the child process, retaining the send right
+//!   // for use in the parent process.
+//!   if (child_port_handshake.RunClient(receive_right,
+//!                                      MACH_MSG_TYPE_MOVE_RECEIVE)) {
+//!     ignore_result(receive_right.release());
+//!   }
+//! \endcode
+//!
+//! Example child process, running a server that receives a receive right from
+//! its parent:
+//! \code
+//!   int main(int argc, char* argv[]) {
+//!     // The parent passed server_write_fd in argv[1].
+//!     base::ScopedFD server_write_fd(atoi(argv[1]));
+//!
+//!     // Obtain a receive right from the parent process.
+//!     base::mac::ScopedMachReceiveRight receive_right(
+//!         ChildPortHandshake::RunServerForFD(
+//!             server_write_fd.Pass(),
+//!             ChildPortHandshake::PortRightType::kReceiveRight));
+//!   }
+//! \endcode
+class ChildPortHandshake {
  public:
-  //! \brief Initializes the server.
+  //! \brief Controls whether a receive or send right is expected to be
-  //!
+  //!     obtained from the client by the server’s call to RunServer().
-  //! This creates the pipe so that the “read” side can be obtained by calling
+  enum class PortRightType {
-  //! ReadPipeFD().
+    //! \brief The server expects to receive a receive right.
-  ChildPortHandshake();
+    kReceiveRight = 0,
 
+    //! \brief The server expects to receive a send or send-once right.
+    kSendRight,
+  };
+
+  ChildPortHandshake();
   ~ChildPortHandshake();
 
   //! \brief Obtains the “read” side of the pipe, to be used by the client.
   //!
-  //! Callers must obtain this file descriptor and arrange for the caller to
+  //! This file descriptor must be passed to RunClientForFD().
-  //! have access to it before calling RunServer().
   //!
   //! \return The file descriptor that the client should read from.
-  int ReadPipeFD() const;
+  base::ScopedFD ClientReadFD();
+
+  //! \brief Obtains the “write” side of the pipe, to be used by the server.
+  //!
+  //! This file descriptor must be passed to RunServerForFD().
+  //!
+  //! \return The file descriptor that the server should write to.
+  base::ScopedFD ServerWriteFD();
 
   //! \brief Runs the server.
   //!
-  //! This method performs these tasks:
+  //! This method closes the “read” side of the pipe in-process, so that the
-  //!  - Closes the “read” side of the pipe in-process, so that the client
+  //! client process holds the only file descriptor that can read from the pipe.
-  //!    process holds the only file descriptor that can read from the pipe.
+  //! It then calls RunServerForFD() using the “write” side of the pipe. If
+  //! ClientReadFD() has already been called in the server process, the caller
+  //! must ensure that the file descriptor returned by ClientReadFD() is closed
+  //! prior to calling this method.
+  mach_port_t RunServer(PortRightType port_right_type);
+
+  //! \brief Runs the client.
+  //!
+  //! This method closes the “write” side of the pipe in-process, so that the
+  //! server process holds the only file descriptor that can write to the pipe.
+  //! It then calls RunClientForFD() using the “read” side of the pipe. If
+  //! ServerWriteFD() has already been called in the client process, the caller
+  //! must ensure that the file descriptor returned by ServerWriteFD() is closed
+  //! prior to calling this method.
+  //!
+  //! \return `true` on success, `false` on failure with a message logged.
+  bool RunClient(mach_port_t port, mach_msg_type_name_t right_type);
+
+  //! \brief Runs the server.
+  //!
+  //! If a ChildPortHandshake object is available, don’t call this static
+  //! function. Instead, call RunServer(), which wraps this function. When using
+  //! this function, the caller is responsible for ensuring that the client
+  //! “read” side of the pipe is closed in the server process prior to calling
+  //! this function.
+  //!
+  //! This function performs these tasks:
   //!  - Creates a random token and sends it via the pipe.
   //!  - Checks its service in with the bootstrap server, and sends the name
   //!    of its bootstrap service mapping via the pipe.
@@ -114,33 +211,35 @@ class ChildPortHandshake : public ChildPortServer::Interface {
   //!    interpret and validate it, and if the message is valid, returns the
   //!    port right extracted from the message. If the message is not valid,
   //!    this method will continue waiting for a valid message. Valid messages
-  //!    are properly formatted and have the correct token. If a valid message
+  //!    are properly formatted and have the correct token. The right carried in
-  //!    carries a send or send-once right, it will be returned. If a valid
+  //!    a valid message will be returned. If a message is not valid, this
-  //!    message contains a receive right, it will be destroyed and
-  //!    `MACH_PORT_NULL` will be returned. If a message is not valid, this
   //!    method will continue waiting for pipe EOF or a valid message.
   //!  - When notified of pipe EOF, returns `MACH_PORT_NULL`.
   //!  - Regardless of return value, destroys the server’s receive right and
   //!    closes the pipe.
   //!
-  //! \return On success, the send or send-once right to the port provided by
+  //! \param[in] port_right_type The port right type expected to be received
-  //!     the client. The caller takes ownership of this right. On failure,
+  //!     from the client. If the port right received from the client does not
-  //!     `MACH_PORT_NULL`, indicating that the client did not check in properly
+  //!     match the expected type, the received port right will be destroyed,
-  //!     before terminating, where termination is detected by noticing that the
+  //!     and `MACH_PORT_NULL` will be returned.
-  //!     read side of the shared pipe has closed. On failure, a message
+  //!
-  //!     indiciating the nature of the failure will be logged.
+  //! \return On success, the port right provided by the client. The caller
-  mach_port_t RunServer();
+  //!     takes ownership of this right. On failure, `MACH_PORT_NULL`,
-
+  //!     indicating that the client did not check in properly before
-  // ChildPortServer::Interface:
+  //!     terminating, where termination is detected by detecting that the read
-  kern_return_t HandleChildPortCheckIn(child_port_server_t server,
+  //!     side of the shared pipe has closed. On failure, a message indicating
-                                       child_port_token_t token,
+  //!     the nature of the failure will be logged.
-                                       mach_port_t port,
+  static mach_port_t RunServerForFD(base::ScopedFD server_write_fd,
-                                       mach_msg_type_name_t right_type,
+                                    PortRightType port_right_type);
-                                       const mach_msg_trailer_t* trailer,
-                                       bool* destroy_request) override;
 
   //! \brief Runs the client.
   //!
+  //! If a ChildPortHandshake object is available, don’t call this static
+  //! function. Instead, call RunClient(), which wraps this function. When using
+  //! this function, the caller is responsible for ensuring that the server
+  //! “write” side of the pipe is closed in the client process prior to calling
+  //! this function.
+  //!
   //! This function performs these tasks:
   //!  - Reads the token from the pipe.
   //!  - Reads the bootstrap service name from the pipe.
@@ -155,32 +254,42 @@ class ChildPortHandshake : public ChildPortServer::Interface {
   //! check-in to occur without blocking to wait for a reply.
   //!
   //! \param[in] pipe_read The “read” side of the pipe shared with the server
-  //!     process.
+  //!     process. This function takes ownership of this file descriptor, and
-  //! \param[in] port The port that will be passed to the server by
+  //!     will close it prior to returning.
+  //! \param[in] port The port right that will be passed to the server by
   //!     `child_port_check_in()`.
-  //! \param[in] right_type The right type to furnish the parent with. If \a
+  //! \param[in] right_type The right type to furnish the server with. If \a
   //!     port is a send right, this can be `MACH_MSG_TYPE_COPY_SEND` or
   //!     `MACH_MSG_TYPE_MOVE_SEND`. If \a port is a send-once right, this can
-  //!      be `MACH_MSG_TYPE_MOVE_SEND_ONCE`. If \a port is a receive right,
+  //!     be `MACH_MSG_TYPE_MOVE_SEND_ONCE`. If \a port is a receive right, this
-  //!      this can be `MACH_MSG_TYPE_MAKE_SEND`. `MACH_MSG_TYPE_MOVE_RECEIVE`
+  //!     can be `MACH_MSG_TYPE_MAKE_SEND`, `MACH_MSG_TYPE_MAKE_SEND_ONCE`, or
-  //!      is supported by the client interface but will be silently rejected by
+  //!     `MACH_MSG_TYPE_MOVE_RECEIVE`.
-  //!      server run by RunServer(), which expects to receive only send or
+  //!
-  //!      send-once rights.
+  //! \return `true` on success, `false` on failure with a message logged. On
-  static void RunClient(int pipe_read,
+  //!     failure, the port right corresponding to a \a right_type of
-                        mach_port_t port,
+  //!     `MACH_MSG_TYPE_MOVE_*` is not consumed, and the caller must dispose of
-                        mach_msg_type_name_t right_type);
+  //!     the right if necessary.
+  static bool RunClientForFD(base::ScopedFD client_read_fd,
+                             mach_port_t port,
+                             mach_msg_type_name_t right_type);
 
  private:
   //! \brief Runs the read-from-pipe portion of the client’s side of the
   //!     handshake. This is an implementation detail of RunClient and is only
   //!     exposed for testing purposes.
   //!
+  //! When using this function and RunClientInternal_SendCheckIn(), the caller
+  //! is responsible for closing \a pipe_read at an appropriate time, normally
+  //! after calling RunClientInternal_SendCheckIn().
+  //!
   //! \param[in] pipe_read The “read” side of the pipe shared with the server
   //!     process.
   //! \param[out] token The token value read from \a pipe_read.
   //! \param[out] service_name The service name as registered with the bootstrap
   //!     server, read from \a pipe_read.
-  static void RunClientInternal_ReadPipe(int pipe_read,
+  //!
+  //! \return `true` on success, `false` on failure with a message logged.
+  static bool RunClientInternal_ReadPipe(int pipe_read,
                                          child_port_token_t* token,
                                          std::string* service_name);
 
@@ -188,34 +297,25 @@ class ChildPortHandshake : public ChildPortServer::Interface {
   //!     This is an implementation detail of RunClient and is only exposed for
   //!     testing purposes.
   //!
+  //! When using this RunClientInternal_ReadPipe() and this function, the caller
+  //! is responsible for closing the “read” side of the pipe at an appropriate
+  //! time, normally after calling this function.
+  //!
   //! \param[in] service_name The service name as registered with the bootstrap
   //!     server, to be looked up with `bootstrap_look_up()`.
   //! \param[in] token The token value to provide during check-in.
   //! \param[in] port The port that will be passed to the server by
   //!     `child_port_check_in()`.
-  //! \param[in] right_type The right type to furnish the parent with.
+  //! \param[in] right_type The right type to furnish the server with.
-  static void RunClientInternal_SendCheckIn(const std::string& service_name,
+  //!
+  //! \return `true` on success, `false` on failure with a message logged.
+  static bool RunClientInternal_SendCheckIn(const std::string& service_name,
                                             child_port_token_t token,
                                             mach_port_t port,
                                             mach_msg_type_name_t right_type);
 
-  // Communicates the token from RunServer(), where it’s generated, to
+  base::ScopedFD client_read_fd_;
-  // HandleChildPortCheckIn(), where it’s validated.
+  base::ScopedFD server_write_fd_;
-  child_port_token_t token_;
-
-  base::ScopedFD pipe_read_;
-  base::ScopedFD pipe_write_;
-
-  // Communicates the port received from the client from
-  // HandleChildPortCheckIn(), where it’s received, to RunServer(), where it’s
-  // returned. This is strongly-owned, but ownership is transferred to
-  // RunServer()’s caller.
-  mach_port_t child_port_;
-
-  // Communicates that a check-in with a valid token was received by
-  // HandleChildPortCheckIn(), and that the value of child_port_ should be
-  // returned to RunServer()’s caller.
-  bool checked_in_;
 
   friend class test::ChildPortHandshakeTest;
 

util/mach/child_port_handshake_test.cc

@@ -26,161 +26,355 @@ namespace {
 
 class ChildPortHandshakeTest : public Multiprocess {
  public:
-  enum TestType {
+  enum class ClientProcess {
-    kTestTypeChildChecksIn = 0,
+    // The child runs the client and the parent runs the server.
-    kTestTypeChildDoesNotCheckIn_ReadsPipe,
+    kChildClient = 0,
-    kTestTypeChildDoesNotCheckIn,
+
-    kTestTypeTokenIncorrect,
+    // The parent runs the client and the child runs the server.
-    kTestTypeTokenIncorrectThenCorrect,
+    kParentClient,
   };
 
-  explicit ChildPortHandshakeTest(TestType test_type)
+  enum class TestType {
-      : Multiprocess(), child_port_handshake_(), test_type_(test_type) {}
+    // The client checks in with the server, transferring a receive right.
-  ~ChildPortHandshakeTest() {}
+    kClientChecksIn_ReceiveRight = 0,
+
+    // In this test, the client checks in with the server normally. It sends a
+    // copy of its bootstrap port to the server, because both parent and child
+    // should have the same bootstrap port, allowing for verification.
+    kClientChecksIn_SendRight,
+
+    // The client checks in with the server, transferring a send-once right.
+    kClientChecksIn_SendOnceRight,
+
+    // In this test, the client reads from its pipe, and subsequently exits
+    // without checking in. This tests that the server properly detects that it
+    // has lost its client after sending instructions to it via the pipe, while
+    // waiting for a check-in message.
+    kClientDoesNotCheckIn,
+
+    // In this test, the client exits without checking in. This tests that the
+    // server properly detects that it has lost a client. Whether or not the
+    // client closes the pipe before the server writes to it is a race, and the
+    // server needs to be able to detect client loss in both cases, so the
+    // ClientDoesNotCheckIn_ReadsPipe and NoClient tests also exist to test
+    // these individual cases more deterministically.
+    kClientDoesNotCheckIn_ReadsPipe,
+
+    // In this test, the client checks in with the server with an incorrect
+    // token value and a copy of its own task port. The server should reject the
+    // message because of the invalid token, and return MACH_PORT_NULL to its
+    // caller.
+    kTokenIncorrect,
+
+    // In this test, the client checks in with the server with an incorrect
+    // token value and a copy of its own task port, and subsequently, the
+    // correct token value and a copy of its bootstrap port. The server should
+    // reject the first because of the invalid token, but it should continue
+    // waiting for a message with a valid token as long as the pipe remains
+    // open. It should wind wind up returning the bootstrap port, allowing for
+    // verification.
+    kTokenIncorrectThenCorrect,
+
+    // The server dies. The failure should be reported in the client. This test
+    // type is only compatible with ClientProcess::kParentClient.
+    kServerDies,
+  };
+
+  ChildPortHandshakeTest(ClientProcess client_process, TestType test_type)
+      : Multiprocess(),
+        child_port_handshake_(),
+        client_process_(client_process),
+        test_type_(test_type) {
+  }
+
+  ~ChildPortHandshakeTest() {
+  }
 
  private:
+  void RunServer() {
+    if (test_type_ == TestType::kServerDies) {
+      return;
+    }
+
+    base::mac::ScopedMachReceiveRight receive_right;
+    base::mac::ScopedMachSendRight send_right;
+    if (test_type_ == TestType::kClientChecksIn_ReceiveRight) {
+      receive_right.reset(child_port_handshake_.RunServer(
+          ChildPortHandshake::PortRightType::kReceiveRight));
+    } else {
+      send_right.reset(child_port_handshake_.RunServer(
+          ChildPortHandshake::PortRightType::kSendRight));
+    }
+
+    switch (test_type_) {
+      case TestType::kClientChecksIn_ReceiveRight:
+        EXPECT_TRUE(receive_right.is_valid());
+        break;
+
+      case TestType::kClientChecksIn_SendRight:
+      case TestType::kTokenIncorrectThenCorrect:
+        EXPECT_EQ(bootstrap_port, send_right);
+        break;
+
+      case TestType::kClientChecksIn_SendOnceRight:
+        EXPECT_TRUE(send_right.is_valid());
+        EXPECT_NE(bootstrap_port, send_right);
+        break;
+
+      case TestType::kClientDoesNotCheckIn:
+      case TestType::kClientDoesNotCheckIn_ReadsPipe:
+      case TestType::kTokenIncorrect:
+        EXPECT_FALSE(send_right.is_valid());
+        break;
+
+      case TestType::kServerDies:
+        // This was special-cased as an early return above.
+        FAIL();
+        break;
+    }
+  }
+
+  void RunClient() {
+    switch (test_type_) {
+      case TestType::kClientChecksIn_SendRight: {
+        ASSERT_TRUE(child_port_handshake_.RunClient(bootstrap_port,
+                                                    MACH_MSG_TYPE_COPY_SEND));
+        break;
+      }
+
+      case TestType::kClientChecksIn_ReceiveRight: {
+        mach_port_t receive_right = NewMachPort(MACH_PORT_RIGHT_RECEIVE);
+        ASSERT_TRUE(child_port_handshake_.RunClient(
+              receive_right, MACH_MSG_TYPE_MOVE_RECEIVE));
+        break;
+      }
+
+      case TestType::kClientChecksIn_SendOnceRight: {
+        base::mac::ScopedMachReceiveRight receive_right(
+            NewMachPort(MACH_PORT_RIGHT_RECEIVE));
+        ASSERT_TRUE(child_port_handshake_.RunClient(
+            receive_right.get(), MACH_MSG_TYPE_MAKE_SEND_ONCE));
+        break;
+      }
+
+      case TestType::kClientDoesNotCheckIn: {
+        child_port_handshake_.ServerWriteFD().reset();
+        child_port_handshake_.ClientReadFD().reset();
+        break;
+      }
+
+      case TestType::kClientDoesNotCheckIn_ReadsPipe: {
+        // Don’t run the standard client routine. Instead, drain the pipe, which
+        // will get the parent to the point that it begins waiting for a
+        // check-in message. Then, exit. The pipe is drained using the same
+        // implementation that the real client would use.
+        child_port_handshake_.ServerWriteFD().reset();
+        base::ScopedFD client_read_fd = child_port_handshake_.ClientReadFD();
+        child_port_token_t token;
+        std::string service_name;
+        ASSERT_TRUE(ChildPortHandshake::RunClientInternal_ReadPipe(
+            client_read_fd.get(), &token, &service_name));
+        break;
+      }
+
+      case TestType::kTokenIncorrect: {
+        // Don’t run the standard client routine. Instead, read the token and
+        // service name, mutate the token, and then check in with the bad token.
+        // The parent should reject the message.
+        child_port_handshake_.ServerWriteFD().reset();
+        base::ScopedFD client_read_fd = child_port_handshake_.ClientReadFD();
+        child_port_token_t token;
+        std::string service_name;
+        ASSERT_TRUE(ChildPortHandshake::RunClientInternal_ReadPipe(
+            client_read_fd.get(), &token, &service_name));
+        child_port_token_t bad_token = ~token;
+        ASSERT_TRUE(ChildPortHandshake::RunClientInternal_SendCheckIn(
+            service_name,
+            bad_token,
+            mach_task_self(),
+            MACH_MSG_TYPE_COPY_SEND));
+        break;
+      }
+
+      case TestType::kTokenIncorrectThenCorrect: {
+        // Don’t run the standard client routine. Instead, read the token and
+        // service name. Mutate the token, and check in with the bad token,
+        // expecting the parent to reject the message. Then, check in with the
+        // correct token, expecting the parent to accept it.
+        child_port_handshake_.ServerWriteFD().reset();
+        base::ScopedFD client_read_fd = child_port_handshake_.ClientReadFD();
+        child_port_token_t token;
+        std::string service_name;
+        ASSERT_TRUE(ChildPortHandshake::RunClientInternal_ReadPipe(
+            client_read_fd.release(), &token, &service_name));
+        child_port_token_t bad_token = ~token;
+        ASSERT_TRUE(ChildPortHandshake::RunClientInternal_SendCheckIn(
+            service_name,
+            bad_token,
+            mach_task_self(),
+            MACH_MSG_TYPE_COPY_SEND));
+        ASSERT_TRUE(ChildPortHandshake::RunClientInternal_SendCheckIn(
+            service_name, token, bootstrap_port, MACH_MSG_TYPE_COPY_SEND));
+        break;
+      }
+
+      case TestType::kServerDies: {
+        ASSERT_EQ(ClientProcess::kParentClient, client_process_);
+        ASSERT_FALSE(child_port_handshake_.RunClient(bootstrap_port,
+                                                     MACH_MSG_TYPE_COPY_SEND));
+        break;
+      }
+    }
+  }
+
   // Multiprocess:
 
   void MultiprocessParent() override {
-    base::mac::ScopedMachSendRight child_port(
+    switch (client_process_) {
-        child_port_handshake_.RunServer());
+      case ClientProcess::kChildClient:
-    switch (test_type_) {
+        RunServer();
-      case kTestTypeChildChecksIn:
-      case kTestTypeTokenIncorrectThenCorrect:
-        EXPECT_EQ(bootstrap_port, child_port);
         break;
-
+      case ClientProcess::kParentClient:
-      case kTestTypeChildDoesNotCheckIn_ReadsPipe:
+        RunClient();
-      case kTestTypeChildDoesNotCheckIn:
-      case kTestTypeTokenIncorrect:
-        EXPECT_EQ(kMachPortNull, child_port);
         break;
     }
   }
 
   void MultiprocessChild() override {
-    int read_pipe = child_port_handshake_.ReadPipeFD();
+    switch (client_process_) {
-    switch (test_type_) {
+      case ClientProcess::kChildClient:
-      case kTestTypeChildChecksIn:
+        RunClient();
-        ChildPortHandshake::RunClient(
-            read_pipe, bootstrap_port, MACH_MSG_TYPE_COPY_SEND);
         break;
-
+      case ClientProcess::kParentClient:
-      case kTestTypeChildDoesNotCheckIn_ReadsPipe: {
+        RunServer();
-        // Don’t run the standard client routine. Instead, drain the pipe, which
-        // will get the parent to the point that it begins waiting for a
-        // check-in message. Then, exit. The pipe is drained using the same
-        // implementation that the real client would use.
-        child_port_token_t token;
-        std::string service_name;
-        ChildPortHandshake::RunClientInternal_ReadPipe(
-            read_pipe, &token, &service_name);
         break;
-      }
-
-      case kTestTypeChildDoesNotCheckIn:
-        break;
-
-      case kTestTypeTokenIncorrect: {
-        // Don’t run the standard client routine. Instead, read the token and
-        // service name, mutate the token, and then check in with the bad token.
-        // The parent should reject the message.
-        child_port_token_t token;
-        std::string service_name;
-        ChildPortHandshake::RunClientInternal_ReadPipe(
-            read_pipe, &token, &service_name);
-        child_port_token_t bad_token = ~token;
-        ChildPortHandshake::RunClientInternal_SendCheckIn(
-            service_name, bad_token, mach_task_self(), MACH_MSG_TYPE_COPY_SEND);
-        break;
-      }
-
-      case kTestTypeTokenIncorrectThenCorrect: {
-        // Don’t run the standard client routine. Instead, read the token and
-        // service name. Mutate the token, and check in with the bad token,
-        // expecting the parent to reject the message. Then, check in with the
-        // correct token, expecting the parent to accept it.
-        child_port_token_t token;
-        std::string service_name;
-        ChildPortHandshake::RunClientInternal_ReadPipe(
-            read_pipe, &token, &service_name);
-        child_port_token_t bad_token = ~token;
-        ChildPortHandshake::RunClientInternal_SendCheckIn(
-            service_name, bad_token, mach_task_self(), MACH_MSG_TYPE_COPY_SEND);
-        ChildPortHandshake::RunClientInternal_SendCheckIn(
-            service_name, token, bootstrap_port, MACH_MSG_TYPE_COPY_SEND);
-        break;
-      }
     }
   }
 
  private:
   ChildPortHandshake child_port_handshake_;
+  ClientProcess client_process_;
   TestType test_type_;
 
   DISALLOW_COPY_AND_ASSIGN(ChildPortHandshakeTest);
 };
 
-TEST(ChildPortHandshake, ChildChecksIn) {
+TEST(ChildPortHandshake, ChildClientChecksIn_ReceiveRight) {
-  // In this test, the client checks in with the server normally. It sends a
-  // copy of its bootstrap port to the server, because both parent and child
-  // should have the same bootstrap port, allowing for verification.
-  ChildPortHandshakeTest test(ChildPortHandshakeTest::kTestTypeChildChecksIn);
-  test.Run();
-}
-
-TEST(ChildPortHandshake, ChildDoesNotCheckIn) {
-  // In this test, the client exits without checking in. This tests that the
-  // server properly detects that it has lost a client. Whether or not the
-  // client closes the pipe before the server writes to it is a race, and the
-  // server needs to be able to detect client loss in both cases, so the
-  // ChildDoesNotCheckIn_ReadsPipe and NoChild tests also exist to test these
-  // individual cases more deterministically.
   ChildPortHandshakeTest test(
-      ChildPortHandshakeTest::kTestTypeChildDoesNotCheckIn);
+      ChildPortHandshakeTest::ClientProcess::kChildClient,
+      ChildPortHandshakeTest::TestType::kClientChecksIn_ReceiveRight);
   test.Run();
 }
 
-TEST(ChildPortHandshake, ChildDoesNotCheckIn_ReadsPipe) {
+TEST(ChildPortHandshake, ChildClientChecksIn_SendRight) {
-  // In this test, the client reads from its pipe, and subsequently exits
-  // without checking in. This tests that the server properly detects that it
-  // has lost its client after sending instructions to it via the pipe, while
-  // waiting for a check-in message.
   ChildPortHandshakeTest test(
-      ChildPortHandshakeTest::kTestTypeChildDoesNotCheckIn_ReadsPipe);
+      ChildPortHandshakeTest::ClientProcess::kChildClient,
+      ChildPortHandshakeTest::TestType::kClientChecksIn_SendRight);
   test.Run();
 }
 
-TEST(ChildPortHandshake, TokenIncorrect) {
+TEST(ChildPortHandshake, ChildClientChecksIn_SendOnceRight) {
-  // In this test, the client checks in with the server with an incorrect token
-  // value and a copy of its own task port. The server should reject the message
-  // because of the invalid token, and return MACH_PORT_NULL to its caller.
-  ChildPortHandshakeTest test(ChildPortHandshakeTest::kTestTypeTokenIncorrect);
-  test.Run();
-}
-
-TEST(ChildPortHandshake, TokenIncorrectThenCorrect) {
-  // In this test, the client checks in with the server with an incorrect token
-  // value and a copy of its own task port, and subsequently, the correct token
-  // value and a copy of its bootstrap port. The server should reject the first
-  // because of the invalid token, but it should continue waiting for a message
-  // with a valid token as long as the pipe remains open. It should wind wind up
-  // returning the bootstrap port, allowing for verification.
   ChildPortHandshakeTest test(
-      ChildPortHandshakeTest::kTestTypeTokenIncorrectThenCorrect);
+      ChildPortHandshakeTest::ClientProcess::kChildClient,
+      ChildPortHandshakeTest::TestType::kClientChecksIn_SendOnceRight);
   test.Run();
 }
 
-TEST(ChildPortHandshake, NoChild) {
+TEST(ChildPortHandshake, ChildClientDoesNotCheckIn) {
-  // In this test, the client never checks in with the parent because the child
+  ChildPortHandshakeTest test(
-  // never even runs. This tests that the server properly detects that it has
+      ChildPortHandshakeTest::ClientProcess::kChildClient,
-  // no client at all, and does not terminate execution with an error such as
+      ChildPortHandshakeTest::TestType::kClientDoesNotCheckIn);
+  test.Run();
+}
+
+TEST(ChildPortHandshake, ChildClientDoesNotCheckIn_ReadsPipe) {
+  ChildPortHandshakeTest test(
+      ChildPortHandshakeTest::ClientProcess::kChildClient,
+      ChildPortHandshakeTest::TestType::kClientDoesNotCheckIn_ReadsPipe);
+  test.Run();
+}
+
+TEST(ChildPortHandshake, ChildClientTokenIncorrect) {
+  ChildPortHandshakeTest test(
+      ChildPortHandshakeTest::ClientProcess::kChildClient,
+      ChildPortHandshakeTest::TestType::kTokenIncorrect);
+  test.Run();
+}
+
+TEST(ChildPortHandshake, ChildClientTokenIncorrectThenCorrect) {
+  ChildPortHandshakeTest test(
+      ChildPortHandshakeTest::ClientProcess::kChildClient,
+      ChildPortHandshakeTest::TestType::kTokenIncorrectThenCorrect);
+  test.Run();
+}
+
+TEST(ChildPortHandshake, ParentClientChecksIn_ReceiveRight) {
+  ChildPortHandshakeTest test(
+      ChildPortHandshakeTest::ClientProcess::kParentClient,
+      ChildPortHandshakeTest::TestType::kClientChecksIn_ReceiveRight);
+  test.Run();
+}
+
+TEST(ChildPortHandshake, ParentClientChecksIn_SendRight) {
+  ChildPortHandshakeTest test(
+      ChildPortHandshakeTest::ClientProcess::kParentClient,
+      ChildPortHandshakeTest::TestType::kClientChecksIn_SendRight);
+  test.Run();
+}
+
+TEST(ChildPortHandshake, ParentClientChecksIn_SendOnceRight) {
+  ChildPortHandshakeTest test(
+      ChildPortHandshakeTest::ClientProcess::kParentClient,
+      ChildPortHandshakeTest::TestType::kClientChecksIn_SendOnceRight);
+  test.Run();
+}
+
+TEST(ChildPortHandshake, ParentClientDoesNotCheckIn) {
+  ChildPortHandshakeTest test(
+      ChildPortHandshakeTest::ClientProcess::kParentClient,
+      ChildPortHandshakeTest::TestType::kClientDoesNotCheckIn);
+  test.Run();
+}
+
+TEST(ChildPortHandshake, ParentClientDoesNotCheckIn_ReadsPipe) {
+  ChildPortHandshakeTest test(
+      ChildPortHandshakeTest::ClientProcess::kParentClient,
+      ChildPortHandshakeTest::TestType::kClientDoesNotCheckIn_ReadsPipe);
+  test.Run();
+}
+
+TEST(ChildPortHandshake, ParentClientTokenIncorrect) {
+  ChildPortHandshakeTest test(
+      ChildPortHandshakeTest::ClientProcess::kParentClient,
+      ChildPortHandshakeTest::TestType::kTokenIncorrect);
+  test.Run();
+}
+
+TEST(ChildPortHandshake, ParentClientTokenIncorrectThenCorrect) {
+  ChildPortHandshakeTest test(
+      ChildPortHandshakeTest::ClientProcess::kParentClient,
+      ChildPortHandshakeTest::TestType::kTokenIncorrectThenCorrect);
+  test.Run();
+}
+
+TEST(ChildPortHandshake, ParentClientServerDies) {
+  ChildPortHandshakeTest test(
+      ChildPortHandshakeTest::ClientProcess::kParentClient,
+      ChildPortHandshakeTest::TestType::kServerDies);
+  test.Run();
+}
+
+TEST(ChildPortHandshake, NoClient) {
+  // In this test, the client never checks in with the server because it never
+  // even runs. This tests that the server properly detects that it has no
+  // client at all, and does not terminate execution with an error such as
   // “broken pipe” when attempting to send instructions to the client. This test
-  // is similar to ChildDoesNotCheckIn, but because there’s no child at all, the
+  // is similar to kClientDoesNotCheckIn, but because there’s no client at all,
-  // server is guaranteed to see that its pipe partner is gone.
+  // the server is guaranteed to see that its pipe partner is gone.
   ChildPortHandshake child_port_handshake;
-  base::mac::ScopedMachSendRight child_port(child_port_handshake.RunServer());
+  base::mac::ScopedMachSendRight child_port(child_port_handshake.RunServer(
-  EXPECT_EQ(kMachPortNull, child_port);
+      ChildPortHandshake::PortRightType::kSendRight));
+  EXPECT_FALSE(child_port.is_valid());
 }
 
 }  // namespace

util/mach/mach_message.cc

@@ -20,6 +20,7 @@
 #include <limits>
 
 #include "base/logging.h"
+#include "base/mac/mach_logging.h"
 #include "util/misc/clock.h"
 #include "util/misc/implicit_cast.h"
 
@@ -249,4 +250,37 @@ pid_t AuditPIDFromMachMessageTrailer(const mach_msg_trailer_t* trailer) {
   return audit_pid;
 }
 
+bool MachMessageDestroyReceivedPort(mach_port_t port,
+                                    mach_msg_type_name_t port_right_type) {
+  // This implements a subset of 10.10.5
+  // xnu-2782.40.9/libsyscall/mach/mach_msg.c mach_msg_destroy_port() that deals
+  // only with port rights that can be received in Mach messages.
+  switch (port_right_type) {
+    case MACH_MSG_TYPE_PORT_RECEIVE: {
+      kern_return_t kr = mach_port_mod_refs(
+          mach_task_self(), port, MACH_PORT_RIGHT_RECEIVE, -1);
+      if (kr != KERN_SUCCESS) {
+        MACH_LOG(ERROR, kr) << "mach_port_mod_refs";
+        return false;
+      }
+      return true;
+    }
+
+    case MACH_MSG_TYPE_PORT_SEND:
+    case MACH_MSG_TYPE_PORT_SEND_ONCE: {
+      kern_return_t kr = mach_port_deallocate(mach_task_self(), port);
+      if (kr != KERN_SUCCESS) {
+        MACH_LOG(ERROR, kr) << "mach_port_deallocate";
+        return false;
+      }
+      return true;
+    }
+
+    default: {
+      LOG(ERROR) << "unexpected port right type " << port_right_type;
+      return false;
+    }
+  }
+}
+
 }  // namespace crashpad

util/mach/mach_message.h

@@ -174,6 +174,21 @@ const mach_msg_trailer_t* MachMessageTrailerFromHeader(
 //!     audit information.
 pid_t AuditPIDFromMachMessageTrailer(const mach_msg_trailer_t* trailer);
 
+//! \brief Destroys or deallocates a Mach port received in a Mach message.
+//!
+//! This function disposes of port rights received in a Mach message. Receive
+//! rights will be destroyed with `mach_port_mod_refs()`. Send and send-once
+//! rights will be deallocated with `mach_port_deallocate()`.
+//!
+//! \param[in] port The port to destroy or deallocate.
+//! \param[in] port_right_type The right type held for \a port:
+//!     `MACH_MSG_TYPE_PORT_RECEIVE`, `MACH_MSG_TYPE_PORT_SEND`, or
+//!     `MACH_MSG_TYPE_PORT_SEND_ONCE`.
+//!
+//! \return `true` on success, or `false` on failure with a message logged.
+bool MachMessageDestroyReceivedPort(mach_port_t port,
+                                    mach_msg_type_name_t port_right_type);
+
 }  // namespace crashpad
 
 #endif  // CRASHPAD_UTIL_MACH_MACH_MESSAGE_H_

util/mach/mach_message_server_test.cc

@@ -216,8 +216,8 @@ class TestMachMessageServer : public MachMessageServer::Interface,
         MACH_MSGH_BITS(MACH_MSG_TYPE_MOVE_SEND, MACH_MSG_TYPE_MOVE_SEND) |
         (options_.client_send_complex ? MACH_MSGH_BITS_COMPLEX : 0);
     EXPECT_EQ(expect_msgh_bits, request->header.msgh_bits);
-    EXPECT_EQ(options_.client_send_large ? sizeof(LargeRequestMessage) :
+    EXPECT_EQ(options_.client_send_large ? sizeof(LargeRequestMessage)
-                                           sizeof(RequestMessage),
+                                         : sizeof(RequestMessage),
               request->header.msgh_size);
     if (options_.client_reply_port_type == Options::kReplyPortNormal) {
       EXPECT_EQ(RemotePort(), request->header.msgh_remote_port);
@@ -277,8 +277,8 @@ class TestMachMessageServer : public MachMessageServer::Interface,
 
   std::set<mach_msg_id_t> MachMessageServerRequestIDs() override {
     const mach_msg_id_t request_ids[] = {kRequestMessageID};
-    return std::set<mach_msg_id_t>(
+    return std::set<mach_msg_id_t>(&request_ids[0],
-        &request_ids[0], &request_ids[arraysize(request_ids)]);
+                                   &request_ids[arraysize(request_ids)]);
   }
 
   mach_msg_size_t MachMessageServerRequestSize() override {
@@ -368,8 +368,8 @@ class TestMachMessageServer : public MachMessageServer::Interface,
         EXPECT_EQ(MACH_PORT_TYPE_SEND, type);
 
         // Destroy the resources here.
-        kr = mach_port_deallocate(
+        kr = mach_port_deallocate(mach_task_self(),
-            mach_task_self(), parent_complex_message_port_);
+                                  parent_complex_message_port_);
         EXPECT_EQ(KERN_SUCCESS, kr)
             << MachErrorMessage(kr, "mach_port_deallocate");
       }
@@ -378,8 +378,8 @@ class TestMachMessageServer : public MachMessageServer::Interface,
       // this task so soon. It’s possible that something else in this task could
       // have reused the name, but it’s unlikely for that to have happened in
       // this test environment.
-      kr = mach_port_type(
+      kr =
-          mach_task_self(), parent_complex_message_port_, &type);
+          mach_port_type(mach_task_self(), parent_complex_message_port_, &type);
       EXPECT_EQ(KERN_INVALID_NAME, kr)
           << MachErrorMessage(kr, "mach_port_type");
     }

util/mach/mach_message_test.cc

@@ -16,6 +16,7 @@
 
 #include <unistd.h>
 
+#include "base/basictypes.h"
 #include "base/mac/scoped_mach_port.h"
 #include "gtest/gtest.h"
 #include "test/mac/mach_errors.h"
@@ -148,6 +149,55 @@ TEST(MachMessage, AuditPIDFromMachMessageTrailer) {
   EXPECT_EQ(getpid(), AuditPIDFromMachMessageTrailer(&receive.trailer));
 }
 
+TEST(MachMessage, MachMessageDestroyReceivedPort) {
+  mach_port_t port = NewMachPort(MACH_PORT_RIGHT_RECEIVE);
+  ASSERT_NE(kMachPortNull, port);
+  EXPECT_TRUE(MachMessageDestroyReceivedPort(port, MACH_MSG_TYPE_PORT_RECEIVE));
+
+  base::mac::ScopedMachReceiveRight receive(
+      NewMachPort(MACH_PORT_RIGHT_RECEIVE));
+  mach_msg_type_name_t right_type;
+  kern_return_t kr = mach_port_extract_right(mach_task_self(),
+                                             receive.get(),
+                                             MACH_MSG_TYPE_MAKE_SEND,
+                                             &port,
+                                             &right_type);
+  ASSERT_EQ(KERN_SUCCESS, kr)
+      << MachErrorMessage(kr, "mach_port_extract_right");
+  ASSERT_EQ(receive, port);
+  ASSERT_EQ(implicit_cast<mach_msg_type_name_t>(MACH_MSG_TYPE_PORT_SEND),
+            right_type);
+  EXPECT_TRUE(MachMessageDestroyReceivedPort(port, MACH_MSG_TYPE_PORT_SEND));
+
+  kr = mach_port_extract_right(mach_task_self(),
+                               receive.get(),
+                               MACH_MSG_TYPE_MAKE_SEND_ONCE,
+                               &port,
+                               &right_type);
+  ASSERT_EQ(KERN_SUCCESS, kr)
+      << MachErrorMessage(kr, "mach_port_extract_right");
+  ASSERT_NE(kMachPortNull, port);
+  EXPECT_NE(receive, port);
+  ASSERT_EQ(implicit_cast<mach_msg_type_name_t>(MACH_MSG_TYPE_PORT_SEND_ONCE),
+            right_type);
+  EXPECT_TRUE(
+      MachMessageDestroyReceivedPort(port, MACH_MSG_TYPE_PORT_SEND_ONCE));
+
+  kr = mach_port_extract_right(mach_task_self(),
+                               receive.get(),
+                               MACH_MSG_TYPE_MAKE_SEND,
+                               &port,
+                               &right_type);
+  ASSERT_EQ(KERN_SUCCESS, kr)
+      << MachErrorMessage(kr, "mach_port_extract_right");
+  ASSERT_EQ(receive, port);
+  ASSERT_EQ(implicit_cast<mach_msg_type_name_t>(MACH_MSG_TYPE_PORT_SEND),
+            right_type);
+  EXPECT_TRUE(MachMessageDestroyReceivedPort(port, MACH_MSG_TYPE_PORT_RECEIVE));
+  ignore_result(receive.release());
+  EXPECT_TRUE(MachMessageDestroyReceivedPort(port, MACH_MSG_TYPE_PORT_SEND));
+}
+
 }  // namespace
 }  // namespace test
 }  // namespace crashpad