Check for SIG_IGN and SIG_DFL before calling previous signal handler - #132900
Check for SIG_IGN and SIG_DFL before calling previous signal handler#132900jtschuster wants to merge 1 commit into
Conversation
macOS doesn't clear sa_flags when calling execve. When dotnet is started from a process that sets a signal handler, the handler is cleared, but not the sa_flags. When the runtime gets a signal from an external source, it sees a stale SA_SIGINGO and tries to call the previous signal handler, but the pointer is set to SIG_IGN or SIG_DFL, which causes a crash. This change checks for those values before calling the previous signal handler. Add a regression test for the issue.
|
Azure Pipelines: Successfully started running 4 pipeline(s). 12 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
Tagging subscribers to this area: @agocke |
There was a problem hiding this comment.
Pull request overview
This PR hardens CoreCLR and NativeAOT activation-signal chaining by ensuring the runtime does not attempt to call a previous signal “handler” when the saved disposition is actually SIG_DFL or SIG_IGN, even if SA_SIGINFO is set (a state observed on macOS across execve). It also adds a macOS-only regression test that reproduces the inherited SA_SIGINFO + default/ignored disposition scenario and validates the runtime remains stable when an external SIGUSR1 arrives.
Changes:
- CoreCLR: gate chaining to the saved activation signal disposition behind
IsSigDfl/IsSigIgnchecks before calling eithersa_sigactionorsa_handler. - NativeAOT: apply the same
SIG_DFL/SIG_IGNguard to activation handler chaining whenSA_SIGINFOis set. - Tests: add a macOS-only regression test that installs an
SA_SIGINFOhandler,execvs into the .NET process, then triggers an externalSIGUSR1to exercise the chaining path.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/coreclr/pal/src/exception/signal.cpp | Prevents calling the previous activation handler when it’s SIG_DFL/SIG_IGN, regardless of stale SA_SIGINFO. |
| src/coreclr/nativeaot/Runtime/unix/PalUnix.cpp | Mirrors the same SIG_DFL/SIG_IGN guard for NativeAOT’s activation handler chaining. |
| src/tests/Regressions/coreclr/GitHub_132581/test132581.csproj | Adds a macOS-only, process-isolated regression test project with a native CMake dependency. |
| src/tests/Regressions/coreclr/GitHub_132581/test132581.cs | Managed test that execvs after installing the signal handler and then triggers an external SIGUSR1. |
| src/tests/Regressions/coreclr/GitHub_132581/nativetest132581.cpp | Native helper to install the SA_SIGINFO handler + execv, and to send SIGUSR1 from a forked child. |
| src/tests/Regressions/coreclr/GitHub_132581/CMakeLists.txt | Builds/installs the native helper library on macOS only. |
| @@ -0,0 +1,17 @@ | |||
| <Project Sdk="Microsoft.NET.Sdk"> | |||
| <PropertyGroup> | |||
| <!-- Needed for CLRTestTargetUnsupported and CMakeProjectReference --> | |||
There was a problem hiding this comment.
| <!-- Needed for CLRTestTargetUnsupported and CMakeProjectReference --> | |
| <!-- Installs a custom signal handler. --> |
This test actually has a reason outside of infra limitations for why it is marked RPI. Let's put that here so later tooling doesn't try to force this in-proc with other tests.
There was a problem hiding this comment.
I find the regression tests by issue number a lot harder to understand if I need to go back to them. Thoughts on naming by the scenario - something like ActivationSignalChaining class and Exec_InheritedFlags method?
| <!-- Needed for CLRTestTargetUnsupported and CMakeProjectReference --> | ||
| <RequiresProcessIsolation>true</RequiresProcessIsolation> | ||
| <CLRTestTargetUnsupported Condition="'$(TargetsOSX)' != 'true'">true</CLRTestTargetUnsupported> | ||
| <CLRTestPriority>1</CLRTestPriority> |
There was a problem hiding this comment.
Since this is pri 1, do we want to kick of an explicit outerloop run (or whatever will run this in coreclr and native AOT)?
macOS doesn't clear sa_flags when calling execve. When dotnet is started from a process that sets a signal handler, the handler is cleared, but not the sa_flags. When the runtime gets a signal from an external source, it sees a stale SA_SIGINGO and tries to call the previous signal handler, but the pointer is set to SIG_IGN or SIG_DFL, which causes a crash. This change checks for those values before calling the previous signal handler. Also adds a regression test for the issue.
Fixes #132581