Arm64: [PAC-RET] NativeAOT changes#128950
Conversation
As suggested in [comment](dotnet#125436 (comment)), this PR covers subset of changes from dotnet#125436 related to NativeAOT. More details on PAC and its role in software security can be found ([here](https://llsoftsec.github.io/llsoftsecbook/#sec:pointer-authentication)).
|
Tagging subscribers to this area: @agocke, @dotnet/ilc-contrib |
|
/azp run runtime-nativeaot-outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Tests failing on linux-arm64 with |
|
The CdacTest failures on MacOS seem unrelated to the patch. |
|
/azp run runtime-nativeaot-outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Tests are crashing on linux-arm64 and osx-arm64 |
Add assert to ensure PAC is being emitted before stack adjustment Change-Id: Ifb636d2bb3fd3c78d33274ea7786291b5ee83fd6
Change-Id: Ic64dd02db14e691d3ea88212544059e9d9892062
These crashes came from changes to bail out if we interrupt on RET when pac is enabled. Previously PR was handling RET incorrect in |
| return false; | ||
| } | ||
|
|
||
| // At RET, FP/LR/SP have already been restored to the caller state so |
There was a problem hiding this comment.
Should this check be in TrailingEpilogueInstructionsCount so that all epilog-related checks are together?
There was a problem hiding this comment.
True, it was in there before and returned 1 on RET instruction (cannot return 0/-1 as it means not in in epilog/unknown). However when PAC is disabled, it allowed hijacking the current frame when interrupted on the RET instruction. That caused the crashes that we saw earlier. To keep PAC related changes separate, I added them in PAC specific area. I can extract the RET check into a separate function for better readability.
| #if defined(TARGET_ARM64) | ||
| if (pacPresent && epilogueInstructions != 0) | ||
| { | ||
| // In an epilog, LR/SP may already be partially restored. Avoid hijacking |
There was a problem hiding this comment.
This looks like unreachable code. TrailingEpilogueInstructionsCount on Arm64 returns -1 or 0. We have checked for -1 above, so epilogueInstructions should be 0 here.
There was a problem hiding this comment.
That's unreachable code now. epilogueInstructions == 0 would be true for regular hijacks in function body so having that check will bailout regular hijacks in function body outside prolog/epilog. Removing the check as it's no longer needed.
This PR covers the final subset of changes from #125436 related to NativeAOT as suggested in comment.
It follows the previous work from- #127949, #127838 and #128147.
More details on PAC and its role in software security can be found (here).
cc: @dotnet/arm64-contrib @a74nh @jkotas @dhartglassMSFT