Skip to content

Commit 4f38f92

Browse files
janvorliJan Vorlicekjkotas
authored
Fix SSP restoring in edge cases (#104820)
* Fix SSP restoring in edge cases There are edge cases when the SSP restoring for continuation after a catch handler completes doesn't work correctly. The problem is caused by the fact that we scan for the Rip of the frame handling the exception on the shadow stack to find where to restore it, and in those edge cases, the same address can be there multiple times and the first occurence is not the right one. For example, when an exception is thrown from a catch handler, it escapes the handler and the handler for the escaped exception is in the same method as the one that invoked the handler. This change fixes it by finding the SSP of the first managed frame where we search for the handler and then updating the SSP with every unwind. The SSP is stored in the REGDISPLAY. So when we reach the CallCatchFunclet, the REGDISPLAY contains the SSP to restore. There was also one more issue with restoring the SSP. I turned out that the incsspq instruction uses only the lowest 8 bits of the argument to increment the SSP, so the ClrRestoreNonVolatileContextWorker needs to have a loop that repeats that instruction in case we need to move it by more than 255 slots. * Fix linux x64 build * Fix release build REGDISPLAY size constant * Add regression test * Update src/tests/Regressions/coreclr/GitHub_104820/test104820.cs --------- Co-authored-by: Jan Vorlicek <jan.vorlicek@volny,cz> Co-authored-by: Jan Kotas <jkotas@microsoft.com>
1 parent 5677d92 commit 4f38f92

File tree

9 files changed

+125
-16
lines changed

9 files changed

+125
-16
lines changed

src/coreclr/System.Private.CoreLib/src/System/Runtime/ExceptionServices/AsmOffsets.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ class AsmOffsets
7272
public const int OFFSETOF__REGDISPLAY__SP = 0x1a70;
7373
public const int OFFSETOF__REGDISPLAY__ControlPC = 0x1a78;
7474
#else // TARGET_UNIX
75-
public const int SIZEOF__REGDISPLAY = 0xbe0;
75+
public const int SIZEOF__REGDISPLAY = 0xbf0;
7676
public const int OFFSETOF__REGDISPLAY__SP = 0xbd0;
7777
public const int OFFSETOF__REGDISPLAY__ControlPC = 0xbd8;
7878
#endif // TARGET_UNIX

src/coreclr/inc/regdisp.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,10 @@ struct REGDISPLAY_BASE {
4545

4646
TADDR SP;
4747
TADDR ControlPC; // LOONGARCH: use RA for PC
48+
49+
#if defined(TARGET_AMD64) && defined(TARGET_WINDOWS)
50+
TADDR SSP;
51+
#endif
4852
};
4953

5054
inline PCODE GetControlPC(const REGDISPLAY_BASE *pRD) {
@@ -510,6 +514,12 @@ inline void FillRegDisplay(const PREGDISPLAY pRD, PT_CONTEXT pctx, PT_CONTEXT pC
510514
// This will setup the PC and SP
511515
SyncRegDisplayToCurrentContext(pRD);
512516

517+
#if !defined(DACCESS_COMPILE)
518+
#if defined(TARGET_AMD64) && defined(TARGET_WINDOWS)
519+
pRD->SSP = GetSSP(pctx);
520+
#endif
521+
#endif // !DACCESS_COMPILE
522+
513523
if (fLightUnwind)
514524
return;
515525

src/coreclr/vm/amd64/Context.S

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,14 @@ NESTED_ENTRY ClrRestoreNonvolatileContextWorker, _TEXT, NoHandler
4444
rdsspq rax
4545
sub r11, rax
4646
shr r11, 3
47-
incsspq r11
47+
// the incsspq instruction uses only the lowest 8 bits of the argument, so we need to loop in case the increment is larger than 255
48+
mov rax, 255
49+
Update_Loop:
50+
cmp r11, rax
51+
cmovb rax, r11
52+
incsspq rax
53+
sub r11, rax
54+
ja Update_Loop
4855
No_Ssp_Update:
4956

5057
// When user-mode shadow stacks are enabled, and for example the intent is to continue execution in managed code after

src/coreclr/vm/amd64/Context.asm

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,14 @@ NESTED_ENTRY ClrRestoreNonvolatileContextWorker, _TEXT
5353
rdsspq rax
5454
sub r11, rax
5555
shr r11, 3
56-
incsspq r11
56+
; the incsspq instruction uses only the lowest 8 bits of the argument, so we need to loop in case the increment is larger than 255
57+
mov rax, 255
58+
Update_Loop:
59+
cmp r11, rax
60+
cmovb rax, r11
61+
incsspq rax
62+
sub r11, rax
63+
ja Update_Loop
5764
No_Ssp_Update:
5865

5966
; When user-mode shadow stacks are enabled, and for example the intent is to continue execution in managed code after

src/coreclr/vm/amd64/cgenamd64.cpp

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,13 +108,20 @@ void InlinedCallFrame::UpdateRegDisplay(const PREGDISPLAY pRD, bool updateFloats
108108
if (updateFloats)
109109
{
110110
UpdateFloatingPointRegisters(pRD);
111+
// The float updating unwinds the stack so the pRD->pCurrentContext->Rip contains correct unwound Rip
112+
// This is used for exception handling and the Rip extracted from m_pCallerReturnAddress is slightly
113+
// off, which causes problem with searching for the return address on shadow stack on x64, so
114+
// we keep the value from the unwind.
111115
}
116+
else
112117
#endif // DACCESS_COMPILE
118+
{
119+
pRD->pCurrentContext->Rip = *(DWORD64 *)&m_pCallerReturnAddress;
120+
}
113121

114122
pRD->IsCallerContextValid = FALSE;
115123
pRD->IsCallerSPValid = FALSE; // Don't add usage of this field. This is only temporary.
116124

117-
pRD->pCurrentContext->Rip = *(DWORD64 *)&m_pCallerReturnAddress;
118125
pRD->pCurrentContext->Rsp = *(DWORD64 *)&m_pCallSiteSP;
119126
pRD->pCurrentContext->Rbp = *(DWORD64 *)&m_pCalleeSavedFP;
120127

src/coreclr/vm/exceptionhandling.cpp

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7673,27 +7673,23 @@ UINT_PTR GetEstablisherFrame(REGDISPLAY* pvRegDisplay, ExInfo* exInfo)
76737673
#endif
76747674
}
76757675

7676-
size_t GetSSPForFrameOnCurrentStack(CONTEXT *pContext)
7677-
{
7678-
size_t *targetSSP = 0;
7679-
76807676
#if defined(HOST_AMD64) && defined(HOST_WINDOWS)
7681-
targetSSP = (size_t *)_rdsspq();
7682-
// Find the shadow stack pointer for the frame we are going to restore to.
7677+
size_t GetSSPForFrameOnCurrentStack(TADDR ip)
7678+
{
7679+
size_t *targetSSP = (size_t *)_rdsspq();
76837680
// The SSP we search is pointing to the return address of the frame represented
7684-
// by the passed in context. So we search for the instruction pointer from
7681+
// by the passed in IP. So we search for the instruction pointer from
76857682
// the context and return one slot up from there.
76867683
if (targetSSP != NULL)
76877684
{
7688-
size_t ip = GetIP(pContext);
76897685
while (*targetSSP++ != ip)
76907686
{
76917687
}
76927688
}
7693-
#endif // HOST_AMD64 && HOST_WINDOWS
76947689

76957690
return (size_t)targetSSP;
76967691
}
7692+
#endif // HOST_AMD64 && HOST_WINDOWS
76977693

76987694
extern "C" void * QCALLTYPE CallCatchFunclet(QCall::ObjectHandleOnStack exceptionObj, BYTE* pHandlerIP, REGDISPLAY* pvRegDisplay, ExInfo* exInfo)
76997695
{
@@ -7711,7 +7707,12 @@ extern "C" void * QCALLTYPE CallCatchFunclet(QCall::ObjectHandleOnStack exceptio
77117707
exInfo->m_ScannedStackRange.ExtendUpperBound(exInfo->m_frameIter.m_crawl.GetRegisterSet()->SP);
77127708
DWORD_PTR dwResumePC = 0;
77137709
UINT_PTR callerTargetSp = 0;
7714-
size_t targetSSP = GetSSPForFrameOnCurrentStack(pvRegDisplay->pCurrentContext);
7710+
#if defined(HOST_AMD64) && defined(HOST_WINDOWS)
7711+
size_t targetSSP = exInfo->m_frameIter.m_crawl.GetRegisterSet()->SSP;
7712+
_ASSERTE(targetSSP == 0 || (*(size_t*)(targetSSP-8) == exInfo->m_frameIter.m_crawl.GetRegisterSet()->ControlPC));
7713+
#else
7714+
size_t targetSSP = 0;
7715+
#endif
77157716

77167717
if (pHandlerIP != NULL)
77177718
{
@@ -7929,6 +7930,12 @@ extern "C" void QCALLTYPE ResumeAtInterceptionLocation(REGDISPLAY* pvRegDisplay)
79297930

79307931
pThread->GetExceptionState()->GetDebuggerState()->GetDebuggerInterceptInfo(&pInterceptMD, NULL, (PBYTE*)&(sfInterceptStackFrame.SP), &ulRelOffset, NULL);
79317932

7933+
#if defined(HOST_AMD64) && defined(HOST_WINDOWS)
7934+
TADDR targetSSP = pExInfo->m_frameIter.m_crawl.GetRegisterSet()->SSP;
7935+
#else
7936+
TADDR targetSSP = 0;
7937+
#endif
7938+
79327939
ExInfo::PopExInfos(pThread, (void*)targetSp);
79337940

79347941
PCODE pStartAddress = pInterceptMD->GetNativeCode();
@@ -7941,8 +7948,6 @@ extern "C" void QCALLTYPE ResumeAtInterceptionLocation(REGDISPLAY* pvRegDisplay)
79417948
_ASSERTE(FitsIn<DWORD>(ulRelOffset));
79427949
uResumePC = codeInfo.GetJitManager()->GetCodeAddressForRelOffset(codeInfo.GetMethodToken(), static_cast<DWORD>(ulRelOffset));
79437950

7944-
size_t targetSSP = GetSSPForFrameOnCurrentStack(pvRegDisplay->pCurrentContext);
7945-
79467951
SetIP(pvRegDisplay->pCurrentContext, uResumePC);
79477952
ClrRestoreNonvolatileContext(pvRegDisplay->pCurrentContext, targetSSP);
79487953
}
@@ -8416,6 +8421,14 @@ extern "C" bool QCALLTYPE SfiInit(StackFrameIterator* pThis, CONTEXT* pStackwalk
84168421
if (result)
84178422
{
84188423
TADDR controlPC = pThis->m_crawl.GetRegisterSet()->ControlPC;
8424+
8425+
#if defined(HOST_AMD64) && defined(HOST_WINDOWS)
8426+
// Get the SSP for the first managed frame. It is incremented during the stack walk so that
8427+
// when we reach the handling frame, it contains correct SSP to set when resuming after
8428+
// the catch handler.
8429+
pThis->m_crawl.GetRegisterSet()->SSP = GetSSPForFrameOnCurrentStack(controlPC);
8430+
#endif
8431+
84198432
if (!pThis->m_crawl.HasFaulted() && !pThis->m_crawl.IsIPadjusted())
84208433
{
84218434
controlPC -= STACKWALK_CONTROLPC_ADJUST_OFFSET;

src/coreclr/vm/stackwalk.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -524,6 +524,12 @@ UINT_PTR Thread::VirtualUnwindCallFrame(PREGDISPLAY pRD, EECodeInfo* pCodeInfo /
524524
VirtualUnwindCallFrame(pRD->pCurrentContext, pRD->pCurrentContextPointers, pCodeInfo);
525525
}
526526

527+
#if defined(TARGET_AMD64) && defined(TARGET_WINDOWS)
528+
if (pRD->SSP != 0)
529+
{
530+
pRD->SSP += 8;
531+
}
532+
#endif // TARGET_AMD64 && TARGET_WINDOWS
527533
SyncRegDisplayToCurrentContext(pRD);
528534
pRD->IsCallerContextValid = FALSE;
529535
pRD->IsCallerSPValid = FALSE; // Don't add usage of this field. This is only temporary.
@@ -1555,6 +1561,9 @@ void StackFrameIterator::SkipTo(StackFrameIterator *pOtherStackFrameIterator)
15551561
*pRD->pCurrentContextPointers = *pOtherRD->pCurrentContextPointers;
15561562
SetIP(pRD->pCurrentContext, GetIP(pOtherRD->pCurrentContext));
15571563
SetSP(pRD->pCurrentContext, GetSP(pOtherRD->pCurrentContext));
1564+
#if defined(TARGET_AMD64) && defined(TARGET_WINDOWS)
1565+
pRD->SSP = pOtherRD->SSP;
1566+
#endif
15581567

15591568
#define CALLEE_SAVED_REGISTER(regname) pRD->pCurrentContext->regname = (pRD->pCurrentContextPointers->regname == NULL) ? pOtherRD->pCurrentContext->regname : *pRD->pCurrentContextPointers->regname;
15601569
ENUM_CALLEE_SAVED_REGISTERS();
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
using System;
4+
using System.Runtime.CompilerServices;
5+
using Xunit;
6+
7+
public class Test104820
8+
{
9+
// Test that SSP is updated properly when resuming after catch when the shadow stack
10+
// contains the instruction pointer of the resume frame multiple times and the SSP needs
11+
// to be restored to the location of one that's not the first one found on the shadow
12+
// stack.
13+
// This test fails with stack overflow of the shadow stack if the SSP is updated incorrectly.
14+
static void ShadowStackPointerUpdateTest(int depth)
15+
{
16+
if (depth == 0)
17+
{
18+
throw new Exception();
19+
}
20+
21+
for (int i = 0; i < 1000; i++)
22+
{
23+
try
24+
{
25+
try
26+
{
27+
ShadowStackPointerUpdateTest(depth - 1);
28+
}
29+
catch (Exception)
30+
{
31+
throw new Exception();
32+
}
33+
}
34+
catch (Exception) when (depth == 100)
35+
{
36+
}
37+
}
38+
}
39+
40+
[Fact]
41+
public static void TestEntryPoint()
42+
{
43+
ShadowStackPointerUpdateTest(100);
44+
}
45+
}
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
<Project Sdk="Microsoft.NET.Sdk">
2+
<PropertyGroup>
3+
<CLRTestPriority>1</CLRTestPriority>
4+
</PropertyGroup>
5+
<ItemGroup>
6+
<Compile Include="test104820.cs" />
7+
</ItemGroup>
8+
<ItemGroup>
9+
<ProjectReference Include="$(TestSourceDir)Common/CoreCLRTestLibrary/CoreCLRTestLibrary.csproj" />
10+
</ItemGroup>
11+
</Project>

0 commit comments

Comments
 (0)