The following testcase crashes on mozilla-central revision e78975b53563 (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-debug --enable-optimize, run with --fuzzing-safe --thread-count=2): See attachment. Backtrace: received signal SIGSEGV, Segmentation fault. js::jit::JitFrameIterator::script (this=0x7fffffffad10) at js/src/jit/JitFrames.cpp:213 #0 js::jit::JitFrameIterator::script (this=0x7fffffffad10) at js/src/jit/JitFrames.cpp:213 #1 0x00000000006b4a92 in js::jit::JitFrameIterator::checkInvalidation (this=this@entry=0x7fffffffad10, ionScriptOut=ionScriptOut@entry=0x7fffffffa5c0) at js/src/jit/JitFrames.cpp:149 #2 0x00000000006b626b in js::jit::JitFrameIterator::ionScript (this=0x7fffffffad10) at js/src/jit/JitFrames.cpp:2208 #3 0x00000000006b68c1 in js::jit::JitFrameIterator::safepoint (this=0x7fffffffad10) at js/src/jit/JitFrames.cpp:2226 #4 0x00000000006b6928 in js::jit::JitFrameIterator::machineState (this=this@entry=0x7fffffffad10) at js/src/jit/JitFrames.cpp:297 #5 0x00000000006bc1e4 in js::jit::InlineFrameIterator::resetOn (this=0x7fffffffae40, iter=0x7fffffffad10) at js/src/jit/JitFrames.cpp:2290 #6 0x00000000006c96a3 in js::jit::HandleException (rfe=<optimized out>) at js/src/jit/JitFrames.cpp:784 #7 0x00007ffff7fe6646 in ?? () [...] #12 0x0000000000000000 in ?? () rax 0x807ffeffb240 141287227372096 rbx 0x7fffffffad10 140737488334096 rcx 0x30 48 rdx 0x7fffffffb1d0 140737488335312 rsi 0x7fffffffa5c0 140737488332224 rdi 0x7fffffffad10 140737488334096 rbp 0x7fffffffa590 140737488332176 rsp 0x7fffffffa590 140737488332176 r8 0x30 48 r9 0x7ffff24fffa0 140737258717088 r10 0x2 2 r11 0x7ffff7e9bdf8 140737352678904 r12 0x7fffffffa5c0 140737488332224 r13 0x7fffffffa8c0 140737488332992 r14 0x7fffffffad10 140737488334096 r15 0x7fffffffad10 140737488334096 rip 0x6b49f9 <js::jit::JitFrameIterator::script() const+25> => 0x6b49f9 <js::jit::JitFrameIterator::script() const+25>: mov 0x10(%rax),%rdi 0x6b49fd <js::jit::JitFrameIterator::script() const+29>: callq 0x6b2030 <js::jit::ScriptFromCalleeToken(js::jit::CalleeToken)> The testcase here is intermittent. Sometimes it crashes after only 1-2 seconds, sometimes it runs for 30 seconds before crashing. The memory access looks sec-critical, marking as such.
Naveed can you help find someone to work on this sec-critical bug?
David can you help out here ? Maybe you can help find an owner to investigate this since it is sec-critical and may affect beta and other versions.
Jandem, any thoughts?
Flags: needinfo?(dbolter) → needinfo?(jdemooij)
I'll take a look tomorrow.
Oops, regression from bug 1272598. We shouldn't jump to masm.exceptionLabel() when ArgumentsObject::finishForIon fails, because we didn't enter an exit frame. This patch changes the code to just fall back to the slow path.
Assignee: nobody → jdemooij
Status: NEW → ASSIGNED
Attachment #8783926 - Flags: review?(nicolas.b.pierron)
Attachment #8783926 - Flags: review?(nicolas.b.pierron) → review+
Jan, can we land this? Is this blocked on something to request sec-approval and uplift?
It only affects 50 and 51. If someone asked for sec-approval+ to land on trunk and Aurora approval, I'd give it.
Comment on attachment 8783926 [details] [diff] [review] Patch [Security approval request comment] > How easily could an exploit be constructed based on the patch? Not trivial but not that difficult... > Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem? No. > Which older supported branches are affected by this flaw? Aurora, beta (50+). > If not all supported branches, which bug introduced the flaw? Bug 1272598. > Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? Should apply or be easy. > How likely is this patch to cause regressions; how much testing does it need? Unlikely.
Attachment #8783926 - Flags: sec-approval?
(In reply to Jan de Mooij [:jandem] from comment #10) > > Which older supported branches are affected by this flaw? > Aurora, beta (50+). Only aurora, actually, since the merge was moved to next week.
Comment on attachment 8783926 [details] [diff] [review] Patch sec-approval+ for trunk. Please request Aurora approval now so we can get this onto 50 before it becomes Beta as well.
Attachment #8783926 - Flags: sec-approval? → sec-approval+
Comment on attachment 8783926 [details] [diff] [review] Patch Approval Request Comment [Feature/regressing bug #]: Bug 1272598. [User impact if declined]: Crashes on OOM, potential security bugs. [Describe test coverage new/current, TreeHerder]: Fixes the fuzz test. [Risks and why]: Low risk. Only affects an OOM case and hitting that here should be unlikely. [String/UUID change made/needed]: None.
Comment on attachment 8783926 [details] [diff] [review] Patch Sec-crit issue, Aurora51+, Beta50+
Whiteboard: [jsbugmon:ignore] → [jsbugmon:ignore][post-critsmash-triage]
You need to log in before you can comment on or make changes to this bug.