Closed Bug 2045309 Opened 5 months ago Closed 5 months ago

Assertion failure: JSOp(*pc) == JSOp::GetElem, at js/src/jit/BaselineIC.cpp:733 (ICScript confusion in BaselineStackBuilder::setNextCallee)

Categories

(Core :: JavaScript Engine: JIT, defect, P1)

x86_64
Linux
defect

Tracking

()

RESOLVED FIXED
153 Branch
Tracking Status
firefox-esr115 --- unaffected
firefox-esr140 --- unaffected
firefox151 --- unaffected
firefox152 + fixed
firefox153 + fixed

People

(Reporter: decoder, Assigned: jandem)

References

(Blocks 1 open bug, Regression)

Details

(4 keywords)

Attachments

(3 files, 2 external files)

Summary

When Ion bails out inside an inlined call chain, BaselineStackBuilder::setNextCallee (js/src/jit/BaselineBailouts.cpp:506-537) selects the ICScript for each reconstructed inline frame from the runtime trial-inlining state of the call site's fallback stub:

if (trialInliningState == TrialInliningState::Inlined &&
    !iter_.ionScript()->purgedICScripts() && canUseTrialInlinedICScripts_) {
  const uint32_t pcOff = script_->pcToOffset(pc_);
  icScript_ = icScript_->findInlinedChild(pcOff);
}

This is only sound if fallback-stub state TrialInliningState::Inlined guarantees Warp compiled that call site from exactly the trial-inlined child ICScript that findInlinedChild(pcOff) returns. Bug 2034632 (b63e150126d7, Apr 2026, "Fall back to monomorphic inlining if a trial-inlined ICScript hasn't been used", js/src/jit/WarpOracle.cpp) broke that invariant: when the child ICScript's warm-up count never advanced past trialInliningInitialWarmUpCount, Warp silently downgrades to monomorphic inlining against the callee's generic ICScript, while the fallback stub keeps state Inlined and the unused child ICScript stays registered. The compile-time inlining graph and the bailout-time ICScript walk can then diverge.

Root cause / attack

  1. Trial-inline callee c at call site S in f so the child ICScript C is created, then never execute S again before f is Ion-compiled. Warp downgrades: it inlines c via c's generic ICScript G, and inside c inlines g at the inner call site pc2. S's fallback stub remains Inlined.
  2. After compilation, force one bailout early in Ion-f; the rest of that invocation runs in baseline, where S's CallInlinedFunction stub enters c with icScript = C. Train C@pc2 with a different callee h and let the trial inliner run on C. This creates child ICScript C2 at C@pc2, trained for h's script, and sets C@pc2's state to Inlined. The IonScript of f stays valid.
  3. Re-enter Ion-f and bail out inside the inlined g. The bailout reconstructs frames [f, c, g]: at S it walks to C; at pc2, C's fallback stub says Inlined, so icScript_ = C.findInlinedChild(pc2) = C2, while nextCallee from the snapshot is g. The rebuilt baseline frame for g gets an ICScript trained for h.

The only mitigations are the two release asserts at js/src/jit/BaselineBailouts.cpp:535-536 (numICEntries / bytecodeSize equality). These pass for two different scripts with equal bytecode length and IC entry count — in the PoC, g and h are crafted to be 47 bytecode bytes with 7 IC entries each, but with different IC op kinds and pc offsets (GetProp/Add vs GetElem/Sub), so the size checks are attacker-bypassable.

After the bailout, g resumes in the baseline interpreter with C2 installed. Every IC lookup is misaligned: g's ops execute IC entries whose fallback stubs belong to other op kinds and whose pcOffset values point into the middle of unrelated instructions of g's bytecode — IC fallbacks run with the wrong op, the wrong pc and partially-uninitialized operand registers: script/ICScript type confusion. This is the bug class the release asserts were added to mitigate (Bug 1819722 follow-ups); Bug 2034632 made it reachable again.

Reproduction

Testcase: poc.js (attached). Run:

js --fuzzing-safe --no-threads --fast-warmup poc.js

The flags only speed up warmup and make tier-up deterministic; the trialInline()/bailout() shell hooks are deterministic stand-ins for warmup-count-triggered trial inlining and a cold-branch bailout. All involved machinery (Warp, trial inlining, bailouts) runs identically in Firefox content processes.

Tested on mozilla-central rev 5cffec5992d3 (shell product_version 20260603-5cffec5992d3, Linux x86-64).

Crash signature

Debug ASan build (m-c-asan-debug; full log poc_crash_debug.log):

Assertion failure: JSOp(*pc) == JSOp::GetElem, at js/src/jit/BaselineIC.cpp:733
    #1 js::jit::DoGetElemFallback(...) js/src/jit/BaselineIC.cpp:733
    ...
    #13 js::jit::DoCallFallback(...)
    #19 js::jit::EnterBaselineInterpreterAtBranch(...)

g's GetProp op invoked C2's GetElem fallback stub; the stub's pcOffset (valid only for h) points at byte 28 of g's bytecode, the middle of the GetProp atom-index operand.

Opt fuzzing ASan build (m-c-fuzzing-asan-opt; full log poc_crash_opt.log) continues past the compiled-out assert and dies one IC later decoding an operand byte as an opcode:

Hit MOZ_CRASH(Unhandled baseline arith op) at js/src/jit/BaselineIC.cpp:2384
    #1 js::jit::DoBinaryArithFallback(...)

That MOZ_CRASH is merely where this particular g/h layout first trips in a release build; the underlying state (wrong fallback kind, wrong pc, stale operand registers, IC stubs attached to wrong ops) is attacker-shaped. E.g. aligning a Call fallback (which trusts GET_ARGC(pc) and walks Values on the baseline stack) against a non-call op of the victim script yields controlled out-of-bounds stack reads and calls on arbitrary Value bits.

If g/h are not size-matched, the same testcase instead trips MOZ_RELEASE_ASSERT(icScript_->bytecodeSize() == calleeScript->length()) at BaselineBailouts.cpp:536, directly demonstrating that setNextCallee selects an ICScript belonging to a foreign script.

Suggested fix

Record in the Warp snapshot/IonScript whether each inlined site actually used the trial-inlined child ICScript (e.g. via InlineScriptTree's isMonomorphicInlined flag, which WarpOracle already computes), and have BaselineStackBuilder::setNextCallee consult that instead of (or in addition to) the runtime trial-inlining state. Alternatively, when Bug 2034632's downgrade triggers, remove the unused inlined child / flip the stub state so the compile-time and bailout-time views cannot diverge. The numICEntries/bytecodeSize release asserts should also be strengthened to compare the ICScript's associated script identity, not just its dimensions.

Files

  • poc.js — reproducer (run with --fuzzing-safe --no-threads --fast-warmup)
  • poc_crash_debug.log — full crash log, m-c-asan-debug (rev 5cffec5992d3)
  • poc_crash_opt.log — full crash log, m-c-fuzzing-asan-opt (rev 5cffec5992d3)
Keywords: regression
Regressed by: 2034632

Set release status flags based on info from the regressing bug 2034632

:iain, since you are the author of the regressor, bug 2034632, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Assignee: nobody → jdemooij
Status: NEW → ASSIGNED
Flags: needinfo?(iireland)
Attached file (secure) —
Attached file (secure) —

The patch here works but I've requested a backout of bug 2034632 because there's also bug 2036303 and although that one is not a security bug, it's kind of related and we should look at the big picture before shipping this.

Blocks: sm-security
Severity: -- → S2
Priority: -- → P1

Fixed by backout. I assume any necessary follow-up work related to this specific bug will happen before it re-lands.

Group: javascript-core-security → core-security-release
Status: ASSIGNED → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → 153 Branch
QA Whiteboard: [sec] [uplift] [qa-triage-done-c153/b152]
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: