Skip to content

Commit

Permalink
Fix getting SSP for threadabort in funceval (dotnet#107320)
Browse files Browse the repository at this point in the history
* Fix getting SSP for threadabort in funceval

Diagnostics test that test threadabort during funceval crashes on
machines with CET enabled. The reason is that when a thread is
redirected to the RedirectForThrowControl, the original instruction
address from which we've redirected is not on the shadow stack.
That causes the exception handling propagating the related
thread abort to fail finding that instruction address on the shadow
stack. It walks out of its range and then crashes with access
violation.

This change fixes it by storing the SSP in the FaultingExceptionFrame
and copying it to the REGDISPLAY when UpdateRegDisplay is called on
that frame. That way the correct SSP is already known in the SfiInit and
we don't need to look it up.

* Clear rax before rdsspq for proper behavior on non-cet devices

---------

Co-authored-by: Jan Vorlicek <jan.vorlicek@volny,cz>
  • Loading branch information
2 people authored and radekdoulik committed Sep 6, 2024
1 parent 647f1fa commit 0e633a5
Show file tree
Hide file tree
Showing 6 changed files with 34 additions and 5 deletions.
4 changes: 4 additions & 0 deletions src/coreclr/vm/amd64/RedirectedHandledJITCase.asm
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,9 @@ NESTED_ENTRY STUB, _TEXT, FILTER
; info. After this push, unwinding will work.
push rcx

xor rax, rax
rdsspq rax

test rsp, 0fh
jnz STUB&_FixRsp

Expand All @@ -141,6 +144,7 @@ STUB&_RspAligned:

mov dword ptr [rcx], 0 ; Initialize vtbl (it is not strictly necessary)
mov dword ptr [rcx + OFFSETOF__FaultingExceptionFrame__m_fFilterExecuted], 0 ; Initialize BOOL for personality routine
mov qword ptr [rcx + OFFSETOF__FaultingExceptionFrame__m_SSP], rax

call TARGET

Expand Down
6 changes: 5 additions & 1 deletion src/coreclr/vm/amd64/asmconstants.h
Original file line number Diff line number Diff line change
Expand Up @@ -409,14 +409,18 @@ ASMCONSTANTS_C_ASSERT(OFFSETOF__CONTEXT__Xmm15
ASMCONSTANTS_C_ASSERT(OFFSETOF__CONTEXT__VectorRegister
== offsetof(CONTEXT, VectorRegister[0]));

#define SIZEOF__FaultingExceptionFrame (0x20 + SIZEOF__CONTEXT)
#define SIZEOF__FaultingExceptionFrame (0x20 + SIZEOF__CONTEXT + 16)
ASMCONSTANTS_C_ASSERT(SIZEOF__FaultingExceptionFrame
== sizeof(FaultingExceptionFrame));

#define OFFSETOF__FaultingExceptionFrame__m_fFilterExecuted 0x10
ASMCONSTANTS_C_ASSERT(OFFSETOF__FaultingExceptionFrame__m_fFilterExecuted
== offsetof(FaultingExceptionFrame, m_fFilterExecuted));

#define OFFSETOF__FaultingExceptionFrame__m_SSP (0x20 + SIZEOF__CONTEXT)
ASMCONSTANTS_C_ASSERT(OFFSETOF__FaultingExceptionFrame__m_SSP
== offsetof(FaultingExceptionFrame, m_SSP));

#define OFFSETOF__PtrArray__m_NumComponents 0x8
ASMCONSTANTS_C_ASSERT(OFFSETOF__PtrArray__m_NumComponents
== offsetof(PtrArray, m_NumComponents));
Expand Down
4 changes: 4 additions & 0 deletions src/coreclr/vm/amd64/cgenamd64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,10 @@ void FaultingExceptionFrame::UpdateRegDisplay(const PREGDISPLAY pRD, bool update

pRD->SP = m_ctx.Rsp;

#ifdef TARGET_WINDOWS
pRD->SSP = m_SSP;
#endif

pRD->pCurrentContextPointers->Rax = &m_ctx.Rax;
pRD->pCurrentContextPointers->Rcx = &m_ctx.Rcx;
pRD->pCurrentContextPointers->Rdx = &m_ctx.Rdx;
Expand Down
7 changes: 4 additions & 3 deletions src/coreclr/vm/excep.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5705,9 +5705,6 @@ void AdjustContextForThreadStop(Thread* pThread,

pThread->ResetThrowControlForThread();

// Should never get here if we're already throwing an exception.
_ASSERTE(!pThread->IsExceptionInProgress() || pThread->IsRudeAbort());

// Should never get here if we're already abort initiated.
_ASSERTE(!pThread->IsAbortInitiated() || pThread->IsRudeAbort());

Expand Down Expand Up @@ -6282,6 +6279,10 @@ void HandleManagedFaultNew(EXCEPTION_RECORD* pExceptionRecord, CONTEXT* pContext
#endif // FEATURE_EH_FUNCLETS
frame->InitAndLink(pContext);

#if defined(TARGET_AMD64) && defined(TARGET_WINDOWS)
frame->SetSSP(GetSSP(pContext));
#endif

Thread *pThread = GetThread();

ExInfo exInfo(pThread, pExceptionRecord, pContext, ExKind::HardwareFault);
Expand Down
7 changes: 6 additions & 1 deletion src/coreclr/vm/exceptionhandling.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8432,7 +8432,12 @@ extern "C" bool QCALLTYPE SfiInit(StackFrameIterator* pThis, CONTEXT* pStackwalk
// Get the SSP for the first managed frame. It is incremented during the stack walk so that
// when we reach the handling frame, it contains correct SSP to set when resuming after
// the catch handler.
pThis->m_crawl.GetRegisterSet()->SSP = GetSSPForFrameOnCurrentStack(controlPC);
// For hardware exceptions and thread abort exceptions propagated from ThrowControlForThread,
// the SSP is already known. For other cases, find it by scanning the shadow stack.
if ((pExInfo->m_passNumber == 2) && (pThis->m_crawl.GetRegisterSet()->SSP == 0))
{
pThis->m_crawl.GetRegisterSet()->SSP = GetSSPForFrameOnCurrentStack(controlPC);
}
#endif

if (!pThis->m_crawl.HasFaulted() && !pThis->m_crawl.IsIPadjusted())
Expand Down
11 changes: 11 additions & 0 deletions src/coreclr/vm/frames.h
Original file line number Diff line number Diff line change
Expand Up @@ -1064,6 +1064,10 @@ class FaultingExceptionFrame : public Frame
T_CONTEXT m_ctx;
#endif // !FEATURE_EH_FUNCLETS

#ifdef TARGET_AMD64
TADDR m_SSP;
#endif

VPTR_VTABLE_CLASS(FaultingExceptionFrame, Frame)

public:
Expand Down Expand Up @@ -1124,6 +1128,13 @@ class FaultingExceptionFrame : public Frame
}
#endif // FEATURE_EH_FUNCLETS

#ifdef TARGET_AMD64
void SetSSP(TADDR value)
{
m_SSP = value;
}
#endif

virtual BOOL NeedsUpdateRegDisplay()
{
return TRUE;
Expand Down

0 comments on commit 0e633a5

Please sign in to comment.