Closed Bug 1646708 Opened 2 years ago Closed 2 years ago

Assertion failure: marker.isDrained(), at gc/GC.cpp:4917

Categories

(Core :: JavaScript Engine, defect, P1)

x86_64
Linux
defect

Tracking

()

VERIFIED FIXED
mozilla79
Tracking Status
firefox-esr68 --- unaffected
firefox-esr78 --- unaffected
firefox77 --- unaffected
firefox78 --- unaffected
firefox79 --- fixed

People

(Reporter: decoder, Assigned: jonco)

References

Details

(5 keywords, Whiteboard: [bugmon:update,bisected,confirmed][sec-survey])

Attachments

(2 files)

The following testcase crashes on mozilla-central revision 20200618-7f0b0cbecd94 (debug build, run with --fuzzing-safe --ion-offthread-compile=off --warp):

function ccwToObject() {
  return evaluate('({})', { global: newGlobal({newCompartment: true})});
}
function ccwToRegistry() {
  return new FinalizationRegistry(value => {});
}
function f(p) {
  let registry = ccwToRegistry();
  let target = ccwToObject();
  registry.register(target, undefined);
  if (p)
    dontStart 
}
function foo() {
  for (let p of [false, true]) {
    f(p);
    gcslice(10000);
  }
}
try { foo(); } catch(exc) {}
foo();

Backtrace:

received signal SIGSEGV, Segmentation fault.
#0  0x000055555621a09d in js::gc::GCRuntime::markGrayReferencesInCurrentGroup(JSFreeOp*, js::SliceBudget&) ()
#1  0x0000555556260621 in sweepaction::SweepActionSequence::run(js::gc::SweepAction::Args&) ()
#2  0x000055555624f617 in sweepaction::SweepActionForEach<js::gc::SweepGroupsIter, JSRuntime*>::run(js::gc::SweepAction::Args&) ()
#3  0x0000555556222bd9 in js::gc::GCRuntime::performSweepActions(js::SliceBudget&) ()
#4  0x0000555556227cc9 in js::gc::GCRuntime::incrementalSlice(js::SliceBudget&, mozilla::Maybe<JSGCInvocationKind> const&, JS::GCReason, js::gc::AutoGCSession&) ()
#5  0x000055555622ab9a in js::gc::GCRuntime::gcCycle(bool, js::SliceBudget, mozilla::Maybe<JSGCInvocationKind> const&, JS::GCReason) ()
#6  0x000055555622c790 in js::gc::GCRuntime::collect(bool, js::SliceBudget, mozilla::Maybe<JSGCInvocationKind> const&, JS::GCReason) ()
#7  0x000055555622dca8 in js::gc::GCRuntime::startDebugGC(JSGCInvocationKind, js::SliceBudget&) ()
#8  0x0000555555ee29db in GCSlice(JSContext*, unsigned int, JS::Value*) ()
#9  0x000055555593e2e2 in CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), js::CallReason, JS::CallArgs const&) ()
[...]
#21 0x00005555557b5c48 in main ()
rax	0x555557079c61	93825020697697
rbx	0x7ffff601b970	140737320696176
rcx	0x555558357840	93825040480320
rdx	0x0	0
rsi	0x7ffff7105770	140737338431344
rdi	0x7ffff7104540	140737338426688
rbp	0x7fffffffb510	140737488336144
rsp	0x7fffffffb4b0	140737488336048
r8	0x7ffff7105770	140737338431344
r9	0x7ffff7f9bd40	140737353727296
r10	0x58	88
r11	0x7ffff6dac7a0	140737334921120
r12	0x7ffff601b9a0	140737320696224
r13	0x7ffff6027000	140737320742912
r14	0x7ffff6029728	140737320752936
r15	0x7fffffffb820	140737488336928
rip	0x55555621a09d <js::gc::GCRuntime::markGrayReferencesInCurrentGroup(JSFreeOp*, js::SliceBudget&)+1085>
=> 0x55555621a09d <_ZN2js2gc9GCRuntime32markGrayReferencesInCurrentGroupEP8JSFreeOpRNS_11SliceBudgetE+1085>:	movl   $0x1335,0x0
   0x55555621a0a8 <_ZN2js2gc9GCRuntime32markGrayReferencesInCurrentGroupEP8JSFreeOpRNS_11SliceBudgetE+1096>:	callq  0x55555584464e <abort>

I'm marking this s-s for investigation because I'm not 100% convinced this is warp-only. I tried a bunch of other options (ion/baseline warmup thresholds or eager flags) and it only reproduces with --warp, but the crash and testcase look like that could be coincidence. Feel free to open this up once it is confirmed to be warp-only.

Attached file Testcase

Jonco: is this likely warp-only? Or something with the new weakrefs functionality?

Flags: needinfo?(jcoppeard)
Whiteboard: [bugmon:update,bisect] → [bugmon:update,bisected,confirmed]
Bugmon Analysis:
Verified bug as reproducible on mozilla-central 20200618094105-f291dd9e075c.
The bug appears to have been introduced in the following build range:
> Start: 431597af517cb380f5706af0d16291a8d21bf106 (20200611151225)
> End: 1ab3acda37b63503e98cb467431716f92cae3e69 (20200611151321)
> Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=431597af517cb380f5706af0d16291a8d21bf106&tochange=1ab3acda37b63503e98cb467431716f92cae3e69

This also happens with just --no-ti (which is implied by --warp). --no-ti prunes the GC graph quite a bit.

Summary: [warp] Assertion failure: marker.isDrained(), at gc/GC.cpp:4917 → Assertion failure: marker.isDrained(), at gc/GC.cpp:4917
Assignee: nobody → jcoppeard
Severity: -- → S3
Flags: needinfo?(jcoppeard)
Priority: -- → P1

The problem here is that if we don't reset sweepMarkTaskStarted then we won't start the task next time there is work for it to do. This was missing from endSweepingSweepGroup but I've refectored so this happens in joinSweepMarkTask now.

What are the security implications? Can this cause us to fail to mark something and thus lead to a UAF?

(In reply to Andrew McCreight [:mccr8] from comment #6)
Yes. This is exposed by using FinalizationRegistry so it only affects nightly for now.

Group: javascript-core-security → core-security-release
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla79
Status: RESOLVED → VERIFIED
Keywords: bugmon
Bugmon Analysis:
Verified bug as fixed on rev mozilla-central 20200624093107-e858eb7ffeba.
Removing bugmon keyword as no further action possible.
Please review the bug and re-add the keyword for further analysis.

As part of a security bug pattern analysis, we are requesting your help with a high level analysis of this bug. It is our hope to develop static analysis (or potentially runtime/dynamic analysis) in the future to identify classes of bugs.

Please visit this google form to reply.

Flags: needinfo?(jcoppeard)
Whiteboard: [bugmon:update,bisected,confirmed] → [bugmon:update,bisected,confirmed][sec-survey]

Survey completed.

Flags: needinfo?(jcoppeard)
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.