From fbceaeb1f57c4fc34668b7745317ef0bead89b6c Mon Sep 17 00:00:00 2001 From: David Stern Date: Tue, 14 Feb 2023 18:24:05 -0500 Subject: [PATCH 1/5] Proof-of-concept change that ignores exceptions from subprocesses. --- .../SentryCrashMonitor_MachException.c | 25 ++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/Sources/SentryCrash/Recording/Monitors/SentryCrashMonitor_MachException.c b/Sources/SentryCrash/Recording/Monitors/SentryCrashMonitor_MachException.c index 723f6d941a5..836e890d58c 100644 --- a/Sources/SentryCrash/Recording/Monitors/SentryCrashMonitor_MachException.c +++ b/Sources/SentryCrash/Recording/Monitors/SentryCrashMonitor_MachException.c @@ -313,7 +313,30 @@ handleExceptions(void *const userData) kern_return_t kr = mach_msg(&exceptionMessage.header, MACH_RCV_MSG, 0, sizeof(exceptionMessage), g_exceptionPort, MACH_MSG_TIMEOUT_NONE, MACH_PORT_NULL); if (kr == KERN_SUCCESS) { - break; + // If the exception was generated in a different process than the one in which + // the handler was installed, ignore it. Task-level exception handlers are + // inherited by subprocesses, and we don't want to handle or report exceptions + // in arbitrary subprocesses of the one in which we were installed. + if (exceptionMessage.task.name != mach_task_self()) { + SentryCrashLOG_DEBUG("Received exception from subprocess. Ignoring message and waiting for another."); + // Send a reply saying "I didn't handle this exception". + replyMessage.header.msgh_id = 0; + replyMessage.header.msgh_bits = exceptionMessage.header.msgh_bits & MACH_MSGH_BITS_REMOTE_MASK; + replyMessage.header.msgh_remote_port = exceptionMessage.header.msgh_remote_port; + replyMessage.header.msgh_local_port = MACH_PORT_NULL; + replyMessage.header.msgh_size = sizeof(mach_msg_header_t); + + replyMessage.NDR = exceptionMessage.NDR; + // Tell the kernel that we failed to handle the exception, so that + // the next handler in the chain gets invoked. + replyMessage.returnCode = KERN_FAILURE; + + mach_msg(&replyMessage.header, MACH_SEND_MSG, sizeof(replyMessage), 0, replyMessage.header.msgh_remote_port, + MACH_MSG_TIMEOUT_NONE, MACH_PORT_NULL); + continue; + } else { + break; + } } // Loop and try again on failure. From 59f585fc1b9171d7964ce2f671346dea1ab37e99 Mon Sep 17 00:00:00 2001 From: David Stern Date: Thu, 16 Feb 2023 11:23:43 -0500 Subject: [PATCH 2/5] Fix mach message size field in header. Co-authored-by: Philipp Hofmann --- .../Recording/Monitors/SentryCrashMonitor_MachException.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/SentryCrash/Recording/Monitors/SentryCrashMonitor_MachException.c b/Sources/SentryCrash/Recording/Monitors/SentryCrashMonitor_MachException.c index 836e890d58c..bf44bac7c00 100644 --- a/Sources/SentryCrash/Recording/Monitors/SentryCrashMonitor_MachException.c +++ b/Sources/SentryCrash/Recording/Monitors/SentryCrashMonitor_MachException.c @@ -324,7 +324,7 @@ handleExceptions(void *const userData) replyMessage.header.msgh_bits = exceptionMessage.header.msgh_bits & MACH_MSGH_BITS_REMOTE_MASK; replyMessage.header.msgh_remote_port = exceptionMessage.header.msgh_remote_port; replyMessage.header.msgh_local_port = MACH_PORT_NULL; - replyMessage.header.msgh_size = sizeof(mach_msg_header_t); + replyMessage.header.msgh_size = sizeof(MachReplyMessage); replyMessage.NDR = exceptionMessage.NDR; // Tell the kernel that we failed to handle the exception, so that From 5d36423d228eee25d119a07b2a8379855454736d Mon Sep 17 00:00:00 2001 From: David Stern Date: Thu, 16 Feb 2023 11:33:32 -0500 Subject: [PATCH 3/5] Revert rcv_name field in mach_msg to MACH_PORT_NULL. --- .../Recording/Monitors/SentryCrashMonitor_MachException.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/SentryCrash/Recording/Monitors/SentryCrashMonitor_MachException.c b/Sources/SentryCrash/Recording/Monitors/SentryCrashMonitor_MachException.c index bf44bac7c00..2a53bc728e3 100644 --- a/Sources/SentryCrash/Recording/Monitors/SentryCrashMonitor_MachException.c +++ b/Sources/SentryCrash/Recording/Monitors/SentryCrashMonitor_MachException.c @@ -331,7 +331,7 @@ handleExceptions(void *const userData) // the next handler in the chain gets invoked. replyMessage.returnCode = KERN_FAILURE; - mach_msg(&replyMessage.header, MACH_SEND_MSG, sizeof(replyMessage), 0, replyMessage.header.msgh_remote_port, + mach_msg(&replyMessage.header, MACH_SEND_MSG, sizeof(replyMessage), 0, MACH_PORT_NULL, MACH_MSG_TIMEOUT_NONE, MACH_PORT_NULL); continue; } else { From cb515120e0b1615ce50f6c2cfdcc296fc5f13957 Mon Sep 17 00:00:00 2001 From: David Stern Date: Tue, 21 Feb 2023 11:48:47 -0500 Subject: [PATCH 4/5] Adjust code structure to use guard expression. --- .../SentryCrashMonitor_MachException.c | 43 +++++++++---------- 1 file changed, 21 insertions(+), 22 deletions(-) diff --git a/Sources/SentryCrash/Recording/Monitors/SentryCrashMonitor_MachException.c b/Sources/SentryCrash/Recording/Monitors/SentryCrashMonitor_MachException.c index 2a53bc728e3..8bafe478259 100644 --- a/Sources/SentryCrash/Recording/Monitors/SentryCrashMonitor_MachException.c +++ b/Sources/SentryCrash/Recording/Monitors/SentryCrashMonitor_MachException.c @@ -313,30 +313,29 @@ handleExceptions(void *const userData) kern_return_t kr = mach_msg(&exceptionMessage.header, MACH_RCV_MSG, 0, sizeof(exceptionMessage), g_exceptionPort, MACH_MSG_TIMEOUT_NONE, MACH_PORT_NULL); if (kr == KERN_SUCCESS) { - // If the exception was generated in a different process than the one in which - // the handler was installed, ignore it. Task-level exception handlers are - // inherited by subprocesses, and we don't want to handle or report exceptions - // in arbitrary subprocesses of the one in which we were installed. - if (exceptionMessage.task.name != mach_task_self()) { - SentryCrashLOG_DEBUG("Received exception from subprocess. Ignoring message and waiting for another."); - // Send a reply saying "I didn't handle this exception". - replyMessage.header.msgh_id = 0; - replyMessage.header.msgh_bits = exceptionMessage.header.msgh_bits & MACH_MSGH_BITS_REMOTE_MASK; - replyMessage.header.msgh_remote_port = exceptionMessage.header.msgh_remote_port; - replyMessage.header.msgh_local_port = MACH_PORT_NULL; - replyMessage.header.msgh_size = sizeof(MachReplyMessage); - - replyMessage.NDR = exceptionMessage.NDR; - // Tell the kernel that we failed to handle the exception, so that - // the next handler in the chain gets invoked. - replyMessage.returnCode = KERN_FAILURE; - - mach_msg(&replyMessage.header, MACH_SEND_MSG, sizeof(replyMessage), 0, MACH_PORT_NULL, - MACH_MSG_TIMEOUT_NONE, MACH_PORT_NULL); - continue; - } else { + // Only handle exceptions generated in this process. Task-level exception + // handlers are inherited by subprocesses, and we don't want to handle or + // report exceptions in arbitrary subprocesses of the one in which we were + // installed. + if (exceptionMessage.task.name == mach_task_self()) { break; } + + SentryCrashLOG_DEBUG("Received exception from subprocess. Ignoring message and waiting for another."); + // Send a reply saying "I didn't handle this exception". + replyMessage.header.msgh_id = 0; + replyMessage.header.msgh_bits = exceptionMessage.header.msgh_bits & MACH_MSGH_BITS_REMOTE_MASK; + replyMessage.header.msgh_remote_port = exceptionMessage.header.msgh_remote_port; + replyMessage.header.msgh_local_port = MACH_PORT_NULL; + replyMessage.header.msgh_size = sizeof(MachReplyMessage); + + replyMessage.NDR = exceptionMessage.NDR; + // Tell the kernel that we failed to handle the exception, so that + // the next handler in the chain gets invoked. + replyMessage.returnCode = KERN_FAILURE; + + mach_msg(&replyMessage.header, MACH_SEND_MSG, sizeof(replyMessage), 0, MACH_PORT_NULL, + MACH_MSG_TIMEOUT_NONE, MACH_PORT_NULL); } // Loop and try again on failure. From 6cd4f8829caa0c6de16080f663edbed63c1df174 Mon Sep 17 00:00:00 2001 From: David Stern Date: Thu, 23 Feb 2023 11:15:26 -0500 Subject: [PATCH 5/5] Reuse the incoming exception message ID for the reply message. --- .../Recording/Monitors/SentryCrashMonitor_MachException.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/SentryCrash/Recording/Monitors/SentryCrashMonitor_MachException.c b/Sources/SentryCrash/Recording/Monitors/SentryCrashMonitor_MachException.c index 8bafe478259..a82806ab9ed 100644 --- a/Sources/SentryCrash/Recording/Monitors/SentryCrashMonitor_MachException.c +++ b/Sources/SentryCrash/Recording/Monitors/SentryCrashMonitor_MachException.c @@ -323,7 +323,7 @@ handleExceptions(void *const userData) SentryCrashLOG_DEBUG("Received exception from subprocess. Ignoring message and waiting for another."); // Send a reply saying "I didn't handle this exception". - replyMessage.header.msgh_id = 0; + replyMessage.header.msgh_id = exceptionMessage.header.msgh_id; replyMessage.header.msgh_bits = exceptionMessage.header.msgh_bits & MACH_MSGH_BITS_REMOTE_MASK; replyMessage.header.msgh_remote_port = exceptionMessage.header.msgh_remote_port; replyMessage.header.msgh_local_port = MACH_PORT_NULL;