Closed Bug 1188290 Opened 4 years ago Closed 4 years ago

Assertion failure: bufferCell.has(this, CellPtrEdge(cellp)) || !CellPtrEdge(cellp).maybeInRememberedSet(nursery_), at js/src/gc/StoreBuffer.h:449

Categories

(Core :: JavaScript Engine, defect, critical)

x86_64
Linux
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla44
Tracking Status
firefox42 --- affected
firefox44 --- fixed

People

(Reporter: decoder, Assigned: terrence)

References

(Blocks 1 open bug)

Details

(Keywords: assertion, regression, testcase, Whiteboard: [jsbugmon:update])

Attachments

(1 file)

The following testcase crashes on mozilla-central revision d3228c82badd (build with --enable-optimize --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-debug, run with --no-threads --ion-eager):

this.__proto__ = [];
var T = TypedObject;
var ObjectStruct = new T.StructType({f: T.Object});
var o = new ObjectStruct();
minorgc();
function writeObject(o, v) {
    o.f = v;
    assertEq(typeof o.f, "object");
}
for (var i = 0; i < 5; i++)
    writeObject(o, { toString: function() { return "helo"} }
);



Backtrace:

Program received signal SIGSEGV, Segmentation fault.
0x00000000005e8221 in assertHasCellEdge (cellp=0x7ffff7e80030, this=<optimized out>) at js/src/gc/StoreBuffer.h:448
#0  0x00000000005e8221 in assertHasCellEdge (cellp=0x7ffff7e80030, this=<optimized out>) at js/src/gc/StoreBuffer.h:448
#1  JSObject::writeBarrierPost (cellp=0x7ffff7e80030, prev=<optimized out>, next=<optimized out>) at js/src/jsobj.h:641
#2  0x000000000061f6d7 in postBarrier (next=0x7ffff54000e0, prev=0x7ffff5400080, vp=0x7ffff7e80030) at js/src/gc/Barrier.h:244
#3  post (next=0x7ffff54000e0, prev=0x7ffff5400080, this=0x7ffff7e80030) at js/src/gc/Barrier.h:423
#4  set (v=<optimized out>, this=0x7ffff7e80030) at js/src/gc/Barrier.h:430
#5  operator= (p=<optimized out>, this=0x7ffff7e80030) at js/src/gc/Barrier.h:420
#6  js::StoreReferenceHeapPtrObject::store (cx=cx@entry=0x7ffff6907000, heap=0x7ffff7e80030, v=..., obj=obj@entry=0x7ffff7e80020, id=..., id@entry=...) at js/src/builtin/TypedObject.cpp:2779
#7  0x000000000061fa5d in js::StoreReferenceHeapPtrObject::Func (cx=0x7ffff6907000, argc=<optimized out>, vp=0x7fffffffbe48) at js/src/builtin/TypedObject.cpp:2823
#8  0x00007ffff7fa1cfe in ?? ()
#9  0xfff9800000000000 in ?? ()
#10 0x00007fffffffbe20 in ?? ()
#11 0x0000000000000000 in ?? ()
rax	0x0	0
rbx	0x7ffff693c3a0	140737330267040
rcx	0x7ffff6ca53b0	140737333842864
rdx	0x0	0
rsi	0x7ffff6f7a9d0	140737336814032
rdi	0x7ffff6f791c0	140737336807872
rbp	0x7fffffffbd50	140737488338256
rsp	0x7fffffffbca0	140737488338080
r8	0x7ffff7fe0780	140737354008448
r9	0x6372732f736a2f6c	7165916604736876396
r10	0x7fffffffba60	140737488337504
r11	0x7ffff6c27960	140737333328224
r12	0x7ffff6934b10	140737330236176
r13	0x7ffff7e80030	140737352564784
r14	0x7fffffffbd00	140737488338176
r15	0x0	0
rip	0x5e8221 <JSObject::writeBarrierPost(void*, JSObject*, JSObject*)+897>
=> 0x5e8221 <JSObject::writeBarrierPost(void*, JSObject*, JSObject*)+897>:	movl   $0x1c1,0x0
   0x5e822c <JSObject::writeBarrierPost(void*, JSObject*, JSObject*)+908>:	callq  0x498fe0 <abort()>


s-s because this is a GC assertion.
Keywords: sec-high
A similar testcase found by randorderfuzz:

// Randomly chosen test: js/src/tests/js1_5/extensions/regress-462734-02.js
__proto__ = __proto__;
// Randomly chosen test: js/src/jit-test/tests/TypedObject/jit-write-references.js
var ObjectStruct = new TypedObject.StructType({
    f: TypedObject.Object
});
with({}) {}
var o = new ObjectStruct();
minorgc();
function f(o) {
    o.f = {
        toString: function() {}
    };
    "" + o.f;
}
for (var i = 0; i < 9; i++) {
    f(o);
}

asserts identically on m-c rev 62cd40885e93 with --fuzzing-safe --no-threads --ion-eager.

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/3c61b61ea4a2
user:        Terrence Cole
date:        Thu Jun 18 10:23:49 2015 -0700
summary:     Bug 1175642 - Fix the interface that RelocatablePtr uses to interact with the StoreBuffer; r=jonco

Terrence, is bug 1175642 a likely regressor?
Blocks: 1175642
Flags: needinfo?(terrence)
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]
Group: core-security → javascript-core-security
This is an incomplete assertion.

The initializing write to the TypedObject happens in jitcode and uses the whole object buffer. Then we take a stub into C++ and the next write to that slot happens in C++, using the cell buffer. We try to assert that the previous write is in the cell buffer and it is not. It has still, however, correctly buffered, just in a different store buffer.

This problem is almost certainly not limited to TypedObjects, could happen at any write in C++, and there is no good way to get the object where we need it. I think the only fix here is to just not do the assertion. Failures will still show up as UAF, as they have in the past.
Assignee: nobody → terrence
Status: NEW → ASSIGNED
Flags: needinfo?(terrence)
Attachment #8663204 - Flags: review?(jdemooij)
This is not sec-sensitive. The code is correct, it is just the assertion going wrong.
Group: javascript-core-security
Attachment #8663204 - Flags: review?(jdemooij) → review+
https://hg.mozilla.org/mozilla-central/rev/b1628bb8771e
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
Keywords: sec-high
Depends on: 1210653
You need to log in before you can comment on or make changes to this bug.