Closed Bug 1355573 Opened 3 years ago Closed 3 years ago

Assertion failure: !runtime->activeThreadHasExclusiveAccess

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: anba, Assigned: jandem)

References

Details

Attachments

(1 file)

Test case:
---
function f(){};
Object.defineProperty(f, "name", {value: "a".repeat((1<<28)-1)});
print(f.bind().name.length);
print(("bound " + f.name).length);
---

Asserts with:
---
0x0000000000564dcc in js::AutoLockForExclusiveAccess::init (this=0x7fffffffa650, rt=0x7ffff696b000) at /home/andre/git/mozilla-central/js/src/jscntxt.h:1178
1178	            MOZ_ASSERT(!runtime->activeThreadHasExclusiveAccess);
---

Stacktrace:
---
#0  0x0000000000564dcc in js::AutoLockForExclusiveAccess::init(JSRuntime*) (this=0x7fffffffa650, rt=0x7ffff696b000) at /home/andre/git/mozilla-central/js/src/jscntxt.h:1178
#1  0x0000000000564e41 in js::AutoLockForExclusiveAccess::AutoLockForExclusiveAccess(JSContext*, mozilla::detail::GuardObjectNotifier&&) (this=0x7fffffffa650, cx=0x7ffff6975000, _notifier=<unknown type in /home/andre/git/mozilla-central/js/src/build-debug-master/js/src/shell/js, CU 0x60fc75, DIE 0x734ea8>) at /home/andre/git/mozilla-central/js/src/jscntxt.h:1188
#2  0x0000000000562191 in AtomizeAndCopyChars<unsigned char>(JSContext*, unsigned char const*, size_t, js::PinningBehavior) (cx=0x7ffff6975000, tbchars=0x7ffff69162a0 "/tmp/t.js", length=9, pin=js::DoNotPinAtom) at /home/andre/git/mozilla-central/js/src/jsatom.cpp:345
#3  0x00000000005629cf in js::Atomize(JSContext*, char const*, unsigned long, js::PinningBehavior) (cx=0x7ffff6975000, bytes=0x7ffff69162a0 "/tmp/t.js", length=9, pin=js::DoNotPinAtom)
    at /home/andre/git/mozilla-central/js/src/jsatom.cpp:442
#4  0x0000000000e67216 in js::SavedStacks::getLocation(JSContext*, js::FrameIter const&, JS::MutableHandle<js::SavedStacks::LocationValue>) (this=0x7ffff694e8f8, cx=0x7ffff6975000, iter=..., locationp=
  {source = 0x0, line = 0, column = 0}) at /home/andre/git/mozilla-central/js/src/vm/SavedStacks.cpp:1574
#5  0x0000000000e6603c in js::SavedStacks::insertFrames(JSContext*, js::FrameIter&, JS::MutableHandle<js::SavedFrame*>, mozilla::Variant<JS::AllFrames, JS::MaxFrames, JS::FirstSubsumedFrame>&&) (this=0x7ffff694e8f8, cx=0x7ffff6975000, iter=..., frame=0x0, capture=<unknown type in /home/andre/git/mozilla-central/js/src/build-debug-master/js/src/shell/js, CU 0x4170a9e, DIE 0x430f982>)
    at /home/andre/git/mozilla-central/js/src/vm/SavedStacks.cpp:1342
#6  0x0000000000e65714 in js::SavedStacks::saveCurrentStack(JSContext*, JS::MutableHandle<js::SavedFrame*>, mozilla::Variant<JS::AllFrames, JS::MaxFrames, JS::FirstSubsumedFrame>&&) (this=0x7ffff694e8f8, cx=0x7ffff6975000, frame=0x0, capture=<unknown type in /home/andre/git/mozilla-central/js/src/build-debug-master/js/src/shell/js, CU 0x4170a9e, DIE 0x430f24d>)
    at /home/andre/git/mozilla-central/js/src/vm/SavedStacks.cpp:1177
#7  0x0000000000b19d24 in JS::CaptureCurrentStack(JSContext*, JS::MutableHandle<JSObject*>, mozilla::Variant<JS::AllFrames, JS::MaxFrames, JS::FirstSubsumedFrame>&&) (cx=0x7ffff6975000, stackp=0x0, capture=<unknown type in /home/andre/git/mozilla-central/js/src/build-debug-master/js/src/shell/js, CU 0x2c230e0, DIE 0x2dcbf18>) at /home/andre/git/mozilla-central/js/src/jsapi.cpp:7084
#8  0x0000000000b2ac8b in CaptureStack(JSContext*, JS::MutableHandleObject) (cx=0x7ffff6975000, stack=0x0) at /home/andre/git/mozilla-central/js/src/jsexn.cpp:362
#9  0x0000000000b2c911 in js::ErrorToException(JSContext*, JSErrorReport*, JSErrorFormatString const* (*)(void*, unsigned int), void*) (cx=0x7ffff6975000, reportp=0x7fffffffb640, callback=0xb1d61d <js::GetErrorMessage(void*, unsigned int)>, userRef=0x0) at /home/andre/git/mozilla-central/js/src/jsexn.cpp:680
#10 0x0000000000b1b30e in ReportError(JSContext*, JSErrorReport*, JSErrorCallback, void*) (cx=0x7ffff6975000, reportp=0x7fffffffb640, callback=0xb1d61d <js::GetErrorMessage(void*, unsigned int)>, userRef=0x0) at /home/andre/git/mozilla-central/js/src/jscntxt.cpp:291
#11 0x0000000000b1c3e5 in js::ReportErrorNumberVA(JSContext*, unsigned int, JSErrorFormatString const* (*)(void*, unsigned int), void*, unsigned int, js::ErrorArgumentsType, __va_list_tag*) (cx=0x7ffff6975000, flags=0, callback=0xb1d61d <js::GetErrorMessage(void*, unsigned int)>, userRef=0x0, errorNumber=108, argumentsType=js::ArgumentsAreASCII, ap=0x7fffffffb720)
    at /home/andre/git/mozilla-central/js/src/jscntxt.cpp:890
#12 0x0000000000b156a8 in JS_ReportErrorNumberASCIIVA(JSContext*, JSErrorFormatString const* (*)(void*, unsigned int), void*, unsigned int, __va_list_tag*) (cx=0x7ffff6975000, errorCallback=0xb1d61d <js::GetErrorMessage(void*, unsigned int)>, userRef=0x0, errorNumber=108, ap=0x7fffffffb720) at /home/andre/git/mozilla-central/js/src/jsapi.cpp:5776
#13 0x0000000000b15642 in JS_ReportErrorNumberASCII(JSContext*, JSErrorFormatString const* (*)(void*, unsigned int), void*, unsigned int, ...) (cx=0x7ffff6975000, errorCallback=0xb1d61d <js::GetErrorMessage(void*, unsigned int)>, userRef=0x0, errorNumber=108) at /home/andre/git/mozilla-central/js/src/jsapi.cpp:5765
#14 0x0000000000b1b6da in js::ReportAllocationOverflow(JSContext*) (cx=0x7ffff6975000) at /home/andre/git/mozilla-central/js/src/jscntxt.cpp:403
#15 0x0000000000565523 in JSString::validateLength(JSContext*, unsigned long) (maybecx=0x7ffff6975000, length=268435461) at /home/andre/git/mozilla-central/js/src/vm/String-inl.h:100
#16 0x0000000000562318 in AtomizeAndCopyChars<unsigned char>(JSContext*, unsigned char const*, size_t, js::PinningBehavior) (cx=0x7ffff6975000, tbchars=0x7fffc2100000 "bound ", 'a' <repeats 194 times>..., length=268435461, pin=js::DoNotPinAtom) at /home/andre/git/mozilla-central/js/src/jsatom.cpp:358
#17 0x000000000056560c in js::AtomizeChars<unsigned char>(JSContext*, unsigned char const*, unsigned long, js::PinningBehavior) (cx=0x7ffff6975000, chars=0x7fffc2100000 "bound ", 'a' <repeats 194 times>..., length=268435461, pin=js::DoNotPinAtom) at /home/andre/git/mozilla-central/js/src/jsatom.cpp:450
#18 0x0000000000eca9b4 in js::StringBuffer::finishAtom() (this=0x7fffffffbb80) at /home/andre/git/mozilla-central/js/src/vm/StringBuffer.cpp:139
#19 0x0000000000e6f1b0 in intrinsic_FinishBoundFunctionInit(JSContext*, unsigned int, JS::Value*) (cx=0x7ffff6975000, argc=3, vp=0x7ffff4149178)
    at /home/andre/git/mozilla-central/js/src/vm/SelfHosting.cpp:530
...
---
This could be related to bug 1350760:
In https://hg.mozilla.org/integration/mozilla-inbound/rev/f3a6587bc94a, the JSString::validateLength() call was moved, so we now have two callers with AutoLockForExclusiveAccess on the stack.
Group: core-security → javascript-core-security
It sounds bad if multiple threads can write to this table at once.
Keywords: sec-high
Setting needinfo? based on Bug 1350760.
Flags: needinfo?(jdemooij)
Attached patch PatchSplinter Review
Call validateLength before taking the exclusive access lock.
Assignee: nobody → jdemooij
Status: NEW → ASSIGNED
Flags: needinfo?(jdemooij)
Attachment #8859118 - Flags: review?(andrebargull)
(In reply to Andrew McCreight [:mccr8] from comment #2)
> It sounds bad if multiple threads can write to this table at once.

Not multiple threads. A single thread is taking the lock twice, in opt builds this would result in a deadlock when there are helper thread zones present. If there are no helper thread zones, we don't take the lock and things should still be okay AFAIK.
Comment on attachment 8859118 [details] [diff] [review]
Patch

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

The test case may need too long to finish in non-opt builds (> 5 sec for me), is this an issue? Otherwise looks good to me!
Attachment #8859118 - Flags: review?(andrebargull) → review+
Thanks for the explanation. I'll clear the sec rating. It sounds like this could be unhidden.
Group: javascript-core-security
Pushed by jandemooij@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/fe64a3b770fd
Fix AtomizeAndCopyChars to validate length before taking the lock as it may reenter. r=anba
(In reply to André Bargull from comment #6)
> The test case may need too long to finish in non-opt builds (> 5 sec for
> me), is this an issue? Otherwise looks good to me!

I changed it to disable the test if getBuildConfiguration().debug is true. This way it at least runs in non-debug builds and the fuzzers can still reuse parts of it.
https://hg.mozilla.org/mozilla-central/rev/fe64a3b770fd
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.