-
Notifications
You must be signed in to change notification settings - Fork 285
Add support for aarch64-unknown-linux-pauthtest
#755
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
jchlanda
wants to merge
3
commits into
rust-lang:master
Choose a base branch
from
jchlanda:jakub/pauthtest
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+33
−1
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is
ipnot signed? It is directly the result of_Unwind_GetIP. It should be possible to pass that to_Unwind_FindEnclosingFunction. How else would you even be able to use_Unwind_FindEnclosingFunctionotherwise?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A raw
ipis expected._Unwind_GetIPauthenticates the pointer and returns a stripped version (viaptrauth_auth_datahere).This is the crux of the issue.
_Unwind_FindEnclosingFunctionis fundamentally incompatible with the pointer authentication model. The problem is that_Unwind_FindEnclosingFunctiondoes not follow PAC semantics, it blindly creates a fresh unwind cursor and then resets the instruction pointer here:__unw_set_reg(cursor, UNW_REG_IP, pc);. As if to say: "treat thispcas if it belonged to this new cursor's frame".__unw_set_regdoes apply PAC logic internally (authenticates) under the assumption that theipbehaves like a return address for the current frame.This cannot be fixed on the Rust side.
_Unwind_FindEnclosingFunctionconstructs a new cursor (and therefore a newsp), so there is no way to correctly sign pointer: any signing we would perform would use the wrong context and would still fail authentication inside libunwind.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That sounds like a bug in libunwind to me. What purpose does
_Unwind_FindEnclosingFunctionhave at all if you can't pass the result of_Unwind_GetIPto it?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure, the documentation seems to be pretty scarce, and one I found is rather quite lax about possible outcomes of the call, explicitly allowing failure.
I personally don't read it as a bug. To me it is more that
_Unwind_FindEnclosingFunctionwas designed before pointer authentication existed, and its API implicitly assumes that instruction pointers are context-free values. But in PAC-aware code PC is only meaningful in the context of the (original) SP.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why does
_Unwind_FindEnclosingFunctionneed an authenticated pointer? It doesn't dereference the pointer, only looks it up in a side table with the result not having any security relevance. The only useful thing you can do with it afaik is looking up debuginfo for the function. Due to inlining and outlining you can't guarantee that it points to any particular function.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me re-phrase the problem here.
It does not, perhaps this was an oversimplification on my side. The real problem is about what invariant
__unw_set_regis enforcing under PAC - and since_Unwind_FindEnclosingFunctioncalls set reg, that's why the simplification crept into the discussion.You are right that
_Unwind_GetIPreturns a usable pointer. It authenticates a signed pointer with the originalspand gives you a stripped version.The problem shows up in
_Unwind_FindEnclosingFunction, because it creates a fresh cursor (ergo a newsp) and then calls:On PAC aware platforms this will go through an auth/resign sequence:
What that code does is it enforces that
value(ipfrom Rust) is a valid for this cursor'ssp- soipmust be a return address for the current frame.However, as you noted:
ipis stripped by_Unwind_GetIPand there is no way for us to sign it. So there is no way to make it pass auth/resign check. We can’t re-sign it correctly because we don’t have the originalsp, and using the newspwill fail authentication.So the issue isn’t just "signed vs unsigned". It’s that
_Unwind_FindEnclosingFunctionrelies on being able to treat a pointer as if it were a return address for an arbitrary frame, while__unw_set_reg(correctly) enforces that return addresses aresp-bound under PAC. And that is what makes_Unwind_FindEnclosingFunctionfundamentally incompatible.As a side note a notion of lack of support for
symbol_addressis already present in the test suite.