boostorg / boostorg/context

Windows ARM64 (PE) `ontop_fcontext`: epilog deallocates 0xc0 of the 0xd0 bytes the prolog allocates — 16-byte SP skew, breaks /GS and SEH-based unwind on resume

未关闭
#336 1 条评论 0 个 reaction 已指派 0 人 在 GitHub 查看
主要语言
Assembly
星标
370
派生
186
平均合并
24 天 6 小时
30 天内合并 PR
1

描述

## Summary

Both PE-target ARM64 assembly implementations of `ontop_fcontext`
(`src/asm/ontop_arm64_aapcs_pe_armasm.asm`, MSVC ARMASM64 syntax, and
`src/asm/ontop_arm64_aapcs_pe_armclang.S`, Clang/GAS syntax) have an asymmetric
prolog/epilog: the prolog reserves `0xd0` (208) bytes of stack, but the epilog releases
only `0xc0` (192) before tail-jumping into the ontop-function. The missing `0x10` (16)
bytes are never reclaimed, so `sp` sits 16 bytes below where it should be for the
remainder of the resumed context's execution.

Confirmed present, byte-for-byte, on `develop` @ `d2142b6925…` (current HEAD as of this
report). Confirmed **not** present in either of the two structurally analogous,
already-correct siblings on the same target:

- `src/asm/jump_arm64_aapcs_pe_armasm.asm` — symmetric, `0xd0`/`0xd0`.
- `src/asm/ontop_arm64_aapcs_elf_gas.S` (the ELF/GAS target's `ontop_fcontext`) —
symmetric, `0xb0`/`0xb0` (smaller frame: no Windows TEB/TIB save/restore, but internally
consistent).

This is a structural, input-independent skew — every invocation of the `ontop` path on
Windows ARM64 hits it, not a data-dependent corruption.

## Where it bites: `/GS` and SEH-based unwind, downstream

`ontop_fcontext` is reached via a tail-jump (`ret x2`, not a `call`), and the
ontop-function it jumps to eventually returns via the *target* context's own restored
`LR`, back into that target's original suspended call site — a site that expects `sp`
restored to its pre-suspend value, exactly as the symmetric `jump_fcontext` epilog would
leave it. The asymmetric `ontop_fcontext` epilog leaves `sp` 16 bytes below that value
instead, for the remaining lifetime of the resumed frame.

Any caller built with `/GS` (MSVC, on by default) or `-fstack-protector-all` (Clang) that
stores a stack cookie in a frame which gets resumed via this path will have its
`__security_check_cookie` read the wrong stack slot on that frame's own exit, and fail
fast with `STATUS_STACK_BUFFER_OVERRUN` (`0xc0000409`) — a `__fastfail`, not a catchable
SEH/C++ exception. A related failure mode hits `boost::context::fiber`'s
`forced_unwind`-driven teardown (which also uses the `ontop` path to inject an unwind
marker into the target context): Windows' `_CxxFrameHandler3` walks frames using static
`.pdata`/`.xdata` unwind metadata that encodes each frame's *expected* SP delta, and a
frame that's silently 16 bytes off from that expectation can make the unwinder
misidentify frame boundaries mid-walk.

We hit this downstream, three vendoring hops removed from this repo: `hermes-windows`
(Microsoft's Windows fork of Meta's Hermes JS engine, used by react-native-windows)
vendors `boost_1_86_0` wholesale, including this exact file pair, unmodified. On Windows
ARM64, running Hermes' interpreter on a custom fiber stack, this produces a deterministic
crash in a production React Native app under load (root-caused from 8 crash dumps across
2 engine builds and 4 ASLR bases — all module-relative-identical, confirming the
structural nature of the bug). Full writeup and a downstream fix for hermes-windows:
the companion downstream fix for hermes-windows (link added below once posted).

Anyone else running `boost::context::fiber` (or anything built on it — Boost.Fiber,
Boost.Coroutine2) on Windows ARM64 under `/GS` or `-fstack-protector-all`, exercising the
`ontop`/`resume_with`/fiber-teardown path under load, is exposed to the same defect.

## History: where this was introduced

The asymmetry was introduced in
[#201 "Windows arm64 fcontext support"](https://github.com/boostorg/context/pull/201)
(merged 2022-07-05), specifically in commit `abf8e04e23cf05a499594e674d1c90db39117662`
("Spport Windows arm64 cpp exception", 2022-06-26). That commit extended the frame from
`0xb0` to `0xd0` to save/restore four TEB fields (`TeStackBase`, `TeStackLimit`,
`TeDeallocationStack`, `TeFiberData`) so C++ exception unwinding sees a consistent TIB
across a fiber switch — a good and necessary change. It correctly rewired every other
offset in the function (the PC-save slot moved `0xa0`→`0xc0`, the new TIB fields landed
at `0xa0`/`0xb0`, the prolog's `sub` moved `0xb0`→`0xd0`) but only bumped the epilog's
`add` from `0xb0` to `0xc0` — a `0x10` step, not the `0x20` step every other offset in
the same commit got. The diff (against
`src/asm/ontop_arm64_aapcs_pe_armasm.asm`) makes the off-by-one-constant plain:

```diff
ontop_fcontext proc BOOST_CONTEXT_EXPORT
; prepare stack for GP + FPU
- sub sp, sp, #0xb0
+ sub sp, sp, #0xd0
...
; save LR as PC
- str x30, [sp, #0xa0]
+ str x30, [sp, #0xc0]
+
+ ; save current stack base and limit
+ ldp x5, x6, [x18, #0x08]
+ stp x5, x6, [sp, #0xa0]
+ ; save current fiber data and deallocation stack
+ ldr x5, [x18, #0x1478]
+ ldr x6, [x18, #0x20]
+ stp x5, x6, [sp, #0xb0]
...
+ ; restore stack base and limit
+ ldp x5, x6, [sp, #0xa0]
+ stp x5, x6, [x18, #0x08]
+ ; restore fiber data and deallocation stack
+ ldp x5, x6, [sp, #0xb0]
+ str x5, [x18, #0x1478]
+ str x6, [x18, #0x20]
...
; skip pc
; restore stack from GP + FPU
- add sp, sp, #0xb0
+ add sp, sp, #0xc0
```

A second commit, `e878e8edb2b2…` ("Convert ARM64 armasm to armclang for Windows clang",
2024-06-19), mechanically transcribed this file's ARMASM64 syntax into the Clang/GAS
syntax now at `src/asm/ontop_arm64_aapcs_pe_armclang.S`, carrying the same `0x10`
shortfall forward verbatim into the second file. Neither commit touched
`jump_fcontext`'s analogous epilog, which is why it stayed correct throughout.

## Proposed fix

Make the epilog symmetric with the prolog (`add sp, sp, #0xd0`) in both files, matching
`jump_fcontext` on the same target and `ontop_fcontext` on the ELF target. No other
register logic changes.

```diff
--- a/src/asm/ontop_arm64_aapcs_pe_armasm.asm
+++ b/src/asm/ontop_arm64_aapcs_pe_armasm.asm
@@ -122,7 +122,7 @@ ontop_fcontext proc BOOST_CONTEXT_EXPORT

; skip pc
; restore stack from GP + FPU
- add sp, sp, #0xc0
+ add sp, sp, #0xd0

; jump to ontop-function
ret x2
```

```diff
--- a/src/asm/ontop_arm64_aapcs_pe_armclang.S
+++ b/src/asm/ontop_arm64_aapcs_pe_armclang.S
@@ -128,7 +128,7 @@ ontop_fcontext:

// skip pc
// restore stack from GP + FPU
- add sp, sp, #0xc0
+ add sp, sp, #0xd0

// jump to ontop-function
ret x2
```

(Exact line numbers as of `develop` @ `d2142b6925…`: `src/asm/ontop_arm64_aapcs_pe_armasm.asm`
prolog at line 65, epilog at line 127; `src/asm/ontop_arm64_aapcs_pe_armclang.S` prolog
at line 71, epilog at line 133.)

## Validation status

- Root cause: confirmed by direct inspection of `develop` HEAD (both files, both
currently `0xd0`/`0xc0`) and by disassembly of a downstream production crash (8 dumps,
hermes-windows on Windows ARM64 — see linked report above). The skew is structural and
input-independent: it does not depend on data, timing, or heap state, only on whether
the `ontop` path is taken.
- Fix: validated by symmetry against the two already-correct sibling implementations in
this same file family (`jump_fcontext` PE variant, `ontop_fcontext` ELF variant), and
independently against a downstream engine that carries an identical fix — see the
linked hermes-windows PR for that fix's own validation status (empirical repro or
build+inspection, whichever applies at time of reading).
- We have **not** run this repo's own test suite (`libs/context/test`) against the fix —
we don't have a boost ARM64-on-Windows test harness in our environment. If maintainers
want a PR rather than (or in addition to) this issue, we're happy to open one with the
diff above; it's a minimal, mechanical, two-line change and should be straightforward
to verify against existing `Boost.Context`/`Boost.Fiber` ARM64 CI if one exists.

贡献指南

这个仓库没有索引到贡献指南

评估

这个 Issue 还没有评估数据。

把新 issue 发到你的邮箱

精选适合新手参与的 GitHub issue 摘要。