diff --git a/test/mac/mach_multiprocess.cc b/test/mac/mach_multiprocess.cc index 834b5d67..6d410d38 100644 --- a/test/mac/mach_multiprocess.cc +++ b/test/mac/mach_multiprocess.cc @@ -16,7 +16,6 @@ #include #include -#include #include @@ -98,13 +97,8 @@ void MachMultiprocess::PreFork() { info_->service_name.append(1, base::RandInt('A', 'Z')); } - mach_port_t local_port; - kern_return_t kr = bootstrap_check_in(bootstrap_port, - info_->service_name.c_str(), - &local_port); - ASSERT_EQ(BOOTSTRAP_SUCCESS, kr) - << BootstrapErrorMessage(kr, "bootstrap_check_in"); - info_->local_port.reset(local_port); + info_->local_port = BootstrapCheckIn(info_->service_name); + ASSERT_NE(kMachPortNull, info_->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; - kern_return_t kr = bootstrap_look_up( - 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); + info_->remote_port = BootstrapLookUp(info_->service_name); + ASSERT_NE(kMachPortNull, info_->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, - MACH_SEND_MSG, - message.header.msgh_size, - 0, - MACH_PORT_NULL, - MACH_MSG_TIMEOUT_NONE, - MACH_PORT_NULL); + kern_return_t kr = mach_msg(&message.header, + MACH_SEND_MSG, + message.header.msgh_size, + 0, + MACH_PORT_NULL, + MACH_MSG_TIMEOUT_NONE, + MACH_PORT_NULL); ASSERT_EQ(MACH_MSG_SUCCESS, kr) << MachErrorMessage(kr, "mach_msg"); MachMultiprocessChild(); diff --git a/tools/mac/catch_exception_tool.cc b/tools/mac/catch_exception_tool.cc index 902010be..b70ce0a5 100644 --- a/tools/mac/catch_exception_tool.cc +++ b/tools/mac/catch_exception_tool.cc @@ -14,7 +14,6 @@ #include #include -#include #include #include #include @@ -263,11 +262,9 @@ int CatchExceptionToolMain(int argc, char* argv[]) { return EXIT_FAILURE; } - mach_port_t service_port; - kern_return_t kr = bootstrap_check_in( - bootstrap_port, options.mach_service.c_str(), &service_port); - if (kr != BOOTSTRAP_SUCCESS) { - BOOTSTRAP_LOG(ERROR, kr) << "bootstrap_check_in " << options.mach_service; + base::mac::ScopedMachReceiveRight + service_port(BootstrapCheckIn(options.mach_service)); + if (service_port == kMachPortNull) { return EXIT_FAILURE; } diff --git a/tools/mac/exception_port_tool.cc b/tools/mac/exception_port_tool.cc index 9efff7cf..818ad446 100644 --- a/tools/mac/exception_port_tool.cc +++ b/tools/mac/exception_port_tool.cc @@ -16,7 +16,6 @@ #include #include #include -#include #include #include #include @@ -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; - kern_return_t kr = bootstrap_look_up( - bootstrap_port, service_name.c_str(), &service_port); - if (kr != BOOTSTRAP_SUCCESS) { - BOOTSTRAP_LOG(ERROR, kr) << "bootstrap_look_up " << service_name; + base::mac::ScopedMachSendRight service_port(BootstrapLookUp(service_name)); + if (service_port == kMachPortNull) { 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; - exception_handler_t service_port = MACH_PORT_NULL; - kern_return_t kr; + base::mac::ScopedMachSendRight service_port; 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); - if (kr != BOOTSTRAP_SUCCESS) { - BOOTSTRAP_LOG(ERROR, kr) << "bootstrap_look_up " << service_name; + service_port = BootstrapLookUp(service_name); + if (service_port == kMachPortNull) { 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; } diff --git a/util/mach/child_port_handshake.cc b/util/mach/child_port_handshake.cc index 560943ab..b01af7a7 100644 --- a/util/mach/child_port_handshake.cc +++ b/util/mach/child_port_handshake.cc @@ -16,7 +16,6 @@ #include #include -#include #include #include #include @@ -102,15 +101,11 @@ mach_port_t ChildPortHandshake::RunServer() { getpid(), thread_id, base::RandUint64()); - DCHECK_LT(service_name.size(), implicit_cast(BOOTSTRAP_MAX_NAME_LEN)); // Check the new service in with the bootstrap server, obtaining a receive // right for it. - mach_port_t server_port; - kern_return_t kr = - 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); + base::mac::ScopedMachReceiveRight server_port(BootstrapCheckIn(service_name)); + CHECK_NE(server_port, kMachPortNull); // 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(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; - kern_return_t kr = - 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); + base::mac::ScopedMachSendRight server_port(BootstrapLookUp(service_name)); + CHECK_NE(server_port, kMachPortNull); // 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"; } diff --git a/util/mach/mach_extensions.cc b/util/mach/mach_extensions.cc index c2343c21..64552f1c 100644 --- a/util/mach/mach_extensions.cc +++ b/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::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() { - const char kSystemCrashReporterServiceName[] = "com.apple.ReportCrash"; - exception_handler_t system_crash_reporter_handler; - 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; - } +base::mac::ScopedMachReceiveRight BootstrapCheckIn( + const std::string& service_name) { + return BootstrapCheckInOrLookUp(service_name); +} - return base::mac::ScopedMachSendRight(system_crash_reporter_handler); +base::mac::ScopedMachSendRight BootstrapLookUp( + const std::string& service_name) { + return BootstrapCheckInOrLookUp(service_name); +} + +base::mac::ScopedMachSendRight SystemCrashReporterHandler() { + return BootstrapLookUp("com.apple.ReportCrash"); } } // namespace crashpad diff --git a/util/mach/mach_extensions.h b/util/mach/mach_extensions.h index c8753035..5eeca9ae 100644 --- a/util/mach/mach_extensions.h +++ b/util/mach/mach_extensions.h @@ -17,6 +17,8 @@ #include +#include + #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. //! diff --git a/util/mach/mach_extensions_test.cc b/util/mach/mach_extensions_test.cc index 30013acb..2b987b37 100644 --- a/util/mach/mach_extensions_test.cc +++ b/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());