[3] [JSC] Move DataView null vector check in IC outside of register save/restore

HighJSC inline cachesOOB

An IC exit undid a stack adjustment that had never happened.

34669c8

High. A cold, script-triggerable exit path returns to compiled JavaScript with a stack pointer raised past live frame data and registers refilled from the caller's own frame. Escalation hinges on how much control script has over the 64-bit words sitting in the slots the bogus fills read.

Machine-code stubs emitted by a JIT must honour one contract above all others: every exit path returns to the caller with exactly the stack pointer and register contents it was entered with. JSC's inline caches are those stubs — small specialised code fragments that handle a property operation for a particular object shape. When a stub needs more scratch registers than are free at the call site, it borrows registers that are live, spilling them to the stack in a prologue and refilling them in a matching epilogue. The prologue and epilogue are a matched pair: the prologue lowers the stack pointer and stores into the space it just made, the epilogue reloads from that space and raises the pointer back.

The angle: any page can detach a resizable ArrayBuffer behind a hot polymorphic byteLength read and drive JIT-compiled JavaScript to continue executing with a desynchronized stack pointer and registers holding words lifted out of its own live frame.

The commit message describes the shape precisely:

In the DataView byteLength getter IC, there is a null vector check which executes before a preserveReusedRegistersByPushing, but jumps to a point before restoreReusedRegistersByPopping. In other words, causing a misbalanced push and pop of register state.

This PR fixes by making the null vector check jump to after the popping of saved registers.

Source/JavaScriptCore/bytecode/InlineCacheCompiler.cpp

JSTests/stress/dataview-bytelength-ic-stub-stack-desync.js

Patch Details

The change is confined to InlineCacheCompiler::emitIntrinsicGetter, in the USE(JSVALUE64) branch taken when isResizableOrGrowableSharedTypedArrayIncludingDataView(accessCase.structure()->classInfoForCells()) holds. Previously a single failAndIgnore JumpList collected side-exit branches created at two different points in the stub's stack-depth timeline: a guard emitted earlier in the function, before allocator.preserveReusedRegistersByPushing(...) — the patch's own comment identifies it as the null-vector guard — and the outOfBounds branch produced by jit.loadDataViewByteLength(...), emitted after the push. Both were linked at a common label that ran allocator.restoreReusedRegistersByPopping(jit, preservedState) before jumping to m_failAndIgnore.

The patch splits the two epochs. The pre-push failAndIgnore is appended directly to m_failAndIgnore with no restore, and a new local CCallHelpers::JumpList postPushFailAndIgnore collects the outOfBounds branch, which alone is linked to the pop-then-jump epilogue. The terminal if (allocator.didReuseRegisters() && !failAndIgnore.empty()) / else m_failAndIgnore.append(failAndIgnore) pair is rewritten to operate on postPushFailAndIgnore. A regression test is added that builds a polymorphic byteLength IC under high register pressure and then detaches the buffer with ArrayBuffer.prototype.transfer().

Branch targets from two different stack-depth epochs merged into one jump list, so a side exit taken before a prologue's stack adjustment executes the epilogue that undoes it.

Background

Inline caches. JSC caches property operations as small machine-code stubs specialised for a particular object structure. InlineCacheCompiler::emitIntrinsicGetter emits the stub body for getters implemented as intrinsics, such as DataView.prototype.byteLength.

Polymorphic ICs. When a call site sees several different object shapes, the IC holds several AccessCases chained together, and stub generation happens with more of the caller's registers marked live.

ScratchRegisterAllocator. A stub-local register allocator. allocateScratchGPR() first hands out registers that are neither locked nor live at the call site; if none remain, it hands out a live one and increments m_numberOfReusedRegisters, which makes didReuseRegisters() return true.

preserveReusedRegistersByPushing / restoreReusedRegistersByPopping. The matched prologue and epilogue for borrowed registers. The prologue subtracts a rounded-up byte count from the stack pointer and then stores the reused registers at positive offsets from the lowered stack pointer, returning a PreservedState carrying numberOfBytesPreserved; the epilogue reloads those registers from the same stack-pointer-relative offsets and adds the byte count back. Both are no-ops when didReuseRegisters() is false.

JumpList and link. An assembler-level list of pending forward branches. list.link(&jit) binds every branch in the list to the assembler's current code location, so all members of a list share one destination. Crucially, a JumpList records only the origin of each branch in the instruction stream — it carries no information about the stack depth in effect when the branch was created.

failAndIgnore versus succeed(). A stub exits either through its success continuation or through a failure path; m_failAndIgnore is the compiler-level collection of branches that abandon this stub and fall back to the generic slow path.

Resizable ArrayBuffers and transfer(). new ArrayBuffer(n, { maxByteLength: m }) creates a resizable buffer, and views over it take the isResizableOrGrowableSharedTypedArrayIncludingDataView code path, whose length must be computed at runtime by loadDataViewByteLength. ArrayBuffer.prototype.transfer() detaches the buffer, after which views over it have a null backing vector.

Analysis

This is a JIT stack and register state desynchronization — an unbalanced push/pop pair producing stack-pointer corruption and clobbering of live caller registers.

  Stub timeline (didReuseRegisters() == true)
  ──────────────────────────────────────────
  entry sp = S
    null-vector guard emitted here ──────────┐   (epoch A: sp == S)
  preserveReusedRegistersByPushing:          │
    subPtr N, sp        -> sp = S - N        │
    store regs at [sp + off]                 │
    loadDataViewByteLength -> outOfBounds ─┐ │   (epoch B: sp == S - N)
  restoreReusedRegistersByPopping (success)│ │
  succeed()                                │ │
                                           ▼ ▼
  shared label:  load regs from [sp + off]      <-- with sp == S, reads
                 addPtr N, sp                       CALLER'S LIVE FRAME
                 jump m_failAndIgnore              and then sp = S + N

The null-vector guard and the outOfBounds branch belong to different epochs in the diagram, yet both landed on the shared label. When the IC is polymorphic and register pressure at the call site is high enough that allocateScratchGPR() must reuse a live register, m_numberOfReusedRegisters becomes non-zero, didReuseRegisters() is true, and both halves of the pair emit real code: a subPtr plus spills, and fills plus a matching addPtr.

If the DataView's backing vector is null because the buffer was detached, the epoch-A guard branches straight to the epilogue label. Execution then runs the restore sequence even though the corresponding subPtr never executed. Because the spill slots are addressed at positive offsets from the lowered stack pointer ([sp + extraBytesAtTopOfStack + offset]), running the fill without the preceding subPtr reads words above the entry stack pointer — memory inside the caller's currently live frame region, not the spill area that was supposed to hold the saved values. The reused live registers are overwritten with unrelated live-frame words, and the stack pointer is then raised by preservedState.numberOfBytesPreserved past live frame data. Control continues via m_failAndIgnore into the IC's slow path and back into the compiled JS function, now running with both a desynchronized stack pointer and registers holding values the register allocator did not put there.

Every element of the trigger is plain JavaScript, and the added regression test is itself a working one. Walking it:

1. The loop calls hot() with decoy, decoy2, decoy3 and dv, so the o.byteLength site's IC becomes polymorphic across four structures and reaches stub compilation in emitIntrinsicGetter.
2. The 32 simultaneously live p0..p31 locals are shaped to keep nearly all GPRs occupied so that allocateScratchGPR() for scratch2GPR must take a live register, which would make didReuseRegisters() true and cause the prologue to emit a real stack adjustment. The supplied ScratchRegisterAllocator sources show this mechanism but do not establish that this particular test reaches the reuse case, so that step is an inference from the test's construction.
3. dv's structure selects the resizable-buffer branch, whose stub contains both the pre-push guard and the post-push outOfBounds branch.
4. ab.transfer() detaches the buffer, so the DataView's vector is null.
5. The final hot(dv, A, objs) enters the stub, the pre-push guard fires before any push, and control lands on the epilogue that executes restoreReusedRegistersByPopping.

The immediate observable effect: the reused registers are refilled from words at positive offsets from the entry stack pointer — inside the caller's live frame region rather than a spill area that was never allocated — and the stack pointer is then raised, after which m_failAndIgnore returns control to the compiled function. The test's p0.marker ... p31.marker reads exist to observe the clobbering that follows; the typical outcome is a crash or garbage values.

Escalation would require, in order: (a) that the live-frame words read back by the fill sequence at the reused registers' offsets can be influenced by the attacker through JS values the compiled function itself keeps in those frame slots — the supplied context includes preserveRegistersToStackForCall/restoreRegistersFromStackForCall but not the frame-layout model that would fix which slots those offsets land on, so the degree of control is a projection; (b) that at least one clobbered register is one the compiled function believes holds a live JSValue, which is the shape the test's pN locals establish; and (c) that the injected bit pattern survives to a use site that dereferences it as a cell. If all three held, an attacker could obtain a fake-JSValue injection and from there a type-confusion primitive suitable for building arbitrary read/write. Separately, the raised stack pointer means subsequent calls made by the same function could place callee frames overlapping the caller's live frame slots, which might yield a second, independent corruption channel.

The bug lives entirely within the WebContent process; no sandbox boundary is crossed, and a separate escape would still be required after achieving renderer code execution.

The discovery signature points at targeted pattern auditing of the resizable-ArrayBuffer IC paths rather than generic fuzzing. A side exit that skips or duplicates a stack-adjustment epilogue is found by reading stub emission code for push/pop balance, since the faulty path requires the conjunction of a polymorphic IC, enough register pressure to force didReuseRegisters() true, a resizable-buffer-backed DataView, and a detached buffer. The test is hand-shaped for reliability rather than fuzzer-minimized: the 64-element decoy setup, the exactly-32 live locals, the polymorphism-priming loop order, and the try/catch around the getter all look deliberately constructed. A fuzzer emitting resizable ArrayBuffers plus transfer() inside hot polymorphic property loads could plausibly have produced the initial crash that motivated the audit.

This vulnerability weakens the memory-safety boundary inside the WebContent process by breaking a core JIT invariant. Before the fix, a detached-buffer side exit left the compiled function running on a stack pointer raised past live frame data, with registers that JSC's register allocator believes hold live JSValues instead holding words read out of the caller's own live frame region. An attacker who could arrange chosen 64-bit values in the frame slots the bogus fills read from would substitute an attacker-influenced bit pattern for a live JS variable — the classic route to a fake-object/type-confusion primitive and, from there, arbitrary read/write in the renderer.

Insight

JumpList is stack-height-agnostic by construction — a branch records only its origin in the instruction stream, not the stack depth in effect when it was created. That makes "one list, two epochs" an easy mistake to make and an invisible one to review, because the two append sites can be dozens of lines apart with a preserveReusedRegistersByPushing between them. Note also the direction of the damage: because the spill slots are addressed at positive offsets from the lowered stack pointer, skipping the subPtr does not read abandoned memory below the stack — it reads the caller's own live frame, the more attacker-adjacent region of the two. The fix's real content is a naming convention (postPushFailAndIgnore) that encodes the epoch in the variable name; a stronger mitigation would be for ScratchRegisterAllocator::PreservedState to carry a debug-mode stack-height token that restoreReusedRegistersByPopping could assert against at each linked predecessor. Resizable/growable ArrayBuffer support is what forced runtime length computation into these stubs at all — the whole preserve/restore block exists only on that path, so the resizable-buffer feature is what created the epoch split in this function.

Audit directions

• A jump created at one stack depth linked to a label whose code assumes a different depth. The invariant is every branch merged into a single link target must have originated at the same stack height, and it is hard to hold because assembler JumpLists carry no depth information and the two append sites can be far apart. Narrow: grep Source/JavaScriptCore for preserveReusedRegistersByPushing and, for each, check whether any JumpList linked after the matching restoreReusedRegistersByPopping also receives append(...) calls textually above the push — InlineCacheCompiler.cpp's other scratch-allocator blocks and DFGSpeculativeJIT/FTLLower uses of ScratchRegisterAllocator are the immediate targets. Wider: the same class appears with any other stack-adjusting primitive paired with a shared exit label — pushToSave/popToRestore, ScratchBuffer save/restore sequences, manual subPtr/addPtr on the stack pointer around snippet emission, and OSR-exit JumpLists that cross a frame-shuffle. Match tell on the narrow rung: a JumpList local declared before a push and link()ed after the corresponding pop. On the wider rung: any label whose code begins with a stack-pointer-restoring sequence and has predecessors emitted before the matching adjustment. Widest: this is the general "control-flow edge must agree with the abstract stack/frame state at its target" invariant, holding in every codegen system that tracks stack height per basic block — V8's Liftoff and SpiderMonkey's Wasm baseline compiler both attach a stack height to branch targets, and LLVM's stackmap/CFI machinery encodes the same constraint. Carry over: if a branch and its target disagree about how many bytes are live below the stack pointer, the bug is silent until the rare path is taken.

• Rare-exit branches of JIT fast paths guarded by conditions foreign JS API calls can flip after stub compilation. Detachment, resizing, and shrinking of ArrayBuffers are the canonical flippers here. These exits are cold, so they are rarely exercised by ordinary tests and by most fuzzing corpora, which is exactly why an incorrect epilogue can survive review. Start with the callers of loadDataViewByteLength and loadTypedArrayByteLength and every use of isResizableOrGrowableSharedTypedArrayIncludingDataView and forResizableTypedArray in InlineCacheCompiler.cpp, then widen to the IndexedResizableTypedArray*Load/Store/In access cases enumerated in toTypedArrayType. Match tell: a guard branch whose destination is shared with a guard emitted at a different point in the stub, or an exit that skips a cleanup the success path performs.

• Investigate whether push/pop balance can be checked mechanically rather than by naming convention. ScratchRegisterAllocator::PreservedState already carries numberOfBytesPreserved and extraStackSpaceRequirement, so a debug-only monotonically increasing epoch counter stored in the allocator and stamped onto each Jump at creation would let restoreReusedRegistersByPopping assert that every predecessor of its label shares the epoch. Trace how many existing stubs would need annotating by counting ScratchRegisterAllocator instantiations across Source/JavaScriptCore/bytecode and Source/JavaScriptCore/jit. Verification here is nontrivial: it requires building with the assertion and running the full JSTests/stress suite with high register pressure and polymorphic ICs, since the mismatch is only observable on paths taken rarely.

• Register pressure as a general amplifier for JIT bugs. didReuseRegisters() gates whether the buggy prologue/epilogue is emitted at all, so a stub can be correct at low pressure and corrupt at high pressure. Grep for didReuseRegisters() and numberOfReusedRegisters() and check each guarded block for paths that bypass one half of the guarded pair. Match tell: an if (allocator.didReuseRegisters() && ...) whose else arm routes jumps that were emitted on the other side of the push. Widest: any optimization whose emitted code shape depends on a resource-pressure predicate needs its rare-exit paths tested under both settings of that predicate — this applies to register allocators in any compiler (V8 Turbofan spill slots, SpiderMonkey Ion's LStackSlot handling) as much as to JSC. Carry over: which paths exist only when spilling happens, and has anything ever exercised them?