mac: Tolerate dead names for reply ports in the exception handler server

Self-monitoring revealed this CHECK was being hit in the wild:

base::debug::BreakDebugger()                debugger_posix.cc:260
logging::LogMessage::~LogMessage()          logging.cc:759
logging::MachLogMessage::~MachLogMessage()  mach_logging.cc:45
crashpad::ExceptionHandlerServer::Run()     exception_handler_server.cc:108
crashpad::HandlerMain()                     handler_main.cc:744

The MACH_CHECK() was:

108        MACH_CHECK(mr == MACH_MSG_SUCCESS, mr) << "MachMessageServer::Run";

Crash reports captured the full message, including the value of mr:

[0418/015158.777231:FATAL:exception_handler_server.cc(108)] Check failed: mr == MACH_MSG_SUCCESS. MachMessageServer::Run: (ipc/send) invalid destination port (0x10000003)

0x10000003 = MACH_SEND_INVALID_DEST.

This can happen when attempting to send a Mach message to a dead name.
Send (and send-once) rights become dead names when the corresponding
receive right dies. This would not normally happen for exception
requests originating in the kernel. It can happen for requests
originating from a user task: when the user task dies, the receive right
dies with it. All it takes to trigger this CHECK() in crashpad_handler
is for a Crashpad client to die (or be killed) while the handler is
processing a SimulateCrash() that the client originated.

Accept MACH_SEND_INVALID_DEST as a valid return value for
MachMessageServer::Run().

Note that MachMessageServer’s test coverage was already aware of this
behavior. MachMessageServer::Run()’s documentation is updated to reflect
it too.

Change-Id: I483c065d3c5f9a7da410ef3ad54db45ee53aa3c2
Reviewed-on: https://chromium-review.googlesource.com/479093
Commit-Queue: Mark Mentovai <mark@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
This commit is contained in:
Mark Mentovai 2017-04-17 17:03:57 -04:00
parent c64fd3f9b4
commit ddcc74f08f
2 changed files with 13 additions and 2 deletions

View File

@ -105,7 +105,14 @@ class ExceptionHandlerServerRun : public UniversalMachExcServer::Interface,
MachMessageServer::kOneShot,
MachMessageServer::kReceiveLargeIgnore,
kMachMessageTimeoutWaitIndefinitely);
MACH_CHECK(mr == MACH_MSG_SUCCESS, mr) << "MachMessageServer::Run";
// MACH_SEND_INVALID_DEST occurs when attempting to reply to a dead name.
// This can happen if a mach_exc or exc client disappears before a reply
// can be sent to it. Thats unusal for kernel-generated requests, but can
// easily happen if a task sends its own exception request (as
// SimulateCrash() does) and dies before the reply is sent.
MACH_CHECK(mr == MACH_MSG_SUCCESS || mr == MACH_SEND_INVALID_DEST, mr)
<< "MachMessageServer::Run";
}
}

View File

@ -162,7 +162,11 @@ class MachMessageServer {
//! timeout_ms is not #kMachMessageTimeoutWaitIndefinitely). This function
//! has no successful return value when \a persistent is #kPersistent and
//! \a timeout_ms is #kMachMessageTimeoutWaitIndefinitely. On failure,
//! returns a value identifying the nature of the error.
//! returns a value identifying the nature of the error. A request
//! received with a reply port that is (or becomes) a dead name before the
//! reply is sent will result in `MACH_SEND_INVALID_DEST` as a return
//! value, which may or may not be considered an error from the callers
//! perspective.
static mach_msg_return_t Run(Interface* interface,
mach_port_t receive_port,
mach_msg_options_t options,