diff --git a/util/mach/exc_server_variants.cc b/util/mach/exc_server_variants.cc index c57492f9..46e85882 100644 --- a/util/mach/exc_server_variants.cc +++ b/util/mach/exc_server_variants.cc @@ -20,6 +20,7 @@ #include "base/basictypes.h" #include "base/logging.h" #include "util/mach/exc.h" +#include "util/mach/exception_behaviors.h" #include "util/mach/excServer.h" #include "util/mach/mach_exc.h" #include "util/mach/mach_excServer.h" @@ -247,8 +248,8 @@ bool ExcServer::MachMessageServerFunction(const mach_msg_header_t* in_header, // in_request_1 is used for the portion of the request after the codes, // which in theory can be variable-length. The check function will set it. const Request* in_request_1; - kern_return_t kr = MIGCheckRequestExceptionRaiseState( - in_request, &in_request_1); + kern_return_t kr = + MIGCheckRequestExceptionRaiseState(in_request, &in_request_1); if (kr != MACH_MSG_SUCCESS) { SetReplyError(out_header, kr); return true; @@ -287,8 +288,8 @@ bool ExcServer::MachMessageServerFunction(const mach_msg_header_t* in_header, // in_request_1 is used for the portion of the request after the codes, // which in theory can be variable-length. The check function will set it. const Request* in_request_1; - kern_return_t kr = MIGCheckRequestExceptionRaiseStateIdentity( - in_request, &in_request_1); + kern_return_t kr = + MIGCheckRequestExceptionRaiseStateIdentity(in_request, &in_request_1); if (kr != MACH_MSG_SUCCESS) { SetReplyError(out_header, kr); return true; @@ -382,8 +383,8 @@ bool MachExcServer::MachMessageServerFunction( // in_request_1 is used for the portion of the request after the codes, // which in theory can be variable-length. The check function will set it. const Request* in_request_1; - kern_return_t kr = MIGCheckRequestMachExceptionRaiseState( - in_request, &in_request_1); + kern_return_t kr = + MIGCheckRequestMachExceptionRaiseState(in_request, &in_request_1); if (kr != MACH_MSG_SUCCESS) { SetReplyError(out_header, kr); return true; @@ -722,4 +723,13 @@ kern_return_t UniversalMachExcServer::CatchException( destroy_complex_request); } +kern_return_t ExcServerSuccessfulReturnValue(exception_behavior_t behavior, + bool set_thread_state) { + if (!set_thread_state && ExceptionBehaviorHasState(behavior)) { + return MACH_RCV_PORT_DIED; + } + + return KERN_SUCCESS; +} + } // namespace crashpad diff --git a/util/mach/exc_server_variants.h b/util/mach/exc_server_variants.h index 81651cec..d605869a 100644 --- a/util/mach/exc_server_variants.h +++ b/util/mach/exc_server_variants.h @@ -249,14 +249,13 @@ class SimplifiedExcServer : public ExcServer, public ExcServer::Interface { // ExcServer::Interface: - virtual kern_return_t CatchExceptionRaise( - exception_handler_t exception_port, - thread_t thread, - task_t task, - exception_type_t exception, - const exception_data_type_t* code, - mach_msg_type_number_t code_count, - bool* destroy_request) override; + virtual kern_return_t CatchExceptionRaise(exception_handler_t exception_port, + thread_t thread, + task_t task, + exception_type_t exception, + const exception_data_type_t* code, + mach_msg_type_number_t code_count, + bool* destroy_request) override; virtual kern_return_t CatchExceptionRaiseState( exception_handler_t exception_port, exception_type_t exception, @@ -411,26 +410,75 @@ class UniversalMachExcServer // internal::SimplifiedExcServer::Interface: - virtual kern_return_t CatchException( - exception_behavior_t behavior, - exception_handler_t exception_port, - thread_t thread, - task_t task, - exception_type_t exception, - const exception_data_type_t* code, - mach_msg_type_number_t code_count, - thread_state_flavor_t* flavor, - const natural_t* old_state, - mach_msg_type_number_t old_state_count, - thread_state_t new_state, - mach_msg_type_number_t* new_state_count, - bool* destroy_complex_request) override; + virtual kern_return_t CatchException(exception_behavior_t behavior, + exception_handler_t exception_port, + thread_t thread, + task_t task, + exception_type_t exception, + const exception_data_type_t* code, + mach_msg_type_number_t code_count, + thread_state_flavor_t* flavor, + const natural_t* old_state, + mach_msg_type_number_t old_state_count, + thread_state_t new_state, + mach_msg_type_number_t* new_state_count, + bool* destroy_complex_request) override; private: internal::SimplifiedExcServer exc_server_; internal::SimplifiedMachExcServer mach_exc_server_; }; +//! \brief Computes an approriate successful return value for an exception +//! handler function. +//! +//! For exception handlers that respond to state-carrying behaviors, when the +//! handler is called by the kernel (as it is normally), the kernel will attempt +//! to set a new thread state when the exception handler returns successfully. +//! Other code that mimics the kernel’s exception-delivery semantics may +//! implement the same or similar behavior. In some situations, it is +//! undesirable to set a new thread state. If the exception handler were to +//! return unsuccessfully, however, the kernel would continue searching for an +//! exception handler at a wider (task or host) scope. This may also be +//! undesirable. +//! +//! If such exception handlers return `MACH_RCV_PORT_DIED`, the kernel will not +//! set a new thread state and will also not search for another exception +//! handler. See 10.9.4 `xnu-2422.110.17/osfmk/kern/exception.c`. +//! `exception_deliver()` will only set a new thread state if the handler’s +//! return code was `MACH_MSG_SUCCESS` (a synonym for `KERN_SUCCESS`), and +//! subsequently, `exception_triage()` will not search for a new handler if the +//! handler’s return code was `KERN_SUCCESS` or `MACH_RCV_PORT_DIED`. +//! +//! This function allows exception handlers to compute an appropriate return +//! code to influence their caller (the kernel) in the desired way with respect +//! to setting a new thread state while suppressing the caller’s subsequent +//! search for other exception handlers. An exception handler should return the +//! value returned by this function. +//! +//! This function is useful even for `EXC_CRASH` handlers, where returning +//! `KERN_SUCCESS` and allowing the kernel to set a new thread state has been +//! observed to cause a perceptible and unnecessary waste of time. The victim +//! task in an `EXC_CRASH` handler is already being terminated and is no longer +//! schedulable, so there is no point in setting the states of any of its +//! threads. +//! +//! \param[in] behavior The behavior of the exception handler as invoked. This +//! may be taken directly from the \a behavior parameter of +//! internal::SimplifiedExcServer::Interface::CatchException(), for example. +//! \param[in] set_thread_state `true` if the handler would like its caller to +//! set the new thread state using the \a flavor, \a new_state, and \a +//! new_state_count out parameters. This can only happen when \a behavior is +//! a state-carrying behavior. +//! +//! \return `KERN_SUCCESS` or `MACH_RCV_PORT_DIED`. `KERN_SUCCESS` is used when +//! \a behavior is not a state-carrying behavior, or when it is a +//! state-carrying behavior and \a set_thread_state is `true`. +//! `MACH_RCV_PORT_DIED` is used when \a behavior is a state-carrying +//! behavior and \a set_thread_state is `false`. +kern_return_t ExcServerSuccessfulReturnValue(exception_behavior_t behavior, + bool set_thread_state); + } // namespace crashpad #endif // CRASHPAD_UTIL_MACH_EXC_SERVER_VARIANTS_H_ diff --git a/util/mach/exc_server_variants_test.cc b/util/mach/exc_server_variants_test.cc index c43b5621..8ca041eb 100644 --- a/util/mach/exc_server_variants_test.cc +++ b/util/mach/exc_server_variants_test.cc @@ -556,17 +556,16 @@ TEST(ExcServerVariants, MockExceptionRaise) { const exception_behavior_t kExceptionBehavior = EXCEPTION_DEFAULT; EXPECT_CALL(server, - MockCatchMachException( - kExceptionBehavior, - kServerLocalPort, - kExceptionThreadPort, - kExceptionTaskPort, - kExceptionType, - AreExceptionCodes( - kTestExceptonCodes[0], kTestExceptonCodes[1]), - Pointee(Eq(THREAD_STATE_NONE)), - IsThreadStateCount(0u), - IsThreadStateCount(0u))) + MockCatchMachException(kExceptionBehavior, + kServerLocalPort, + kExceptionThreadPort, + kExceptionTaskPort, + kExceptionType, + AreExceptionCodes(kTestExceptonCodes[0], + kTestExceptonCodes[1]), + Pointee(Eq(THREAD_STATE_NONE)), + IsThreadStateCount(0u), + IsThreadStateCount(0u))) .WillOnce(Return(KERN_SUCCESS)) .RetiresOnSaturation(); @@ -595,18 +594,18 @@ TEST(ExcServerVariants, MockExceptionRaiseState) { const exception_behavior_t kExceptionBehavior = EXCEPTION_STATE; - EXPECT_CALL(server, - MockCatchMachException( - kExceptionBehavior, - kServerLocalPort, - MACH_PORT_NULL, - MACH_PORT_NULL, - kExceptionType, - AreExceptionCodes( - kTestExceptonCodes[0], kTestExceptonCodes[1]), - Pointee(Eq(kThreadStateFlavor)), - IsThreadStateCount(kThreadStateFlavorCount), - IsThreadStateCount(arraysize(reply.new_state)))) + EXPECT_CALL( + server, + MockCatchMachException( + kExceptionBehavior, + kServerLocalPort, + MACH_PORT_NULL, + MACH_PORT_NULL, + kExceptionType, + AreExceptionCodes(kTestExceptonCodes[0], kTestExceptonCodes[1]), + Pointee(Eq(kThreadStateFlavor)), + IsThreadStateCount(kThreadStateFlavorCount), + IsThreadStateCount(arraysize(reply.new_state)))) .WillOnce(Return(KERN_SUCCESS)) .RetiresOnSaturation(); @@ -638,18 +637,18 @@ TEST(ExcServerVariants, MockExceptionRaiseStateIdentity) { const exception_behavior_t kExceptionBehavior = EXCEPTION_STATE_IDENTITY; - EXPECT_CALL(server, - MockCatchMachException( - kExceptionBehavior, - kServerLocalPort, - kExceptionThreadPort, - kExceptionTaskPort, - kExceptionType, - AreExceptionCodes( - kTestExceptonCodes[0], kTestExceptonCodes[1]), - Pointee(Eq(kThreadStateFlavor)), - IsThreadStateCount(kThreadStateFlavorCount), - IsThreadStateCount(arraysize(reply.new_state)))) + EXPECT_CALL( + server, + MockCatchMachException( + kExceptionBehavior, + kServerLocalPort, + kExceptionThreadPort, + kExceptionTaskPort, + kExceptionType, + AreExceptionCodes(kTestExceptonCodes[0], kTestExceptonCodes[1]), + Pointee(Eq(kThreadStateFlavor)), + IsThreadStateCount(kThreadStateFlavorCount), + IsThreadStateCount(arraysize(reply.new_state)))) .WillOnce(Return(KERN_SUCCESS)) .RetiresOnSaturation(); @@ -681,17 +680,16 @@ TEST(ExcServerVariants, MockMachExceptionRaise) { EXPECT_CALL( server, - MockCatchMachException( - kExceptionBehavior, - kServerLocalPort, - kExceptionThreadPort, - kExceptionTaskPort, - kExceptionType, - AreExceptionCodes( - kTestMachExceptionCodes[0], kTestMachExceptionCodes[1]), - Pointee(Eq(THREAD_STATE_NONE)), - IsThreadStateCount(0u), - IsThreadStateCount(0u))) + MockCatchMachException(kExceptionBehavior, + kServerLocalPort, + kExceptionThreadPort, + kExceptionTaskPort, + kExceptionType, + AreExceptionCodes(kTestMachExceptionCodes[0], + kTestMachExceptionCodes[1]), + Pointee(Eq(THREAD_STATE_NONE)), + IsThreadStateCount(0u), + IsThreadStateCount(0u))) .WillOnce(Return(KERN_SUCCESS)) .RetiresOnSaturation(); @@ -723,17 +721,16 @@ TEST(ExcServerVariants, MockMachExceptionRaiseState) { EXPECT_CALL( server, - MockCatchMachException( - kExceptionBehavior, - kServerLocalPort, - MACH_PORT_NULL, - MACH_PORT_NULL, - kExceptionType, - AreExceptionCodes( - kTestMachExceptionCodes[0], kTestMachExceptionCodes[1]), - Pointee(Eq(kThreadStateFlavor)), - IsThreadStateCount(kThreadStateFlavorCount), - IsThreadStateCount(arraysize(reply.new_state)))) + MockCatchMachException(kExceptionBehavior, + kServerLocalPort, + MACH_PORT_NULL, + MACH_PORT_NULL, + kExceptionType, + AreExceptionCodes(kTestMachExceptionCodes[0], + kTestMachExceptionCodes[1]), + Pointee(Eq(kThreadStateFlavor)), + IsThreadStateCount(kThreadStateFlavorCount), + IsThreadStateCount(arraysize(reply.new_state)))) .WillOnce(Return(KERN_SUCCESS)) .RetiresOnSaturation(); @@ -768,17 +765,16 @@ TEST(ExcServerVariants, MockMachExceptionRaiseStateIdentity) { EXPECT_CALL( server, - MockCatchMachException( - kExceptionBehavior, - kServerLocalPort, - kExceptionThreadPort, - kExceptionTaskPort, - kExceptionType, - AreExceptionCodes( - kTestMachExceptionCodes[0], kTestMachExceptionCodes[1]), - Pointee(Eq(kThreadStateFlavor)), - IsThreadStateCount(kThreadStateFlavorCount), - IsThreadStateCount(arraysize(reply.new_state)))) + MockCatchMachException(kExceptionBehavior, + kServerLocalPort, + kExceptionThreadPort, + kExceptionTaskPort, + kExceptionType, + AreExceptionCodes(kTestMachExceptionCodes[0], + kTestMachExceptionCodes[1]), + Pointee(Eq(kThreadStateFlavor)), + IsThreadStateCount(kThreadStateFlavorCount), + IsThreadStateCount(arraysize(reply.new_state)))) .WillOnce(Return(KERN_SUCCESS)) .RetiresOnSaturation(); @@ -870,8 +866,7 @@ class TestExcServerVariants : public UniversalMachExcServer, behavior_(behavior), flavor_(flavor), state_count_(state_count), - handled_(false) { - } + handled_(false) {} // UniversalMachExcServer: @@ -936,14 +931,7 @@ class TestExcServerVariants : public UniversalMachExcServer, EXPECT_EQ(NULL, new_state); } - // Even for an EXC_CRASH handler, returning KERN_SUCCESS with a - // state-carrying reply will cause the kernel to try to set a new thread - // state, leading to a perceptible waste of time. Returning - // MACH_RCV_PORT_DIED is the only way to suppress this behavior while also - // preventing the kernel from looking for another (host-level) EXC_CRASH - // handler. See 10.9.4 xnu-2422.110.17/osfmk/kern/exception.c - // exception_triage(). - return has_state ? MACH_RCV_PORT_DIED : KERN_SUCCESS; + return ExcServerSuccessfulReturnValue(behavior, false); } private: @@ -1057,18 +1045,18 @@ TEST(ExcServerVariants, ThreadStates) { // Additionaly, the AVX state flavors are also not tested because they’re // not available on all CPUs and OS versions. #if defined(ARCH_CPU_X86) - { x86_THREAD_STATE32, x86_THREAD_STATE32_COUNT }, - { x86_FLOAT_STATE32, x86_FLOAT_STATE32_COUNT }, - { x86_EXCEPTION_STATE32, x86_EXCEPTION_STATE32_COUNT }, + {x86_THREAD_STATE32, x86_THREAD_STATE32_COUNT}, + {x86_FLOAT_STATE32, x86_FLOAT_STATE32_COUNT}, + {x86_EXCEPTION_STATE32, x86_EXCEPTION_STATE32_COUNT}, #endif #if defined(ARCH_CPU_X86_64) - { x86_THREAD_STATE64, x86_THREAD_STATE64_COUNT }, - { x86_FLOAT_STATE64, x86_FLOAT_STATE64_COUNT }, - { x86_EXCEPTION_STATE64, x86_EXCEPTION_STATE64_COUNT }, + {x86_THREAD_STATE64, x86_THREAD_STATE64_COUNT}, + {x86_FLOAT_STATE64, x86_FLOAT_STATE64_COUNT}, + {x86_EXCEPTION_STATE64, x86_EXCEPTION_STATE64_COUNT}, #endif - { x86_THREAD_STATE, x86_THREAD_STATE_COUNT }, - { x86_FLOAT_STATE, x86_FLOAT_STATE_COUNT }, - { x86_EXCEPTION_STATE, x86_EXCEPTION_STATE_COUNT }, + {x86_THREAD_STATE, x86_THREAD_STATE_COUNT}, + {x86_FLOAT_STATE, x86_FLOAT_STATE_COUNT}, + {x86_EXCEPTION_STATE, x86_EXCEPTION_STATE_COUNT}, #else #error Port this test to your CPU architecture. #endif @@ -1076,8 +1064,8 @@ TEST(ExcServerVariants, ThreadStates) { for (size_t index = 0; index < arraysize(test_data); ++index) { const TestData& test = test_data[index]; - SCOPED_TRACE(base::StringPrintf( - "index %zu, flavor %d", index, test.flavor)); + SCOPED_TRACE( + base::StringPrintf("index %zu, flavor %d", index, test.flavor)); TestExcServerVariants test_exc_server_variants( MACH_EXCEPTION_CODES | EXCEPTION_STATE_IDENTITY, @@ -1087,4 +1075,41 @@ TEST(ExcServerVariants, ThreadStates) { } } +TEST(ExcServerVariants, ExcServerSuccessfulReturnValue) { + struct TestData { + exception_behavior_t behavior; + bool set_thread_state; + kern_return_t kr; + }; + const TestData kTestData[] = { + {EXCEPTION_DEFAULT, false, KERN_SUCCESS}, + {EXCEPTION_STATE, false, MACH_RCV_PORT_DIED}, + {EXCEPTION_STATE_IDENTITY, false, MACH_RCV_PORT_DIED}, + {kMachExceptionCodes | EXCEPTION_DEFAULT, false, KERN_SUCCESS}, + {kMachExceptionCodes | EXCEPTION_STATE, false, MACH_RCV_PORT_DIED}, + {kMachExceptionCodes | EXCEPTION_STATE_IDENTITY, + false, + MACH_RCV_PORT_DIED}, + {EXCEPTION_DEFAULT, true, KERN_SUCCESS}, + {EXCEPTION_STATE, true, KERN_SUCCESS}, + {EXCEPTION_STATE_IDENTITY, true, KERN_SUCCESS}, + {kMachExceptionCodes | EXCEPTION_DEFAULT, true, KERN_SUCCESS}, + {kMachExceptionCodes | EXCEPTION_STATE, true, KERN_SUCCESS}, + {kMachExceptionCodes | EXCEPTION_STATE_IDENTITY, true, KERN_SUCCESS}, + }; + + for (size_t index = 0; index < arraysize(kTestData); ++index) { + const TestData& test_data = kTestData[index]; + SCOPED_TRACE( + base::StringPrintf("index %zu, behavior %d, set_thread_state %s", + index, + test_data.behavior, + test_data.set_thread_state ? "true" : "false")); + + EXPECT_EQ(test_data.kr, + ExcServerSuccessfulReturnValue(test_data.behavior, + test_data.set_thread_state)); + } +} + } // namespace