Crash [@ js::jit::Simulator::decodeType01] or Assertion failure: Invalid caller frame type when exiting from Ion frame., at jit/MacroAssembler.cpp

RESOLVED FIXED in Firefox 42

Status

()

defect
--
critical
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: gkw, Assigned: sstangl)

Tracking

(Blocks 2 bugs, 5 keywords)

Trunk
mozilla43
x86_64
macOS
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox41 unaffected, firefox42 fixed, firefox43 fixed, firefox-esr38 unaffected, b2g-v2.0 unaffected, b2g-v2.0M unaffected, b2g-v2.1 unaffected, b2g-v2.1S unaffected, b2g-v2.2 unaffected, b2g-v2.2r unaffected, b2g-master fixed)

Details

(Whiteboard: [jsbugmon:update][post-critsmash-triage][b2g-adv-main2.5-], crash signature)

Attachments

(3 attachments)

Reporter

Description

4 years ago
// Randomly chosen test: jit-test/tests/saved-stacks/bug-1012646-strlen-crasher.js
function f(x) {
    try {
        eval(x);
    } catch (e) {}
};
f("enableSPSProfilingWithSlowAssertions();");
f("enableTrackAllocations(); throw Error();");

asserts js debug ARM-simulator shell on m-c changeset 9673c75864be with --fuzzing-safe --no-threads --ion-eager at Assertion failure: Invalid caller frame type when exiting from Ion frame., at jit/MacroAssembler.cpp and crashes js opt ARM-simulator builds at js::jit::Simulator::decodeType01

Debug configure options:

LD=ld CROSS_COMPILE=1 CC="clang -Qunused-arguments -msse2 -mfpmath=sse -arch i386" RANLIB=ranlib CXX="clang++ -Qunused-arguments -msse2 -mfpmath=sse -arch i386" AS=$CC AR=ar STRIP="strip -x -S" HOST_CC="clang -Qunused-arguments -msse2 -mfpmath=sse" AUTOCONF=/usr/local/Cellar/autoconf213/2.13/bin/autoconf213 HOST_CXX="clang++ -Qunused-arguments -msse2 -mfpmath=sse" sh /Users/skywalker/trees/mozilla-central/js/src/configure --target=i386-apple-darwin9.2.0 --enable-macos-target=10.5 --enable-arm-simulator --enable-simulator=arm --enable-debug --enable-nspr-build --enable-more-deterministic --with-ccache --enable-gczeal --enable-debug-symbols --disable-tests

python -u ~/funfuzz/js/compileShell.py -b "--enable-debug --enable-more-deterministic --enable-nspr-build --32 --enable-simulator=arm" -r 9673c75864be

Opt configure options:

LD=ld CROSS_COMPILE=1 CC="clang -Qunused-arguments -msse2 -mfpmath=sse -arch i386" RANLIB=ranlib CXX="clang++ -Qunused-arguments -msse2 -mfpmath=sse -arch i386" AS=$CC AR=ar STRIP="strip -x -S" HOST_CC="clang -Qunused-arguments -msse2 -mfpmath=sse" AUTOCONF=/usr/local/Cellar/autoconf213/2.13/bin/autoconf213 HOST_CXX="clang++ -Qunused-arguments -msse2 -mfpmath=sse" sh /Users/skywalker/trees/mozilla-central/js/src/configure --target=i386-apple-darwin9.2.0 --enable-macos-target=10.5 --enable-arm-simulator --enable-simulator=arm --disable-debug --enable-nspr-build --enable-more-deterministic --with-ccache --enable-gczeal --enable-debug-symbols --disable-tests

python -u ~/funfuzz/js/compileShell.py -b "--disable-debug --enable-more-deterministic --enable-nspr-build --32 --enable-simulator=arm" -r 9673c75864be

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/2729e432104c
user:        Nick Fitzgerald
date:        Tue Jul 28 13:04:56 2015 -0700
summary:     Bug 1028418 - Part 5: Minimize stack walking when capturing SavedFrame stacks with a cache; r=shu

Nick, is bug 1028418 a likely regressor?

This also manifests in native ARM builds, as SIGTRAP crashes.
Flags: needinfo?(nfitzgerald)
Reporter

Comment 1

4 years ago
Posted file opt stack
(lldb) bt
* thread #1: tid = 0xbcb0d, 0x0047cdcc js-32-dm-nsprBuild-armSim-darwin-9673c75864be`js::jit::Simulator::decodeType01(this=<unavailable>, instr=<unavailable>) + 4028 at Simulator-arm.cpp:2735, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
  * frame #0: 0x0047cdcc js-32-dm-nsprBuild-armSim-darwin-9673c75864be`js::jit::Simulator::decodeType01(this=<unavailable>, instr=<unavailable>) + 4028 at Simulator-arm.cpp:2735
    frame #1: 0x00000036
(lldb)
Reporter

Comment 2

4 years ago
Posted file debug stack
(lldb) bt 5
* thread #1: tid = 0xbd141, 0x00793bbc js-dbg-32-dm-nsprBuild-armSim-darwin-9673c75864be`js::jit::Simulator::decodeType01(this=<unavailable>, instr=<unavailable>) + 5964 at Simulator-arm.cpp:2735, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
  * frame #0: 0x00793bbc js-dbg-32-dm-nsprBuild-armSim-darwin-9673c75864be`js::jit::Simulator::decodeType01(this=<unavailable>, instr=<unavailable>) + 5964 at Simulator-arm.cpp:2735
    frame #1: 0x007904f2 js-dbg-32-dm-nsprBuild-armSim-darwin-9673c75864be`js::jit::Simulator::instructionDecode(this=0x0079247e, instr=0x03b000f0) + 802 at Simulator-arm.cpp:4171
    frame #2: 0x008237b6 js-dbg-32-dm-nsprBuild-armSim-darwin-9673c75864be`void js::jit::Simulator::execute<false>(this=0x01e93000) + 134 at Simulator-arm.cpp:4244
    frame #3: 0x00798973 js-dbg-32-dm-nsprBuild-armSim-darwin-9673c75864be`js::jit::Simulator::callInternal(this=0x01e93000, entry=0x03b00940) + 227 at Simulator-arm.cpp:4332
    frame #4: 0x00799091 js-dbg-32-dm-nsprBuild-armSim-darwin-9673c75864be`js::jit::Simulator::call(this=0x01e93000, entry=<unavailable>, argument_count=<unavailable>) + 161 at Simulator-arm.cpp:4415
(lldb)
It seems like maybe my patch may have caused some bad interactions with the JIT, but the JIT is largely outside my area of expertise, unfortunately.

Shu, do you have any idea what might be going on here?
Flags: needinfo?(nfitzgerald) → needinfo?(shu)

Comment 4

4 years ago
Sorry, this isn't something I have expertise to debug.
Flags: needinfo?(shu)
Reporter

Comment 5

4 years ago
If it's SPS profiler, it might be Kannan's world.

Also, I'm setting s-s because it seems it's not just involving SavedFrame stack stuff not, as per the potential regressor in comment 0 (which might not have been correct either).
Group: core-security, javascript-core-security
Flags: needinfo?(kvijayan)
Reporter

Comment 6

4 years ago
(In reply to Gary Kwong [:gkw] [:nth10sd] from comment #5)
> involving SavedFrame stack stuff not

-not

Comment 7

4 years ago
Well, it looks like SPS toggling causing malformed ARM instructions, probably during toggling stuff in Baseline code. Should get someone who knows our ARM codegen to look.
Reporter

Comment 8

4 years ago
Not sure if we have an available ARM JS folk now, so needinfo? from Jan as well as a fallback.
Flags: needinfo?(jdemooij)
(In reply to Gary Kwong [:gkw] [:nth10sd] from comment #8)
> Not sure if we have an available ARM JS folk now, so needinfo? from Jan as
> well as a fallback.

NI Sean, though this does look like a profiler issue.
Flags: needinfo?(jdemooij) → needinfo?(sstangl)
Assignee

Comment 10

4 years ago
The FrameType mask in ARM profiler code included one extra bit erroneously. This patch used that bit as the "saved frame bit", which caused the read FrameType to be gibberish, from which point we ran off into impossible code.
Flags: needinfo?(sstangl)
Flags: needinfo?(kvijayan)
Attachment #8651902 - Flags: review?(kvijayan)
Attachment #8651902 - Flags: review?(kvijayan) → review+
Assignee

Updated

4 years ago
Assignee: nobody → sstangl
Assignee

Comment 11

4 years ago
OK to include test code in commit?

Bug is long-standing, but probably requires Bug 1028418 to expose bad behavior.
Flags: needinfo?(gary)
Reporter

Comment 12

4 years ago
I'd defer to sec-approval folks for this - setting needinfo? from Al.
Flags: needinfo?(gary) → needinfo?(abillings)
Don't ni? me. Ask for sec-approval? on a patch and fill out the form. :-)

That said, this needs a security rating before asking for sec-approval.
Flags: needinfo?(abillings)
Assignee

Comment 14

4 years ago
Comment on attachment 8651902 [details] [diff] [review]
0001-Fix-treating-saved-frame-bit-as-part-of-the-frame-ty.patch

[Security approval request comment]
>How easily could an exploit be constructed based on the patch?

Very easily, but although the problem existed in-tree for a long time, it probably requires Bug 1028418 to make the bad behavior observable. Without the patchset in that bug, the extra bit that the instruction reads should always be zero, which we assumed it would be.

>Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?

Yes, but again it probably is not a security problem on non-

>Which older supported branches are affected by this flaw?

All contain the bug, just on the ARM platform. But they don't contain the code for Bug 1028418, so they probably don't need to be patched.

>Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?

Yeah, it's the same one-line patch.

>How likely is this patch to cause regressions; how much testing does it need?

Extremely unlikely; none.
Attachment #8651902 - Flags: sec-approval?
Calling this a sec-moderate. This means you don't need sec-approval+ and can just check in.
Keywords: sec-moderate
Attachment #8651902 - Flags: sec-approval?
https://hg.mozilla.org/mozilla-central/rev/3ab040e254a0

Please nominate this for Aurora approval when you get a chance.
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Flags: needinfo?(sstangl)
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
Group: javascript-core-security

Updated

4 years ago
Group: core-security → core-security-release
Assignee

Comment 18

4 years ago
Comment on attachment 8651902 [details] [diff] [review]
0001-Fix-treating-saved-frame-bit-as-part-of-the-frame-ty.patch

Review of attachment 8651902 [details] [diff] [review]:
-----------------------------------------------------------------

Nominating for Aurora per Comment 17.
Attachment #8651902 - Flags: approval-mozilla-aurora?
Assignee

Updated

4 years ago
Flags: needinfo?(sstangl)
Comment on attachment 8651902 [details] [diff] [review]
0001-Fix-treating-saved-frame-bit-as-part-of-the-frame-ty.patch

Next time, please fill the form even if a sheriff asked you too. We still need the information. Thanks!
Attachment #8651902 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Assignee

Comment 20

4 years ago
https://hg.mozilla.org/releases/mozilla-aurora/rev/c3fec00fc2bd

The form did not appear in the textarea when I selected the flag.
Reporter

Updated

4 years ago
Blocks: 1100132
Whiteboard: [jsbugmon:update] → [jsbugmon:update][post-critsmash-triage]
Group: core-security-release
Whiteboard: [jsbugmon:update][post-critsmash-triage] → [jsbugmon:update][post-critsmash-triage][b2g-adv-main2.5-]
You need to log in before you can comment on or make changes to this bug.