[NativeAOT] Adding CET support #102680

VSadov · 2024-05-25T00:37:45Z

Fixes: #101942

Added support for STATUS_RETURN_ADDRESS_HIJACK_ATTEMPT
Set /CETCOMPAT in executables by default, unless /CETCOMPAT:NO
Opt into EHCONT (if CFG is enabled)
Reconcile shadow stack with SP changes in RhpCallCatchFunclet

dotnet-policy-service · 2024-05-25T00:38:12Z

Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas
See info in area-owners.md if you want to be subscribed.

src/coreclr/vm/excep.cpp

VSadov · 2024-05-25T21:10:04Z

/azp run runtime-nativeaot-outerloop

azure-pipelines · 2024-05-25T21:10:12Z

Azure Pipelines successfully started running 1 pipeline(s).

janvorli · 2024-05-28T20:59:08Z

src/coreclr/nativeaot/Runtime/StackFrameIterator.cpp

+    // NOTE: When thunks are present, the thunk sequence may report a conservative GC reporting
+    // lower bound that must be applied when processing the managed frame.
+
+    ReturnAddressCategory category = CategorizeUnadjustedReturnAddress(m_ControlPC);


I am not sure how this is related to the CET stuff

InternalInit is supposed to leave the iterator in managed code. Until this change the InternalInit that takes NATIVE_CONTEXT was always starting from a location that is already in managed code. (unlike the one that takes PInvokeTransitionFrame)

Now, when we receive hijack exception, we pop to the caller and perform suspension in the caller's context.
Typically managed method that is not a reverse PInvoke must only be called by another managed method, so that would require no changes here. There are rare cases though in NativeAOT when ordinary managed methods are called from asm thunks. So it becomes possible for this code to see a location in a thunk. That is handled in the same way as in the other InternalInit method. (i.e. via UnwindNonEHThunkSequence)

Perhaps I should unify this code into a helper to not duplicate it in both InternalInit methods.

I've unified the adjustment for asm thunks

VSadov · 2024-05-29T02:03:25Z

Turns out my CET-compatible machine already enables usermode shadow stacks for executables that are CET-compatible. (i.e. msedge.exe, conhost.exe, ...). No extra steps necessary. This probably came with some OS update.

If we pass /CETCOMPAT to linker, NativeAOT executables are also opted in. And tests are passing, while running in "Compatible modules only" mode. (same as msedge.exe and others).

The next step is to make sure we are not relying on some compat/fixup behavior in exception handling, and whether we need /guard:ehcont

jkotas · 2024-05-29T02:07:42Z

src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.Windows.targets

@@ -99,6 +99,10 @@ The .NET Foundation licenses this file to you under the MIT license.
      <LinkerArg Condition="'$(OutputType)' == 'WinExe' or '$(OutputType)' == 'Exe'" Include="/STACK:$(IlcDefaultStackSize)" />
      <!-- Do not warn if someone declares UnmanagedCallersOnly with an entrypoint of 'DllGetClassObject' and similar -->
      <LinkerArg Include="/IGNORE:4104" />
+      <!-- Opt into CETCOMPAT by default. -->
+      <LinkerArg Condition="'$(ShadowStacksCompatible)' != 'false'" Include="/CETCOMPAT" />


We should call this property CETCompat. It is what it is called in .vcxproj files.

Also, I think we may want make it x64-only, just in case it starts doing something for x86.
According to docs /CETCOMPAT is accepted for x86 and currently does nothing.

I think we may want make it x64-only,

Sounds good to me.

(I will start doing something for arm64 eventually.)

Also /CETCOMPAT seems not accepted on arm64. Not yet, I guess.

VSadov · 2024-05-29T05:38:25Z

/azp run runtime-nativeaot-outerloop

azure-pipelines · 2024-05-29T05:38:40Z

Azure Pipelines successfully started running 1 pipeline(s).

VSadov · 2024-05-29T18:41:21Z

/guard:ehcont is a bit tricky. Once that is set, things like following start failing RIP validation in NtContinue/RtlRestoreContext .

    if (translateToManagedException)
    {
        pExPtrs->ContextRecord->SetIp(PCODEToPINSTR((PCODE)&RhpThrowHwEx));
        pExPtrs->ContextRecord->SetArg0Reg(faultCode);
        pExPtrs->ContextRecord->SetArg1Reg(faultingIP);

        return EXCEPTION_CONTINUE_EXECUTION;
    }

I think .GEHCONT might be a solution to this.

VSadov · 2024-05-29T18:43:40Z

The other interesting part is that /guard:ehcont requires CFG which we do not enable by default, as there are reasons.
One possibility is to enable /guard:ehcont only when CGF is already enabled.

For now I will assume that we will be enabling /guard:ehcont one way or another, together with CFG. We need to make the scenario work before settling under which conditions it is enabled.

VSadov · 2024-05-29T23:23:14Z

/azp run runtime-nativeaot-outerloop

azure-pipelines · 2024-05-29T23:23:24Z

Azure Pipelines successfully started running 1 pipeline(s).

VSadov · 2024-06-17T22:38:02Z

src/coreclr/vm/gcinfodecoder.cpp

@@ -786,7 +786,7 @@ bool GcInfoDecoder::EnumerateLiveSlots(

                UINT32 normStart = lastNormStop + normStartDelta;
                UINT32 normStop = normStart + normStopDelta;
-                if(normBreakOffset >= normStart && normBreakOffset < normStop)
+                if(normBreakOffset >= normStart && normBreakOffset <= normStop)


This is an ugly hack to workaround a rare case where a call to a hijacked method is immediately followed by a NoGC region thus the caller IP is outside of interruptible code.
It is always safe to pop to the caller of a regular managed method and start stackwalk at the call site, but when we do that as a part of hijack and the above condition is present, it triggers asserts if caller is fully-interruptible because we end up trying a stackwalk in a NoGC region.

A proper fix for this condition is in PR: #103540

VSadov · 2024-06-17T23:57:34Z

/azp run runtime-nativeaot-outerloop

azure-pipelines · 2024-06-17T23:57:49Z

Azure Pipelines successfully started running 1 pipeline(s).

VSadov · 2024-06-19T17:20:33Z

I think the technical part of this PR is complete - we can enable CET for NativeAOT. What remains is to settle on enablement policy and how to test this.

What I can suggest for enablement:

The basic opt-in to CET could happen unconditionally.
This is the part where we mark binary as CET compatible and if OS enables CET, we use different APIs for hijacking.
This is the state that mitigates typical ROP attacks, but things like exception handling could work as before and rely on OS fixups (we would not be relying on that).
In this mode the actual binary is roughly the same and the perf should be roughly the same.

It may be possible that some environments will be configured to treat this as "not enough CET" and require EHCONT as well.

The next level of opt-in is opting into EHCONT
The inconvenient part here is that it will also require opting into CFG, which has known costs in terms of size and throughput.
I can suggest to opt into EHCONT automatically if user has already opted into CFG.

Basically:

If user explicitly opts out of CET (as in /CETCOMPAT:NO), then nothing is enabled. Binary is not marked for CET
By default the binary is marked as CET compatible, but not opted into EHCONT
If CFG is enabled, we also opt into EHCONT.

I am basically optimizing for minimum number of knobs and assume that CET without EHCONT could be a good default.

Any other ideas?
@jkotas @janvorli

Re: https://techcommunity.microsoft.com/t5/windows-os-platform-blog/developer-guidance-for-hardware-enforced-stack-protection/ba-p/2163340

VSadov · 2024-06-19T17:27:30Z

For the testing part - it looks like we have a bunch of tests that are incompatible with CFG.
A PR without any of these changes, but with forcing CFG (#103215) demonstrates that Libraries tests mostly run, but smoke tests mostly fail.

We need to do something about that - either make tests compatible with CFG (where possible) or conditionally disable some tests/scenarios if can't made compatible.
Perhaps have a leg in runtime-nativeaot-outerloop that enables CFG?

@MichalStrehovsky

jkotas · 2024-06-19T18:46:30Z

I am basically optimizing for minimum number of knobs and assume that CET without EHCONT could be a good default.

Sounds good to me.

jkotas · 2024-06-19T18:57:12Z

For the testing part - it looks like we have a bunch of tests that are incompatible with CFG.

At least some of these failures look like product bugs to me. We want to fix them - CFG is a supported config for naot. Have you looked into where the problems are?

Perhaps have a leg in runtime-nativeaot-outerloop that enables CFG?

I agree that we can use more outer loops tests with CFG enabled given these results.

src/coreclr/nativeaot/Runtime/i386/GcProbe.asm

… us.

Co-authored-by: Jan Kotas <[email protected]>

jkotas

LGTM. Thank you!

src/coreclr/nativeaot/Runtime/windows/PalRedhawkMinWin.cpp

Co-authored-by: Jan Kotas <[email protected]>

VSadov · 2024-07-02T19:47:13Z

Thanks!!!

dotnet-issue-labeler bot added the area-NativeAOT-coreclr label May 25, 2024

dotnet-policy-service bot assigned VSadov May 25, 2024

jkotas reviewed May 25, 2024

View reviewed changes

src/coreclr/vm/excep.cpp Outdated Show resolved Hide resolved

build-analysis bot mentioned this pull request May 26, 2024

[x86] stress failure in RayTracer.GetNaturalColor with DOTNET_JitStress=2 #102590

Closed

janvorli reviewed May 28, 2024

View reviewed changes

VSadov force-pushed the aotCET01 branch from e94c69c to 42c2838 Compare May 28, 2024 21:48

jkotas reviewed May 29, 2024

View reviewed changes

VSadov force-pushed the aotCET01 branch 3 times, most recently from 73f484b to 051c2d7 Compare May 29, 2024 23:22

VSadov force-pushed the aotCET01 branch from 051c2d7 to 6765642 Compare June 9, 2024 23:46

VSadov force-pushed the aotCET01 branch from 6765642 to c5ccc84 Compare June 17, 2024 21:47

VSadov mentioned this pull request Jun 17, 2024

Allow GC on the first instruction of NoGC region (except in prologs/epilogs) #103540

Merged

VSadov commented Jun 17, 2024

View reviewed changes

VSadov force-pushed the aotCET01 branch from cb0f2df to d7777b3 Compare June 26, 2024 14:29

jkotas reviewed Jun 26, 2024

View reviewed changes

src/coreclr/nativeaot/Runtime/i386/GcProbe.asm Outdated Show resolved Hide resolved

This was referenced Jun 26, 2024

The Operation will be canceled. The next steps may not contain expected logs. dotnet/dnceng#3008

Open

The job running on agent NetCore-Public ran longer than the maximum time #104044

Closed

VSadov mentioned this pull request Jun 29, 2024

Use resumable leaf frames in CET hijack and in GC stress. #104198

Merged

VSadov and others added 17 commits July 1, 2024 15:31

Add support for STATUS_RETURN_ADDRESS_HIJACK_ATTEMPT

ecb5c6c

fix build with clang

a56e166

Allow hijacked returns that land in assembly thunks

0f0c3b6

fix x86 build

e69f551

fail fast if hijack is hit on an unhijacked thread.

c518684

comment

0ab6e40

assert that OS unhijacked the thread to the same target as stashed by…

f0193bf

… us.

opt into CETCOMPAT by default

bd1ac22

unify adjustment for thunks

3411400

Use CETCompat as condition. Narrow to x64.

931e37e

Enable EHCONT, if CET and CFG are enabled

7711ad8

tweak comments

a0c798b

Reconcile shadow stack with SP changes in RhpCallCatchFunclet

c991f59

Use RhFailFast for failfast

af84a14

drop __fastcall in HijackFunc

125b742

Apply suggestions from code review

4daf9cd

Co-authored-by: Jan Kotas <[email protected]>

remove fastcall from RhpHijackForGcStress

ca52149

VSadov force-pushed the aotCET01 branch from 07f4258 to ca52149 Compare July 1, 2024 22:38

build-analysis bot mentioned this pull request Jul 2, 2024

Failed test: JIT/SIMD/LdfldGeneric_ro/LdfldGeneric_ro (arm64) #104263

Closed

jkotas approved these changes Jul 2, 2024

View reviewed changes

src/coreclr/nativeaot/Runtime/windows/PalRedhawkMinWin.cpp Outdated Show resolved Hide resolved

src/coreclr/nativeaot/Runtime/windows/PalRedhawkMinWin.cpp Outdated Show resolved Hide resolved

src/coreclr/nativeaot/Runtime/windows/PalRedhawkMinWin.cpp Outdated Show resolved Hide resolved

Apply suggestions from code review

024208a

Co-authored-by: Jan Kotas <[email protected]>

VSadov merged commit c8e4f2c into dotnet:main Jul 2, 2024
89 checks passed

VSadov deleted the aotCET01 branch July 2, 2024 19:47

github-actions bot locked and limited conversation to collaborators Aug 2, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[NativeAOT] Adding CET support #102680

[NativeAOT] Adding CET support #102680

VSadov commented May 25, 2024 •

edited

Loading

dotnet-policy-service bot commented May 25, 2024

VSadov commented May 25, 2024

azure-pipelines bot commented May 25, 2024

janvorli May 28, 2024

VSadov May 28, 2024

VSadov May 29, 2024

VSadov commented May 29, 2024 •

edited

Loading

jkotas May 29, 2024

VSadov May 29, 2024 •

edited

Loading

jkotas May 29, 2024

jkotas May 29, 2024

VSadov May 29, 2024 •

edited

Loading

VSadov commented May 29, 2024

azure-pipelines bot commented May 29, 2024

VSadov commented May 29, 2024 •

edited

Loading

VSadov commented May 29, 2024 •

edited

Loading

VSadov commented May 29, 2024

azure-pipelines bot commented May 29, 2024

VSadov Jun 17, 2024 •

edited

Loading

VSadov commented Jun 17, 2024

azure-pipelines bot commented Jun 17, 2024

VSadov commented Jun 19, 2024 •

edited

Loading

VSadov commented Jun 19, 2024 •

edited

Loading

jkotas commented Jun 19, 2024

jkotas commented Jun 19, 2024

jkotas left a comment

VSadov commented Jul 2, 2024

[NativeAOT] Adding CET support #102680

[NativeAOT] Adding CET support #102680

Conversation

VSadov commented May 25, 2024 • edited Loading

dotnet-policy-service bot commented May 25, 2024

VSadov commented May 25, 2024

azure-pipelines bot commented May 25, 2024

janvorli May 28, 2024

Choose a reason for hiding this comment

VSadov May 28, 2024

Choose a reason for hiding this comment

VSadov May 29, 2024

Choose a reason for hiding this comment

VSadov commented May 29, 2024 • edited Loading

jkotas May 29, 2024

Choose a reason for hiding this comment

VSadov May 29, 2024 • edited Loading

Choose a reason for hiding this comment

jkotas May 29, 2024

Choose a reason for hiding this comment

jkotas May 29, 2024

Choose a reason for hiding this comment

VSadov May 29, 2024 • edited Loading

Choose a reason for hiding this comment

VSadov commented May 29, 2024

azure-pipelines bot commented May 29, 2024

VSadov commented May 29, 2024 • edited Loading

VSadov commented May 29, 2024 • edited Loading

VSadov commented May 29, 2024

azure-pipelines bot commented May 29, 2024

VSadov Jun 17, 2024 • edited Loading

Choose a reason for hiding this comment

VSadov commented Jun 17, 2024

azure-pipelines bot commented Jun 17, 2024

VSadov commented Jun 19, 2024 • edited Loading

VSadov commented Jun 19, 2024 • edited Loading

jkotas commented Jun 19, 2024

jkotas commented Jun 19, 2024

jkotas left a comment

Choose a reason for hiding this comment

VSadov commented Jul 2, 2024

VSadov commented May 25, 2024 •

edited

Loading

VSadov commented May 29, 2024 •

edited

Loading

VSadov May 29, 2024 •

edited

Loading

VSadov May 29, 2024 •

edited

Loading

VSadov commented May 29, 2024 •

edited

Loading

VSadov commented May 29, 2024 •

edited

Loading

VSadov Jun 17, 2024 •

edited

Loading

VSadov commented Jun 19, 2024 •

edited

Loading

VSadov commented Jun 19, 2024 •

edited

Loading