Add and use scoped-right-returning wrappers for Mach bootstrap routines

This wraps bootstrap_check_in() in BootstrapCheckIn(), and
bootstrap_look_up() in BootstrapLookUp(). The wrappers make it more
difficult to accidentally leak a returned right. They’re easier to use,
encapsulating common error checking and logging, simplifying all call
sites.

TEST=crashpad_util_test MachExtensions.BootstrapCheckInAndLookUp
R=rsesek@chromium.org

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

test/mac/mach_multiprocess.cc

@@ -16,7 +16,6 @@
 
 #include <AvailabilityMacros.h>
 #include <bsm/libbsm.h>
-#include <servers/bootstrap.h>
 
 #include <string>
 
@@ -98,13 +97,8 @@ void MachMultiprocess::PreFork() {
     info_->service_name.append(1, base::RandInt('A', 'Z'));
   }
 
-  mach_port_t local_port;
+  info_->local_port = BootstrapCheckIn(info_->service_name);
-  kern_return_t kr = bootstrap_check_in(bootstrap_port,
+  ASSERT_NE(kMachPortNull, info_->local_port);
-                                        info_->service_name.c_str(),
-                                        &local_port);
-  ASSERT_EQ(BOOTSTRAP_SUCCESS, kr)
-      << BootstrapErrorMessage(kr, "bootstrap_check_in");
-  info_->local_port.reset(local_port);
 }
 
 mach_port_t MachMultiprocess::LocalPort() const {
@@ -224,12 +218,8 @@ void MachMultiprocess::MultiprocessChild() {
   ASSERT_NE(kMachPortNull, info_->local_port);
 
   // The remote port can be obtained from the bootstrap server.
-  mach_port_t remote_port;
+  info_->remote_port = BootstrapLookUp(info_->service_name);
-  kern_return_t kr = bootstrap_look_up(
+  ASSERT_NE(kMachPortNull, info_->remote_port);
-      bootstrap_port, info_->service_name.c_str(), &remote_port);
-  ASSERT_EQ(BOOTSTRAP_SUCCESS, kr)
-      << BootstrapErrorMessage(kr, "bootstrap_look_up");
-  info_->remote_port.reset(remote_port);
 
   // The “hello” message will provide the parent with its remote port, a send
   // right to the child task’s local port receive right. It will also carry a
@@ -246,13 +236,13 @@ void MachMultiprocess::MultiprocessChild() {
   message.port_descriptor.disposition = MACH_MSG_TYPE_COPY_SEND;
   message.port_descriptor.type = MACH_MSG_PORT_DESCRIPTOR;
 
-  kr = mach_msg(&message.header,
+  kern_return_t kr = mach_msg(&message.header,
-                MACH_SEND_MSG,
+                              MACH_SEND_MSG,
-                message.header.msgh_size,
+                              message.header.msgh_size,
-                0,
+                              0,
-                MACH_PORT_NULL,
+                              MACH_PORT_NULL,
-                MACH_MSG_TIMEOUT_NONE,
+                              MACH_MSG_TIMEOUT_NONE,
-                MACH_PORT_NULL);
+                              MACH_PORT_NULL);
   ASSERT_EQ(MACH_MSG_SUCCESS, kr) << MachErrorMessage(kr, "mach_msg");
 
   MachMultiprocessChild();

tools/mac/catch_exception_tool.cc

@@ -14,7 +14,6 @@
 
 #include <getopt.h>
 #include <libgen.h>
-#include <servers/bootstrap.h>
 #include <stdio.h>
 #include <string.h>
 #include <unistd.h>
@@ -263,11 +262,9 @@ int CatchExceptionToolMain(int argc, char* argv[]) {
     return EXIT_FAILURE;
   }
 
-  mach_port_t service_port;
+  base::mac::ScopedMachReceiveRight
-  kern_return_t kr = bootstrap_check_in(
+      service_port(BootstrapCheckIn(options.mach_service));
-      bootstrap_port, options.mach_service.c_str(), &service_port);
+  if (service_port == kMachPortNull) {
-  if (kr != BOOTSTRAP_SUCCESS) {
-    BOOTSTRAP_LOG(ERROR, kr) << "bootstrap_check_in " << options.mach_service;
     return EXIT_FAILURE;
   }
 

tools/mac/exception_port_tool.cc

@@ -16,7 +16,6 @@
 #include <getopt.h>
 #include <libgen.h>
 #include <mach/mach.h>
-#include <servers/bootstrap.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -184,17 +183,14 @@ bool ParseHandlerString(const char* handler_string_ro,
 // |mach_send_right_pool|.
 void ShowBootstrapService(const std::string& service_name,
                           MachSendRightPool* mach_send_right_pool) {
-  mach_port_t service_port;
+  base::mac::ScopedMachSendRight service_port(BootstrapLookUp(service_name));
-  kern_return_t kr = bootstrap_look_up(
+  if (service_port == kMachPortNull) {
-      bootstrap_port, service_name.c_str(), &service_port);
-  if (kr != BOOTSTRAP_SUCCESS) {
-    BOOTSTRAP_LOG(ERROR, kr) << "bootstrap_look_up " << service_name;
     return;
   }
 
-  mach_send_right_pool->AddSendRight(service_port);
+  printf("service %s %#x\n", service_name.c_str(), service_port.get());
 
-  printf("service %s %#x\n", service_name.c_str(), service_port);
+  mach_send_right_pool->AddSendRight(service_port.release());
 }
 
 // Prints information about all exception ports known for |exception_ports|. If
@@ -279,22 +275,18 @@ void ShowExceptionPorts(const ExceptionPorts& exception_ports,
 // desired.
 bool SetExceptionPort(const ExceptionHandlerDescription* description,
                       mach_port_t target_port) {
-  base::mac::ScopedMachSendRight service_port_owner;
+  base::mac::ScopedMachSendRight service_port;
-  exception_handler_t service_port = MACH_PORT_NULL;
-  kern_return_t kr;
   if (description->handler.compare(
           0, strlen(kHandlerBootstrapColon), kHandlerBootstrapColon) == 0) {
     const char* service_name =
         description->handler.c_str() + strlen(kHandlerBootstrapColon);
-    kr = bootstrap_look_up(bootstrap_port, service_name, &service_port);
+    service_port = BootstrapLookUp(service_name);
-    if (kr != BOOTSTRAP_SUCCESS) {
+    if (service_port == kMachPortNull) {
-      BOOTSTRAP_LOG(ERROR, kr) << "bootstrap_look_up " << service_name;
       return false;
     }
 
     // The service port doesn’t need to be added to a MachSendRightPool because
     // it’s not used for display at all. ScopedMachSendRight is sufficient.
-    service_port_owner.reset(service_port);
   } else if (description->handler != kHandlerNull) {
     return false;
   }

util/mach/child_port_handshake.cc

@@ -16,7 +16,6 @@
 
 #include <errno.h>
 #include <pthread.h>
-#include <servers/bootstrap.h>
 #include <sys/event.h>
 #include <sys/socket.h>
 #include <sys/time.h>
@@ -102,15 +101,11 @@ mach_port_t ChildPortHandshake::RunServer() {
       getpid(),
       thread_id,
       base::RandUint64());
-  DCHECK_LT(service_name.size(), implicit_cast<size_t>(BOOTSTRAP_MAX_NAME_LEN));
 
   // Check the new service in with the bootstrap server, obtaining a receive
   // right for it.
-  mach_port_t server_port;
+  base::mac::ScopedMachReceiveRight server_port(BootstrapCheckIn(service_name));
-  kern_return_t kr =
+  CHECK_NE(server_port, kMachPortNull);
-      bootstrap_check_in(bootstrap_port, service_name.c_str(), &server_port);
-  BOOTSTRAP_CHECK(kr == BOOTSTRAP_SUCCESS, kr) << "bootstrap_check_in";
-  base::mac::ScopedMachReceiveRight server_port_owner(server_port);
 
   // Share the service name with the client via the pipe.
   uint32_t service_name_length = service_name.size();
@@ -132,7 +127,8 @@ mach_port_t ChildPortHandshake::RunServer() {
       NewMachPort(MACH_PORT_RIGHT_PORT_SET));
   CHECK_NE(server_port_set, kMachPortNull);
 
-  kr = mach_port_insert_member(mach_task_self(), server_port, server_port_set);
+  kern_return_t kr =
+      mach_port_insert_member(mach_task_self(), server_port, server_port_set);
   MACH_CHECK(kr == KERN_SUCCESS, kr) << "mach_port_insert_member";
 
   // Set up a kqueue to monitor both the server’s receive right and the write
@@ -317,8 +313,6 @@ void ChildPortHandshake::RunClientInternal_ReadPipe(int pipe_read,
   // Read the service name from the pipe.
   uint32_t service_name_length;
   CheckedReadFile(pipe_read, &service_name_length, sizeof(service_name_length));
-  DCHECK_LT(service_name_length,
-            implicit_cast<uint32_t>(BOOTSTRAP_MAX_NAME_LEN));
 
   service_name->resize(service_name_length);
   if (!service_name->empty()) {
@@ -334,14 +328,11 @@ void ChildPortHandshake::RunClientInternal_SendCheckIn(
     mach_msg_type_name_t right_type) {
   // Get a send right to the server by looking up the service with the bootstrap
   // server by name.
-  mach_port_t server_port;
+  base::mac::ScopedMachSendRight server_port(BootstrapLookUp(service_name));
-  kern_return_t kr =
+  CHECK_NE(server_port, kMachPortNull);
-      bootstrap_look_up(bootstrap_port, service_name.c_str(), &server_port);
-  BOOTSTRAP_CHECK(kr == BOOTSTRAP_SUCCESS, kr) << "bootstrap_look_up";
-  base::mac::ScopedMachSendRight server_port_owner(server_port);
 
   // Check in with the server.
-  kr = child_port_check_in(server_port, token, port, right_type);
+  kern_return_t kr = child_port_check_in(server_port, token, port, right_type);
   MACH_CHECK(kr == KERN_SUCCESS, kr) << "child_port_check_in";
 }
 

util/mach/mach_extensions.cc

@@ -21,6 +21,59 @@
 #include "base/mac/mach_logging.h"
 #include "util/mac/mac_util.h"
 
+namespace {
+
+// This forms the internal implementation for BootstrapCheckIn() and
+// BootstrapLookUp(), which follow the same logic aside from the routine called
+// and the right type returned.
+
+struct BootstrapCheckInTraits {
+  using Type = base::mac::ScopedMachReceiveRight;
+  static kern_return_t Call(mach_port_t bootstrap_port,
+                            const char* service_name,
+                            mach_port_t* service_port) {
+    return bootstrap_check_in(bootstrap_port, service_name, service_port);
+  }
+  static const char kName[];
+};
+const char BootstrapCheckInTraits::kName[] = "bootstrap_check_in";
+
+struct BootstrapLookUpTraits {
+  using Type = base::mac::ScopedMachSendRight;
+  static kern_return_t Call(mach_port_t bootstrap_port,
+                            const char* service_name,
+                            mach_port_t* service_port) {
+    return bootstrap_look_up(bootstrap_port, service_name, service_port);
+  }
+  static const char kName[];
+};
+const char BootstrapLookUpTraits::kName[] = "bootstrap_look_up";
+
+template <typename Traits>
+typename Traits::Type BootstrapCheckInOrLookUp(
+    const std::string& service_name) {
+  // bootstrap_check_in() and bootstrap_look_up() silently truncate service
+  // names longer than BOOTSTRAP_MAX_NAME_LEN. This check ensures that the name
+  // will not be truncated.
+  if (service_name.size() >= BOOTSTRAP_MAX_NAME_LEN) {
+    LOG(ERROR) << Traits::kName << " " << service_name << ": name too long";
+    return typename Traits::Type(MACH_PORT_NULL);
+  }
+
+  mach_port_t service_port;
+  kern_return_t kr = Traits::Call(bootstrap_port,
+                                  service_name.c_str(),
+                                  &service_port);
+  if (kr != BOOTSTRAP_SUCCESS) {
+    BOOTSTRAP_LOG(ERROR, kr) << Traits::kName << " " << service_name;
+    service_port = MACH_PORT_NULL;
+  }
+
+  return typename Traits::Type(service_port);
+}
+
+}  // namespace
+
 namespace crashpad {
 
 thread_t MachThreadSelf() {
@@ -97,19 +150,18 @@ exception_mask_t ExcMaskValid() {
   return kExcMaskValid_10_11;
 }
 
-base::mac::ScopedMachSendRight SystemCrashReporterHandler() {
+base::mac::ScopedMachReceiveRight BootstrapCheckIn(
-  const char kSystemCrashReporterServiceName[] = "com.apple.ReportCrash";
+    const std::string& service_name) {
-  exception_handler_t system_crash_reporter_handler;
+  return BootstrapCheckInOrLookUp<BootstrapCheckInTraits>(service_name);
-  kern_return_t kr = bootstrap_look_up(bootstrap_port,
+}
-                                       kSystemCrashReporterServiceName,
-                                       &system_crash_reporter_handler);
-  if (kr != BOOTSTRAP_SUCCESS) {
-    BOOTSTRAP_LOG(ERROR, kr) << "bootstrap_look_up "
-                             << kSystemCrashReporterServiceName;
-    system_crash_reporter_handler = MACH_PORT_NULL;
-  }
 
-  return base::mac::ScopedMachSendRight(system_crash_reporter_handler);
+base::mac::ScopedMachSendRight BootstrapLookUp(
+    const std::string& service_name) {
+  return BootstrapCheckInOrLookUp<BootstrapLookUpTraits>(service_name);
+}
+
+base::mac::ScopedMachSendRight SystemCrashReporterHandler() {
+  return BootstrapLookUp("com.apple.ReportCrash");
 }
 
 }  // namespace crashpad

util/mach/mach_extensions.h

@@ -17,6 +17,8 @@
 
 #include <mach/mach.h>
 
+#include <string>
+
 #include "base/mac/scoped_mach_port.h"
 
 namespace crashpad {
@@ -117,6 +119,29 @@ exception_mask_t ExcMaskAll();
 //! support is present.
 exception_mask_t ExcMaskValid();
 
+//! \brief Makes a `boostrap_check_in()` call to the process’ bootstrap server.
+//!
+//! This function is provided to make it easier to call `bootstrap_check_in()`
+//! while avoiding accidental leaks of the returned receive right.
+//!
+//! \param[in] service_name The service name to check in.
+//!
+//! \return On success, a receive right to the service port. On failure,
+//!     `MACH_PORT_NULL` with a message logged.
+base::mac::ScopedMachReceiveRight BootstrapCheckIn(
+    const std::string& service_name);
+
+//! \brief Makes a `boostrap_look_up()` call to the process’ bootstrap server.
+//!
+//! This function is provided to make it easier to call `bootstrap_look_up()`
+//! while avoiding accidental leaks of the returned send right.
+//!
+//! \param[in] service_name The service name to look up.
+//!
+//! \return On success, a send right to the service port. On failure,
+//!     `MACH_PORT_NULL` with a message logged.
+base::mac::ScopedMachSendRight BootstrapLookUp(const std::string& service_name);
+
 //! \brief Obtains the system’s default Mach exception handler for crash-type
 //!     exceptions.
 //!

util/mach/mach_extensions_test.cc

@@ -15,6 +15,7 @@
 #include "util/mach/mach_extensions.h"
 
 #include "base/mac/scoped_mach_port.h"
+#include "base/rand_util.h"
 #include "gtest/gtest.h"
 #include "test/mac/mach_errors.h"
 #include "util/mac/mac_util.h"
@@ -131,6 +132,44 @@ TEST(MachExtensions, ExcMaskValid) {
   EXPECT_TRUE(ExcMaskValid() & ~ExcMaskAll());
 }
 
+TEST(MachExtensions, BootstrapCheckInAndLookUp) {
+  // This should always exist.
+  base::mac::ScopedMachSendRight
+      report_crash(BootstrapLookUp("com.apple.ReportCrash"));
+  EXPECT_NE(report_crash, kMachPortNull);
+
+  std::string service_name = "com.googlecode.crashpad.test.bootstrap_check_in.";
+  for (int index = 0; index < 16; ++index) {
+    service_name.append(1, base::RandInt('A', 'Z'));
+  }
+
+  {
+    // The new service hasn’t checked in yet, so this should fail.
+    base::mac::ScopedMachSendRight send(BootstrapLookUp(service_name));
+    EXPECT_EQ(kMachPortNull, send);
+
+    // Check it in.
+    base::mac::ScopedMachReceiveRight receive(BootstrapCheckIn(service_name));
+    EXPECT_NE(receive, kMachPortNull);
+
+    // Now it should be possible to look up the new service.
+    send = BootstrapLookUp(service_name);
+    EXPECT_NE(send, kMachPortNull);
+
+    // It shouldn’t be possible to check the service in while it’s active.
+    base::mac::ScopedMachReceiveRight receive_2(BootstrapCheckIn(service_name));
+    EXPECT_EQ(kMachPortNull, receive_2);
+  }
+
+  // The new service should be gone now.
+  base::mac::ScopedMachSendRight send(BootstrapLookUp(service_name));
+  EXPECT_EQ(kMachPortNull, send);
+
+  // It should be possible to check it in again.
+  base::mac::ScopedMachReceiveRight receive(BootstrapCheckIn(service_name));
+  EXPECT_NE(receive, kMachPortNull);
+}
+
 TEST(MachExtensions, SystemCrashReporterHandler) {
   base::mac::ScopedMachSendRight
       system_crash_reporter_handler(SystemCrashReporterHandler());