Repository navigation
Arm64: [PAC-RET] cDAC changes - #135959
SwapnilGaikwad wants to merge 14 commits into
Conversation
Strip PAC from saved return addresses before unwinding and resolving code.
dotnet-policy-service
Bot
added
the
community-contribution
|
Azure Pipelines: Successfully started running 4 pipeline(s). 12 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag |
|
The CI status of changes when PAC is enabled is found on PR #125436. |
| #if defined(TARGET_UNIX) | ||
| *Pointer &= 0x0000FFFFFFFFFFFF; | ||
| #else | ||
| // Windows uses 47-bit user addresses, so bit 47 can also contain PAC. |
There was a problem hiding this comment.
We may need to change wording with certainty if we can confirm TCR_EL1.T0SZ always = 17 on Windows. It would make PAC algorithm use TCR_EL1.T0SZ = PACEffectiveTxSZ = 17 in InsertPAC(). Thus, bit indexed 47 will always be used for storing PAC.
Some runtime paths report stack-interior references without using PromoteCarefully. Filter these references from both cDAC and runtime results before comparison so asymmetric filtering does not produce false stress-test failures. NOTE: This PR is created with the help of codex to address failing CI pipeline for #135959 cc: @dotnet/arm64-contrib @dhartglassMSFT @max-charlamb
| return new TargetPointer(code.Value); | ||
| } | ||
|
|
||
| internal static TargetCodePointer StripPtrAuthFromReturnAddress(TargetCodePointer returnAddress, Target target) |
There was a problem hiding this comment.
Is there a reason this is separated out from stripping the thumb bit?
This breaks the idea that TargetCodePointers are the platform representation and TargetPointers are able to be used for comparison / arithmetic.
If there are cases we aren't converting correctly, I'd rather update those to use the proper TargetCodePointer/TargetPointer over changing the semantics.
There was a problem hiding this comment.
This applies to return addresses during stackwalk only. It does not apply to arbitrary code addresses like Arm32 thumb-bit stripping.
There was a problem hiding this comment.
AddressFromCodePointer should handle both the PAC stripping and thumb bit stripping (where applicable), right now it does not.
There was a problem hiding this comment.
This feature is about PAC for return addresses only.
If the PAC striping is done in AddressFromCodePointer, it is going to be done for code addresses, not just for return addresses. I guess it would work, but it would behave differently from how the runtime behaves. @max-charlamb Are you intentionally accepting this divergence to simplify cDAC?
|
The CI failures look unrelated to PAC changes. |
|
Changes look pretty good. |
This PR covers the cDAC subset of changes from 1#25436 related to cDAC.
It follows the previous work from- #127949, #127838, #128147 and #128950
More details on PAC and its role in software security can be found (here).
cc: @dotnet/arm64-contrib @a74nh @jkotas @dhartglassMSFT