1698543 - Assertion failure: !cell.asCell()->isForwarded(), at gc/Marking.cpp:4199

Status: VERIFIED FIXED

(Core :: JavaScript: GC, defect, P1)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 20210315-29ed711969d6 (debug build, run with --fuzzing-safe --cpu-count=2 --ion-offthread-compile=off):

foo = "";
doit(`
  // XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
  // XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
  function u() { broken(
  // XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
  // XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
  // XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
  // XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
  // XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
  // XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
`);
function t(f88) {
    var testObj = {
        [Symbol.toPrimitive]() {
            gczeal(4);
        }
    };
    f88(testObj);
}
t(Number);
doit("");
unescape(foo);
function doit(x) {
    try {
      evaluate(x);
    } catch (e) {
      if (e instanceof SyntaxError)
        doit(x);
    }
    x = x.replace(/!/g, "");
    foo += x + " ";
}

Backtrace:

received signal SIGSEGV, Segmentation fault.
#0  0x000055555746a50f in js::GCMarker::traceBarrieredCell(JS::GCCellPtr) ()
#1  0x0000555557456000 in js::GCMarker::traceBarrieredCells(js::SliceBudget&) ()
#2  0x0000555557450ab8 in js::gc::BarrierTracer::performBarrier(JS::GCCellPtr) ()
#3  0x0000555556fcd093 in JSLinearString* JSRope::flattenInternal<(JSRope::UsingBarrier)0, unsigned char>(JSContext*) ()
#4  0x0000555556fa6a2f in JSRope::flatten(JSContext*) ()
#5  0x0000555556dfab9f in str_unescape(JSContext*, unsigned int, JS::Value*) ()
#6  0x0000555556b85201 in CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), js::CallReason, JS::CallArgs const&) ()
#7  0x0000555556b84940 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) ()
#8  0x0000555556b85d61 in InternalCall(JSContext*, js::AnyInvokeArgs const&, js::CallReason) ()
#9  0x0000555556b794bd in Interpret(JSContext*, js::RunState&) ()
[...]
#18 0x00005555569f4cb1 in main ()
rax	0x555555753d54	93824994327892
rbx	0x3243e634132	3454200398130
rcx	0x555557ff1088	93825036914824
rdx	0x0	0
rsi	0x7ffff6abd770	140737331844976
rdi	0x7ffff6abc540	140737331840320
rbp	0x7fffffffba50	140737488337488
rsp	0x7fffffffba10	140737488337424
r8	0x7ffff6abd770	140737331844976
r9	0x7ffff7fe3840	140737354020928
r10	0x58	88
r11	0x7ffff67647a0	140737328334752
r12	0x7ffff5749430	140737311446064
r13	0x7ffff5749400	140737311446016
r14	0x7ffff5749400	140737311446016
r15	0x3243e634130	3454200398128
rip	0x55555746a50f <js::GCMarker::traceBarrieredCell(JS::GCCellPtr)+4687>
=> 0x55555746a50f <_ZN2js8GCMarker18traceBarrieredCellEN2JS9GCCellPtrE+4687>:	movl   $0x1067,0x0
   0x55555746a51a <_ZN2js8GCMarker18traceBarrieredCellEN2JS9GCCellPtrE+4698>:	callq  0x555556a7e364 <abort>

This is a fairly frequent and hard to reduce, marking as fuzzblocker.

Comment 1

[Christian Holler (:decoder)]

Attachment: Testcase

Comment 2

[Bugmon [:jkratzer for issues]]

Bugmon Analysis:
Verified bug as reproducible on mozilla-central 20210315091836-29ed711969d6.
The bug appears to have been introduced in the following build range:

Start: 8994342c7c86cda3b90a5c66aa5968fe9de32bf6 (20210301115638)
End: 678d1355789d1891e6a3a8530a0ce8240613683a (20210301140308)
Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=8994342c7c86cda3b90a5c66aa5968fe9de32bf6&tochange=678d1355789d1891e6a3a8530a0ce8240613683a

Comment 3

[Jon Coppeard (:jonco)]

Attachment: Bug 1698543 - Rework handling of interaction between incremental barriers and rope flattening r?sfink

Currently we have a special path for performing barriers during rope flattening to take account of the fact that this overwrites the cell header word of ropes temporarily, making these cells untraceable.

The problem is that a rope can already be in the barrier buffer before flattening starts and so we need to check for this when processing the buffer.

The patch reworks use of temporary GC data to set one of the cell header flags when in use. This means we can check for it when processing the barrier buffer. It removes the special case function for barriers, which makes it slightly less efficient as we will now buffer all these ropes before skipping them later on. Hopefully that is not too deterimental to performance.

Comment 4

[Andrew McCreight (out of office until 8/21) [:mccr8]]

This sounds like it could cause some kind of UAF issue, so I'll mark it sec-high.

Is this a regression from the given patch, or did the patch just add more asserts that caught an existing issue?

Comment 5

[Jon Coppeard (:jonco)]

(In reply to Andrew McCreight [:mccr8] from comment #4)
It's a regression from bug 1694209, yes.

Comment 6

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1694209

Comment 7

[Andrew McCreight (out of office until 8/21) [:mccr8]]

[Tracking Requested - why for this release]: sec-high regression

Comment 8

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Rework handling of interaction between incremental barriers and rope flattening r=sfink
https://hg.mozilla.org/integration/autoland/rev/fabb8df993fb044dbf52c3c74f6d679cd42c7f73
https://hg.mozilla.org/mozilla-central/rev/fabb8df993fb

Comment 9

[Bugmon [:jkratzer for issues]]

Bugmon Analysis:
Verified bug as fixed on rev mozilla-central 20210317212527-4d4bc56f77a1.
Removing bugmon keyword as no further action possible.
Please review the bug and re-add the keyword for further analysis.

Comment 10

[BugBot [:suhaib / :marco/ :calixte]]

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.