Check for SIG_IGN and SIG_DFL before calling previous signal handler

Project: dotnet/runtime | Language: C++/C#/CMake | Concepts: error-handling, testing-strategy, memory-management | Difficulty: senior
Link: https://github.com/dotnet/runtime/pull/132900

The problem

On macOS, when a process that has set a custom signal handler calls execve, the handler is cleared but the sa_flags field is not reset. This can leave the new process with a mismatched state: the sa_flags (such as SA_SIGINFO) indicate a handler is set, but the actual handler function pointer is set to SIG_IGN (ignore) or SIG_DFL (default). If the .NET runtime then tries to call this "previous" handler, it may dereference SIG_IGN or SIG_DFL as a function pointer, causing a crash.

Why it's non-trivial

A naive fix might only check if the handler pointer is non-null, or only look at the sa_flags to decide how to dispatch. However, on macOS, the combination of stale sa_flags with cleared handler pointers is possible and subtle. The code must check both the flags and the actual handler values, and must recognize SIG_IGN and SIG_DFL as special, non-callable values, even if sa_flags suggests a handler is present.

The approach

1. Utility functions to check handler state

The PR introduces helpers to check if a sigaction struct represents an ignored or default handler, and whether SA_SIGINFO is set:

+static bool IsSaSigInfo(struct sigaction* action)
+{
+    return (action->sa_flags & SA_SIGINFO) != 0;
+}

Checks if the SA_SIGINFO flag is set, which determines whether sa_sigaction or sa_handler should be used.

+static bool IsSigDfl(struct sigaction* action)
+{
+    // macOS can return sigaction with SIG_DFL and SA_SIGINFO.
+    // SA_SIGINFO means we should use sa_sigaction, but here we want to check sa_handler.
+    // So we ignore SA_SIGINFO when sa_sigaction and sa_handler are at the same address.
+    return (&action->sa_handler == (void*)&action->sa_sigaction || !IsSaSigInfo(action)) &&
+            action->sa_handler == SIG_DFL;
+}

Determines if the action represents the default signal handler, even in the presence of confusing SA_SIGINFO flags.

+static bool IsSigIgn(struct sigaction* action)
+{
+    return (&action->sa_handler == (void*)&action->sa_sigaction || !IsSaSigInfo(action)) &&
+            action->sa_handler == SIG_IGN;
+}

Analogous check for the ignore signal handler.

2. Only call the previous handler if it's not SIG_IGN or SIG_DFL

The signal handler logic is updated to use these helpers:

-    if (g_previousActivationHandler.sa_flags & SA_SIGINFO)
-    {
-        _ASSERTE(g_previousActivationHandler.sa_sigaction != NULL);
-        g_previousActivationHandler.sa_sigaction(code, siginfo, context);
-    }
-    else
-    {
-        if (g_previousActivationHandler.sa_handler != SIG_IGN &&
-            g_previousActivationHandler.sa_handler != SIG_DFL)
-        {
-            _ASSERTE(g_previousActivationHandler.sa_handler != NULL);
-            g_previousActivationHandler.sa_handler(code);
-        }
-    }
+    if (!IsSigDfl(&g_previousActivationHandler) && !IsSigIgn(&g_previousActivationHandler))
+    {
+        if (IsSaSigInfo(&g_previousActivationHandler))
+        {
+            _ASSERTE(g_previousActivationHandler.sa_sigaction != NULL);
+            g_previousActivationHandler.sa_sigaction(code, siginfo, context);
+        }
+        else
+        {
+            _ASSERTE(g_previousActivationHandler.sa_handler != NULL);
+            g_previousActivationHandler.sa_handler(code);
+        }
+    }

Now, before calling the previous handler, the code checks that it is neither SIG_IGN nor SIG_DFL, regardless of the state of sa_flags.

3. Regression test for the scenario

A new test is added under src/tests/Regressions/coreclr/GitHub_132581/:

• Native helper (nativetest132581.cpp) installs a signal handler, then execs the test process, simulating the macOS handler/flag leak.
• Managed test (test132581.cs) ensures that when the process receives a signal, the runtime does not try to call a cleared handler pointer, which would crash.

+    [Fact]
+    public static void DoNotCallPreviousSignalHandlerIfHandlerIsNotSet()
+    {
+        if (Environment.GetEnvironmentVariable(ChildEnvironmentVariable) is null)
+        {
+            string executable = Environment.ProcessPath ?? throw new InvalidOperationException("The process path is unavailable.");
+            string command = Environment.GetCommandLineArgs()[0];
+            string managedAssembly = string.Equals(executable, command, StringComparison.Ordinal) ? string.Empty : command;
+
+            int error = InstallSignalHandlerAndExec(executable, managedAssembly);
+            throw new InvalidOperationException($"execv failed with error {error}.");
+        }
+
+        Assert.Equal(0, SendSignalFromChildProcess());
+    }

This test verifies that after execve, the process can safely receive a signal without the runtime crashing due to misinterpreted handler pointers.

From the review thread

[elinor-fung]

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?

[jkoritzinsky]

    <!-- 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.

Takeaways

• Always check both handler pointers and flags when working with low-level OS signal APIs, especially across exec boundaries, as OSes may not reset all fields as expected.
• Recognize and handle special values like SIG_IGN and SIG_DFL explicitly, rather than treating all non-null pointers as valid handlers.
• Add regression tests that reproduce subtle platform-specific bugs, especially those involving process state inheritance, to prevent future regressions.