Skip to content

Fix CET - remove writing to shadow stack#103171

Merged
mangod9 merged 1 commit intodotnet:mainfrom
janvorli:fix-cet-remove-wrssq
Jun 8, 2024
Merged

Fix CET - remove writing to shadow stack#103171
mangod9 merged 1 commit intodotnet:mainfrom
janvorli:fix-cet-remove-wrssq

Conversation

@janvorli
Copy link
Member

@janvorli janvorli commented Jun 7, 2024

In my recent fix for failures with CET enabled, I have also added a call to the _wrssq intrinsic to push an address to shadow stack. It turns out that instruction is privileged and cannot be used by user code. Moreover, I have realized that it is not needed at all there, so I am removing it.

@janvorli janvorli added this to the 9.0.0 milestone Jun 7, 2024
@janvorli janvorli requested a review from mangod9 June 7, 2024 18:13
@janvorli janvorli self-assigned this Jun 7, 2024
@janvorli
Copy link
Member Author

janvorli commented Jun 7, 2024

The reason why I have not detected that when testing my previous change locally was that I was testing it on machine with non-english Windows. I haven't realized the coreclr test results parsing scripts crash when parsing output on windows that are not running with english locale and so only the results of the tests that were not combined into the large test units are shown. And none of those tests was hitting the problem.

In my recent fix for failures with CET enabled, I have also added a call to
the _wrssq intrinsic to push an address to shadow stack. It turns out
that instruction is privileged and cannot be used by user code.
Moreover, I have realized that it is not needed at all there, so I am
removing it.
@janvorli janvorli force-pushed the fix-cet-remove-wrssq branch from dda965c to e98caa1 Compare June 7, 2024 22:19
@mangod9 mangod9 merged commit 39ecbe0 into dotnet:main Jun 8, 2024
@VSadov
Copy link
Member

VSadov commented Jun 8, 2024

I was wondering if the instruction must be privileged, but figured “if tests pass, it might be not” :)

@VSadov
Copy link
Member

VSadov commented Jun 8, 2024

The docs are somewhat arcane on what instructions are allowed and when.

@github-actions github-actions bot locked and limited conversation to collaborators Jul 9, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants