search

android / ndk

The Android Native Development Kit
2k stars 257 forks source link

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

Open DanAlbert opened 1 year ago

DanAlbert commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 9 months ago

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.