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

[FR] enable PAC/BTI by default? #1914

Open
DanAlbert opened this issue Aug 3, 2023 · 6 comments
Open

[FR] enable PAC/BTI by default? #1914

DanAlbert opened this issue Aug 3, 2023 · 6 comments

Comments

@DanAlbert
Copy link
Member

Description

NDK r25 shipped support for PAC/BTI, but it was (and remains in r26) off by default. These features mitigate ROP/JOP attacks, and our docs (https://developer.android.com/ndk/guides/memory-debug) recommend always enabling them.

The downsides to enabling this by default are a marginal (1%) code size increase, and a risk that developers may do their testing on devices that do not support PAC/BTI. The instructions are NOPs on devices that do not support them so that won't cause problems if the code is correct, but it does mean that developers won't know if their PAC/BTI enabled code will run correctly on a capable device.

The code size increase is marginal, and we prefer to be safe by default. Developers can opt out of PAC/BTI is that marginal increase in code size is not worth the increased security for their app.

The odds that developers have PAC/BTI capable devices for testing are going up every year, and it sounds like it might be a reasonable option in 2024 (when r27 will ship). It will of course be mentioned in the changelog so developers without such devices will be aware of the risk.

@enh-google
Copy link
Collaborator

here's the "citation needed" for my claim that chrome already shipped a PAC/BTI-enabled binary:
https://community.arm.com/arm-community-blogs/b/operating-systems-blog/posts/control-flow-integrity

(though as that blog post mentions, the real question is always "at what point does an app developer actually have access to the hardware?".)

also, to surface it here rather than on the other side of that link, the Samsung Galaxy S22 and Vivo X80 5G were early phones to ship PAC/BTI enablement.

@DanAlbert
Copy link
Member Author

My inclination is to do this in r27 with an announcement in the changelog so it doesn't surprise anyone.

iirc false positives are possible, but only for hand written assembler that doesn't handle it?

@enh-google
Copy link
Collaborator

iirc false positives are possible, but only for hand written assembler that doesn't handle it?

the only problem i've seen was hand-written assembler that tried to use PAC but got the stack manipulation mixed up (since the current value of the stack pointer is one of the inputs, you need to make sure that your PAC instructions are in the right place relative to your sp sub/add instructions).

@DanAlbert
Copy link
Member Author

Ah, that's right, I'd remembered wrong.

So the only place this is likely to cause trouble is for anyone that has inactive PAC/BTI code that's wrong, and everywhere else it causes "trouble" should actually be the mitigation working correctly? I'm even less worried then.

@enh-google
Copy link
Collaborator

yeah, about the only possibility i can think of is that maybe there's some app drm that will be incompatible with this (since drm is largely indistinguishable from malware)? but that's a catch-22 that will always be in the way if we do nothing (and an easy choice --- fix the drm or disable PAC/BTI, whichever you prefer [though if you're using app drm in the first place, PAC/BTI is likely to be of interest to you for similar reasons!]).

@DanAlbert
Copy link
Member Author

Revisiting this, this should probably be done in the driver so it's not unique to our build systems. A bit late to do that for r27, so retriaging to r28.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Awaiting triage
Status: Triaged
Development

No branches or pull requests

2 participants