Skip to content

Commit

Permalink
Fix SSP issue with HW exceptions from JIT helpers (dotnet#107665)
Browse files Browse the repository at this point in the history
The recent change that was fixing bad SSP updating during exception
handling with funceval has also modified the way SSP is extracted for
hardware exceptions. That works fine for many cases, but there is a
problem when the exception occurs in a JIT helper. The
`AjustContextForJITHelpers` uses only the basic `CONTEXT` structure for
unwinding to the managed caller and so the SSP is not properly updated.
That makes it off by one slot when we set it when continuing execution
after catch.
This change removes storing SSP for hardware exceptions in the
FaultingExceptionFrame to mitigate the issue. It effectively returns to
the way software exceptions use - scanning shadow stack for the
instruction pointer of the frame to which it is going to resume after
catch.

Co-authored-by: Jan Vorlicek <jan.vorlicek@volny,cz>
  • Loading branch information
janvorli and Jan Vorlicek authored Sep 11, 2024
1 parent b8d7ec0 commit 6219d18
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 6 deletions.
6 changes: 4 additions & 2 deletions src/coreclr/vm/amd64/RedirectedHandledJITCase.asm
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +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
mov r8, rax

call TARGET

Expand Down Expand Up @@ -185,6 +185,7 @@ NESTED_ENTRY RedirectForThrowControl2, _TEXT

save_reg_postrsp rcx, REDIRECT_FOR_THROW_CONTROL_FRAME_SIZE + 8h ; FaultingExceptionFrame
save_reg_postrsp rdx, REDIRECT_FOR_THROW_CONTROL_FRAME_SIZE + 10h ; Original RSP
save_reg_postrsp r8, REDIRECT_FOR_THROW_CONTROL_FRAME_SIZE + 18h ; SSP

END_PROLOGUE

Expand All @@ -197,7 +198,8 @@ NESTED_ENTRY RedirectForThrowControl2, _TEXT
mov rdx, [rsp + REDIRECT_FOR_THROW_CONTROL_FRAME_SIZE + 10h] ; Original RSP
mov [rdx - 8], rax

mov rcx, [rsp + REDIRECT_FOR_THROW_CONTROL_FRAME_SIZE + 8h] ; FaultingExceptionFrame
mov rcx, [rsp + REDIRECT_FOR_THROW_CONTROL_FRAME_SIZE + 8h] ; FaultingExceptionFrame
mov rdx, [rsp + REDIRECT_FOR_THROW_CONTROL_FRAME_SIZE + 18h] ; SSP
call ThrowControlForThread

; ThrowControlForThread doesn't return.
Expand Down
9 changes: 5 additions & 4 deletions src/coreclr/vm/excep.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6279,10 +6279,6 @@ 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 Expand Up @@ -6364,6 +6360,11 @@ void FaultingExceptionFrame::Init(CONTEXT *pContext)
m_ReturnAddress = ::GetIP(pContext);
CopyOSContext(&m_ctx, pContext);
#endif // !FEATURE_EH_FUNCLETS

#if defined(TARGET_AMD64) && defined(TARGET_WINDOWS)
m_SSP = 0;
#endif

}

//
Expand Down
3 changes: 3 additions & 0 deletions src/coreclr/vm/threads.h
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,9 @@ EXTERN_C void ThrowControlForThread(
#ifdef FEATURE_EH_FUNCLETS
FaultingExceptionFrame *pfef
#endif // FEATURE_EH_FUNCLETS
#if defined(TARGET_AMD64) && defined(TARGET_WINDOWS)
, TADDR ssp
#endif // TARGET_AMD64 && TARGET_WINDOWS
);

#if defined(_DEBUG)
Expand Down
7 changes: 7 additions & 0 deletions src/coreclr/vm/threadsuspend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3735,6 +3735,9 @@ ThrowControlForThread(
#ifdef FEATURE_EH_FUNCLETS
FaultingExceptionFrame *pfef
#endif // FEATURE_EH_FUNCLETS
#if defined(TARGET_AMD64) && defined(TARGET_WINDOWS)
, TADDR ssp
#endif // TARGET_AMD64 && TARGET_WINDOWS
)
{
STATIC_CONTRACT_THROWS;
Expand Down Expand Up @@ -3784,6 +3787,10 @@ ThrowControlForThread(
#endif // FEATURE_EH_FUNCLETS
pfef->InitAndLink(pThread->m_OSContext);

#if defined(TARGET_AMD64) && defined(TARGET_WINDOWS)
pfef->SetSSP(ssp);
#endif

// !!! Can not assert here. Sometimes our EHInfo for catch clause extends beyond
// !!! Jit_EndCatch. Not sure if we have guarantee on catch clause.
//_ASSERTE (pThread->ReadyForAbort());
Expand Down

0 comments on commit 6219d18

Please sign in to comment.