Skip to content

Commit 38f741f

Browse files
committed
Remove pointer authentication from fibers
This commit removes the pointer authentication instructions from our fiber implementation. There's more rationale in bytecodealliance#12778 for this but the basic problems are: * Android uses the "A" key to sign return addresses, but the "A" key differs between threads. This causes resuming fibers across threads to crash. * Rust does not provide the ability to know when pointer authentication is enabled, nor does it have a way of enabling pointer authentication for generated code on stable. * We can't currently find reference documentation for which key should be used ABI-wise for each target, nor why the Android "A" key is changing across threads in the same process. Overall it seems like we're a bit too far out on a limb trying to enable this. In the future if all Rust code, except this inline assembly, uses pointer authentication we'll be in a good spot to reevaluate this perhaps. Until that day though it seems premature to enable this. Closes bytecodealliance#12778
1 parent df15d7c commit 38f741f

File tree

2 files changed

+15
-63
lines changed

2 files changed

+15
-63
lines changed

crates/fiber/src/lib.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -438,4 +438,18 @@ mod tests {
438438
assert!(FiberStack::new(usize::MAX, true).is_err());
439439
assert!(FiberStack::new(usize::MAX, false).is_err());
440440
}
441+
442+
#[test]
443+
fn cross_thread_fiber() {
444+
let fiber = Fiber::<(), (), ()>::new(fiber_stack(1024 * 1024), move |_, s| {
445+
s.suspend(());
446+
})
447+
.unwrap();
448+
assert!(fiber.resume(()).is_err());
449+
std::thread::spawn(move || {
450+
assert!(fiber.resume(()).is_ok());
451+
})
452+
.join()
453+
.unwrap();
454+
}
441455
}

crates/fiber/src/stackswitch/aarch64.rs

Lines changed: 1 addition & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -7,31 +7,9 @@
77
//
88
// Also at this time this file is heavily based off the x86_64 file, so you'll
99
// probably want to read that one as well.
10-
//
11-
// Finally, control flow integrity hardening has been applied to the code using
12-
// the Pointer Authentication (PAuth) and Branch Target Identification (BTI)
13-
// technologies from the Arm instruction set architecture:
14-
// * All callable functions start with either the `BTI c` or `PACIASP`/`PACIBSP`
15-
// instructions
16-
// * Return addresses are signed and authenticated using the stack pointer
17-
// value as a modifier (similarly to the salt in a HMAC operation); the
18-
// `DW_CFA_AARCH64_negate_ra_state` DWARF operation (aliased with the
19-
// `.cfi_window_save` assembler directive) informs an unwinder about this
2010

2111
use core::arch::naked_asm;
2212

23-
cfg_if::cfg_if! {
24-
if #[cfg(target_vendor = "apple")] {
25-
macro_rules! paci1716 { () => ("pacib1716\n"); }
26-
macro_rules! pacisp { () => ("pacibsp\n"); }
27-
macro_rules! autisp { () => ("autibsp\n"); }
28-
} else {
29-
macro_rules! paci1716 { () => ("pacia1716\n"); }
30-
macro_rules! pacisp { () => ("paciasp\n"); }
31-
macro_rules! autisp { () => ("autiasp\n"); }
32-
}
33-
}
34-
3513
#[inline(never)] // FIXME(rust-lang/rust#148307)
3614
pub(crate) unsafe extern "C" fn wasmtime_fiber_switch(top_of_stack: *mut u8) {
3715
unsafe { wasmtime_fiber_switch_(top_of_stack) }
@@ -42,10 +20,6 @@ unsafe extern "C" fn wasmtime_fiber_switch_(top_of_stack: *mut u8 /* x0 */) {
4220
naked_asm!(concat!(
4321
"
4422
.cfi_startproc
45-
",
46-
pacisp!(),
47-
"
48-
.cfi_window_save
4923
// Save all callee-saved registers on the stack since we're
5024
// assuming they're clobbered as a result of the stack switch.
5125
stp x29, x30, [sp, -16]!
@@ -80,10 +54,6 @@ unsafe extern "C" fn wasmtime_fiber_switch_(top_of_stack: *mut u8 /* x0 */) {
8054
ldp x25, x26, [sp], 16
8155
ldp x27, x28, [sp], 16
8256
ldp x29, x30, [sp], 16
83-
",
84-
autisp!(),
85-
"
86-
.cfi_window_save
8757
ret
8858
.cfi_endproc
8959
",
@@ -133,45 +103,14 @@ pub(crate) unsafe fn wasmtime_fiber_init(
133103
x20: entry_point as *mut u8,
134104
x21: entry_arg0,
135105
x22: wasmtime_fiber_switch_ as *mut u8,
136-
137-
// We set up the newly initialized fiber, so that it resumes
138-
// execution from wasmtime_fiber_start(). As a result, we need a
139-
// signed address of this function because `wasmtime_fiber_switch`
140-
// ends with a `auti{a,b}sp` instruction. There are 2 requirements:
141-
// * We would like to use an instruction that is executed as a no-op
142-
// by processors that do not support PAuth, so that the code is
143-
// backward-compatible and there is no duplication; `PACIA1716` is
144-
// a suitable one.
145-
// * The fiber stack pointer value that is used by the signing
146-
// operation must match the value when the pointer is
147-
// authenticated inside wasmtime_fiber_switch(), which is 16 bytes
148-
// below the `top_of_stack` which will be `sp` at the time of the
149-
// `auti{a,b}sp`.
150-
//
151-
// TODO: Use the PACGA instruction to authenticate the saved register
152-
// state, which avoids creating signed pointers to
153-
// wasmtime_fiber_start(), and provides wider coverage.
154-
lr: paci1716(wasmtime_fiber_start as *mut u8, top_of_stack.sub(16)),
106+
lr: wasmtime_fiber_start as *mut u8,
155107

156108
last_sp: initial_stack.cast(),
157109
..InitialStack::default()
158110
});
159111
}
160112
}
161113

162-
/// Signs `r17` with the value in `r16` using either `paci{a,b}1716` depending
163-
/// on the platform.
164-
fn paci1716(mut r17: *mut u8, r16: *mut u8) -> *mut u8 {
165-
unsafe {
166-
core::arch::asm!(
167-
paci1716!(),
168-
inout("x17") r17,
169-
in("x16") r16,
170-
);
171-
r17
172-
}
173-
}
174-
175114
// See the x86_64 file for more commentary on what these CFI directives are
176115
// doing. Like over there note that the relative offsets to registers here
177116
// match the frame layout in `wasmtime_fiber_switch`.
@@ -188,7 +127,6 @@ unsafe extern "C" fn wasmtime_fiber_start() -> ! {
188127
0x23, 0xa0, 0x1 /* DW_OP_plus_uconst 0xa0 */
189128
.cfi_rel_offset x30, -0x08
190129
.cfi_rel_offset x29, -0x10
191-
.cfi_window_save
192130
.cfi_rel_offset x28, -0x18
193131
.cfi_rel_offset x27, -0x20
194132
.cfi_rel_offset x26, -0x28

0 commit comments

Comments
 (0)