Skip to content
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

fix(backtrace): Strip pointer auth mask before returning usable IP #319

Merged
merged 1 commit into from
Nov 29, 2018

Conversation

kattrali
Copy link
Contributor

@kattrali kattrali commented Nov 27, 2018

Background

The Arm64e architecture introduces pointer authentication codes to
detect and guard against unexpected changes to pointers in memory. For
most application functions, this is a nice bonus that nobody needs to
think about, however, for a crash reporting library, we need to strip
this extraneous value ahead of time to avoid reporting a stack frame
address which does not align to a function exactly, causing unreadable
stack traces.

PACs work by adding a signature to the higher order bits of a pointer
before it is stored. When the pointer is read, the signature is
validated prior to executing a function. If the signature has been
tampered with, then the app is forced to crash rather than execute
altered code.

Design

Modified the Arm64-specific method for reading the instruction
pointer from a crash context to strip a value prior to reporting the
value.

Tests

@kattrali kattrali force-pushed the kattrali/arm64e-fix branch from 73936e6 to a6a50f5 Compare November 27, 2018 01:15
@fractalwrench fractalwrench self-requested a review November 27, 2018 11:05
Copy link
Contributor

@fractalwrench fractalwrench left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change seems reasonable to me. Approving on the understanding that it's been tested on an iPhone XS, and that the symbolication of stack addresses has been checked.

The Arm64e architecture introduces pointer authentication codes to
detect and guard against unexpected changes to pointers in memory. For
most application functions, this is a nice bonus that nobody needs to
think about, however, for a crash reporting library, we need to strip
this extraneous value ahead of time to avoid reporting a stack frame
address which does not align to a function exactly, causing unreadable
stack traces.

PACs work by adding a signature to the higher order bits of a pointer
before it is stored. When the pointer is read, the signature is
validated prior to executing a function. If the signature has been
tampered with, then the app is forced to crash rather than execute
altered code.
@kattrali kattrali force-pushed the kattrali/arm64e-fix branch from a6a50f5 to bffdf4b Compare November 28, 2018 18:35
@kattrali kattrali merged commit f0f22a4 into master Nov 29, 2018
@kattrali kattrali deleted the kattrali/arm64e-fix branch November 29, 2018 01:18
* devices. Example usage, assuming the usage is guarded for __arm64__:
* uintptr_t ptr_address = ptr & BSG_PACStrippingMaskArm64e;
*/
#define BSG_PACStrippingMaskArm64e 0x0000000fffffffff
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The address is located at VA_SIZE - 1; what's the rationale to hardcode 36bits here? Is this true for all devices?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @Reflejo! It is true for the existing devices, but you're right, and it could change in the future. I'll slot it in for a subsequent patch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants