Skip to content

fix: mask signal during CHAIN_AT_START to survive chained handler re-raises #1572

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jpnurmi wants to merge 6 commits into
base: master
Choose a base branch
from
+29 −0
Open

fix: mask signal during CHAIN_AT_START to survive chained handler re-raises #1572

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
91afd1a
fix: skip SA_NODEFER when CHAIN_AT_START is active
jpnurmi Mar 11, 2026
ab26763
Update CHANGELOG.md
jpnurmi Mar 11, 2026
66b86a6
Revert "fix: skip SA_NODEFER when CHAIN_AT_START is active"
jpnurmi Mar 11, 2026
b25f91d
fix: mask signal during CHAIN_AT_START to prevent re-raise from killi…
jpnurmi Mar 11, 2026
57a447a
Update CHANGELOG.md
jpnurmi Mar 11, 2026
986a153
fix: use raw syscall for sigtimedwait and correct sigset size
jpnurmi Mar 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# Changelog

## Unreleased

**Fixes**:

- Fix `CHAIN_AT_START` handler strategy crashing when the chained handler resets the signal handler and re-raises. ([#1572](https://github.com/getsentry/sentry-native/pull/1572))

## 0.13.2

**Features**:
Expand Down
23 changes: 23 additions & 0 deletions src/backends/sentry_backend_inproc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include <limits.h>
#ifdef SENTRY_PLATFORM_UNIX
# include <poll.h>
# include <sys/syscall.h>
#endif
#include <string.h>

Expand Down Expand Up @@ -1563,6 +1564,14 @@ process_ucontext(const sentry_ucontext_t *uctx)
uintptr_t ip = get_instruction_pointer(uctx);
uintptr_t sp = get_stack_pointer(uctx);

// Mask the signal so SA_NODEFER doesn't let re-raises from the chained
// handler to kill the process before we regain control.
sigset_t mask, old_mask;
sigemptyset(&mask);
sigaddset(&mask, uctx->signum);
// raw syscall to bypass libsigchain on Android
syscall(SYS_rt_sigprocmask, SIG_BLOCK, &mask, &old_mask, _NSIG / 8);
Copy link

@cursor cursor bot Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Raw syscall buffer overflow on 32-bit Android

High Severity

On 32-bit Android (LP32), bionic's sigset_t is only 4 bytes, but _NSIG / 8 evaluates to 8. The raw syscall(SYS_rt_sigprocmask, SIG_BLOCK, &mask, &old_mask, _NSIG / 8) tells the kernel to write 8 bytes into the 4-byte old_mask buffer, causing a stack buffer overflow. The project explicitly targets armeabi-v7a and x86 (32-bit ABIs). Bionic's libc wrapper normally handles this size mismatch via an internal kernel_sigset_t union, but the raw syscall bypasses that conversion.

Additional Locations (2)
Fix in Cursor Fix in Web


// invoke the previous handler (typically the CLR/Mono
// signal-to-managed-exception handler)
invoke_signal_handler(
Expand All @@ -1578,6 +1587,20 @@ process_ucontext(const sentry_ucontext_t *uctx)
return;
}

// restore our handler
struct sigaction current;
sigaction(uctx->signum, NULL, &current);
if (current.sa_handler == SIG_DFL) {
sigaction(uctx->signum, &g_sigaction, NULL);
}

// consume pending signal
struct timespec timeout = { 0, 0 };
syscall(SYS_rt_sigtimedwait, &mask, NULL, &timeout, _NSIG / 8);
Comment on lines +1598 to +1599
Copy link

@sentry sentry bot Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: The function returns early if the chained handler modifies ip or sp, but it does so without unmasking the signal or consuming the pending signal raised by the handler.
Severity: HIGH

Suggested Fix

Before the early return, add the necessary calls to consume the pending signal and restore the original signal mask. Specifically, call syscall(SYS_rt_sigtimedwait, ...) with a zero timeout to consume the signal, and then call syscall(SYS_rt_sigprocmask, SIG_SETMASK, &old_mask, ...) to unmask it.

Prompt for AI Agent 
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: src/backends/sentry_backend_inproc.c#L1598-L1599

Potential issue: In the signal handler, a signal is masked before invoking a chained
handler. If this chained handler re-raises the signal (e.g., via `raise(signum)`) and
also modifies the instruction pointer (`ip`) or stack pointer (`sp`), the function will
`return` early. This early return path skips the logic that consumes the now-pending
signal with `sigtimedwait` and unmasks it with `sigprocmask`. This leaves the process
with a pending signal that is still masked, preventing its delivery and potentially
causing undefined behavior or crashes later on.


// unmask
syscall(SYS_rt_sigprocmask, SIG_SETMASK, &old_mask, NULL, _NSIG / 8);

// return from runtime handler; continue processing the crash on the
// signal thread until the worker takes over
}
Expand Down
Loading