The following testcase crashes on mozilla-central revision 67adec79eb8a (build with --enable-optimize --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-debug, run with --fuzzing-safe --thread-count=2 --ion-shared-stubs=on): (No proper testcase available, filing on behalf of h4writer). Backtrace: Program terminated with signal SIGSEGV, Segmentation fault. #0 0x00000000009b2c0a in js::jit::LazyLink (cx=cx@entry=0x7fce97806c00, calleeScript=..., calleeScript@entry=...) at js/src/jit/Ion.cpp:619 #1 0x00000000009b46f1 in js::jit::CanEnter (cx=cx@entry=0x7fce97806c00, state=...) at js/src/jit/Ion.cpp:2573 #2 0x0000000000705f1d in js::RunScript (cx=cx@entry=0x7fce97806c00, state=...) at js/src/vm/Interpreter.cpp:685 #3 0x000000000070674f in js::Invoke (cx=cx@entry=0x7fce97806c00, args=..., construct=construct@entry=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:786 #4 0x00000000007072bd in js::Invoke (cx=cx@entry=0x7fce97806c00, thisv=..., fval=..., argc=argc@entry=1, argv=argv@entry=0x7fff0cdc7770, rval=..., rval@entry=...) at js/src/vm/Interpreter.cpp:823 #5 0x00000000008d687b in js::jit::DoCallFallback (cx=0x7fce97806c00, frame=0x7fff0cdc77c8, stub_=<optimized out>, argc=<optimized out>, vp=0x7fff0cdc7760, res=...) at js/src/jit/BaselineIC.cpp:8905 #6 0x00007fce98d87f9f in ?? () #33 0x0000000000000000 in ?? () rax 0x0 0 rbx 0x7fce97806c00 140525281766400 rcx 0x7fce97bd688d 140525285763213 rdx 0x0 0 rsi 0x7fce97eab9d0 140525288733136 rdi 0x7fce97eaa1c0 140525288726976 rbp 0x7fff0cdc6f80 140733409161088 rsp 0x7fff0cdc6d90 140733409160592 r8 0x7fce98f1b780 140525305968512 r9 0x6372732f736a2f6c 7165916604736876396 r10 0x7fce97ea7be0 140525288717280 r11 0x0 0 r12 0x7fce95920258 140525249364568 r13 0x7fff0cdc6ed0 140733409160912 r14 0x7fff0cdc6e20 140733409160736 r15 0x7fff0cdc6e38 140733409160760 rip 0x9b2c0a <js::jit::LazyLink(JSContext*, JS::Handle<JSScript*>)+1514> => 0x9b2c0a <js::jit::LazyLink(JSContext*, JS::Handle<JSScript*>)+1514>: movl $0x26b,0x0 0x9b2c15 <js::jit::LazyLink(JSContext*, JS::Handle<JSScript*>)+1525>: callq 0x4979a0 <abort()>
Created attachment 8670754 [details] [diff] [review] Patch Could repro using testcase: https://fuzzmanager.fuzzing.mozilla.org/crashmanager/crashes/106821/ which isn't reduced. Rate of segmentation fault on this testcase has reduced from: 1 out of 4 to 0 out of 25. Forgot to mark baselinescript as active if it is on the stack. As a result the script was getting gc'ed, while we are expecting that we atleast still have the baselinescript after lazy linking.
Assignee: nobody → hv1989
Attachment #8670754 - Flags: review?(jdemooij)
FF35: Bug 1047346: first enabled lazy linking. But only in extreme cases. Very hard to have it (only 0.01% uses it). Even harder to trigger this (which on its own is very hard) FF43: Bug 1178834: enabled lazy linking for everything. Still hard to reproduce, though. Because of this I'm gonna land this on FF43+ only.
status-firefox41: --- → affected
status-firefox42: --- → affected
status-firefox43: --- → affected
Comment on attachment 8670754 [details] [diff] [review] Patch Review of attachment 8670754 [details] [diff] [review]: ----------------------------------------------------------------- Good find.
Attachment #8670754 - Flags: review?(jdemooij) → review+
status-b2g-v2.0: --- → unaffected
status-b2g-v2.0M: --- → unaffected
status-b2g-v2.1: --- → unaffected
status-b2g-v2.1S: --- → unaffected
status-b2g-v2.2: --- → wontfix
status-b2g-v2.2r: --- → wontfix
status-b2g-master: --- → affected
status-firefox41: affected → wontfix
status-firefox42: affected → wontfix
status-firefox-esr38: --- → wontfix
Comment on attachment 8670754 [details] [diff] [review] Patch [Security approval request comment] How easily could an exploit be constructed based on the patch? I think it is quite hard to construct an exploit, even knowing the issue. Even harder when not understanding the issue and only looking at the patch. It depends on a lot of timings going right. Also the patch doesn't make it obvious the debugger hooks need to be enabled, before it is possible to exploit this. 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? FF35+, but on a very minor scale. This feature was used very little back then. FF43+, this is now enabled by default. Still hard to reproduce. But intend is to land on FF43+ because of this. If not all supported branches, which bug introduced the flaw? see above. Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? Should apply on FF43 cleanly How likely is this patch to cause regressions; how much testing does it need? Quite safe.
Attachment #8670754 - Flags: sec-approval?
Is this disabled by default then on 41 and 42? sec-approval+. We'll want it on Aurora too.
Attachment #8670754 - Flags: sec-approval? → sec-approval+
(In reply to Al Billings [:abillings] from comment #5) > Is this disabled by default then on 41 and 42? No. There is a small path that allows this in FF41 and FF42, but very very hard to trigger. (Nexto this bug being hard to trigger in itself). As a result I imagine we can see this path as good as not happening in FF41/FF42.
Comment on attachment 8670754 [details] [diff] [review] Patch Approval Request Comment [Feature/regressing bug #]: See bug 1212305 comment 2. Originally bug 1047346, but only bug 1178834 made it wide spread enough to maybe become exploitable? [User impact if declined]: Unsafe crashes and exploitable. [Describe test coverage new/current, TreeHerder]: I haven't pushed to nightly yet (Should I wait before aurora approval? So we can land this security on all branches at the same time.). It passes all our shell testcases and the testcase that gave us this issue. [Risks and why]: Doesn't adjust any code on normal code paths. Only on code paths that would crash currently. I think that this code is definitely better than crashing. Very very worst case scenario this code would introduce a new crash. Which still sounds better than a definite crash. Though I (and the reviewer) think this fully solves the issue ;) [String/UUID change made/needed]: /
Attachment #8670754 - Flags: approval-mozilla-aurora?
Attachment #8670754 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox44: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
status-firefox43: affected → fixed
Whiteboard: [jsbugmon:ignore] → [jsbugmon:ignore][b2g-adv-main2.5+]
Whiteboard: [jsbugmon:ignore][b2g-adv-main2.5+] → [jsbugmon:ignore][b2g-adv-main2.5+][adv-main43+]
Whiteboard: [jsbugmon:ignore][b2g-adv-main2.5+][adv-main43+] → [jsbugmon:ignore][b2g-adv-main2.5+][adv-main43+][post-critsmash-triage]
You need to log in before you can comment on or make changes to this bug.