Skip to content

Commit

Permalink
Fix stack overflow handling with SuperPMI
Browse files Browse the repository at this point in the history
When SuperPMI shared library is loaded last, its SIGSEGV handler
is the first one that's executed. But since there is no coreclr runtime handler
installed for it, it returns from the SwitchStackAndExecuteHandler in case
of SIGSEGV. The remaining code in the SIGSEGV handler was not expecting that and
thought that there was no stack overflow and attempted to run the hardware exception
handler on the original stack of the thread, which obviously crashed since the original
stack overflowed.
The fix is to make sure that we only call the previously registered signal handler in
this case.

Close dotnet#84911
  • Loading branch information
janvorli committed Aug 15, 2024
1 parent a842025 commit c9b8b43
Showing 1 changed file with 24 additions and 17 deletions.
41 changes: 24 additions & 17 deletions src/coreclr/pal/src/exception/signal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -621,7 +621,8 @@ static void sigsegv_handler(int code, siginfo_t *siginfo, void *context)

// If the failure address is at most one page above or below the stack pointer,
// we have a stack overflow.
if ((failureAddress - (sp - GetVirtualPageSize())) < 2 * GetVirtualPageSize())
bool isStackOverflow = (failureAddress - (sp - GetVirtualPageSize())) < 2 * GetVirtualPageSize();
if (isStackOverflow)
{
if (GetCurrentPalThread())
{
Expand All @@ -641,6 +642,9 @@ static void sigsegv_handler(int code, siginfo_t *siginfo, void *context)
{
PROCAbort(SIGSEGV, siginfo);
}

// The current executable (shared library) doesn't have hardware exception handler installed or opted to not to
// handle it. So this handler will invoke the previously installed handler at the end of this function.
}
else
{
Expand All @@ -649,26 +653,29 @@ static void sigsegv_handler(int code, siginfo_t *siginfo, void *context)
}
}

// Now that we know the SIGSEGV didn't happen due to a stack overflow, execute the common
// hardware signal handler on the original stack.

if (GetCurrentPalThread() && IsRunningOnAlternateStack(context))
if (!isStackOverflow)
{
if (SwitchStackAndExecuteHandler(code, siginfo, context, 0 /* sp */)) // sp == 0 indicates execution on the original stack
// Now that we know the SIGSEGV didn't happen due to a stack overflow, execute the common
// hardware signal handler on the original stack.

if (GetCurrentPalThread() && IsRunningOnAlternateStack(context))
{
return;
if (SwitchStackAndExecuteHandler(code, siginfo, context, 0 /* sp */)) // sp == 0 indicates execution on the original stack
{
return;
}
}
}
else
{
// The code flow gets here when the signal handler is not running on an alternate stack or when it wasn't created
// by coreclr. In both cases, we execute the common_signal_handler directly.
// If thread isn't created by coreclr and has alternate signal stack GetCurrentPalThread() will return NULL too.
// But since in this case we don't handle hardware exceptions (IsSafeToHandleHardwareException returns false)
// we can call common_signal_handler on the alternate stack.
if (common_signal_handler(code, siginfo, context, 2, (size_t)0, (size_t)siginfo->si_addr))
else
{
return;
// The code flow gets here when the signal handler is not running on an alternate stack or when it wasn't created
// by coreclr. In both cases, we execute the common_signal_handler directly.
// If thread isn't created by coreclr and has alternate signal stack GetCurrentPalThread() will return NULL too.
// But since in this case we don't handle hardware exceptions (IsSafeToHandleHardwareException returns false)
// we can call common_signal_handler on the alternate stack.
if (common_signal_handler(code, siginfo, context, 2, (size_t)0, (size_t)siginfo->si_addr))
{
return;
}
}
}
}
Expand Down

0 comments on commit c9b8b43

Please sign in to comment.