1046015 - crash in js::gc::StoreBuffer::putFromAnyThread<js::gc::StoreBuffer::MonoTypeBuffer<js::gc::StoreBuffer::SlotsEdge>, js::gc::StoreBuffer::SlotsEdge>(js::gc::StoreBuffer::MonoTypeBuffer<js::gc::StoreBuffer::SlotsEdge>&, js::gc::StoreBuffer::SlotsEdge const&

Status: RESOLVED DUPLICATE of bug 1029299

(Core :: JavaScript: GC, defect)

Description

[(Away)]

This bug was filed from the Socorro interface and is 
report bp-4178e340-c020-459f-9a60-a91912140724.
=============================================================

mozjs!js::gc::StoreBuffer::putFromAnyThread<js::gc::StoreBuffer::MonoTypeBuffer<js::gc::StoreBuffer::SlotsEdge>,js::gc::StoreBuffer::SlotsEdge>
mozjs!js::gc::StoreBuffer::putSlotFromAnyThread
mozjs!js::HeapSlot::post
mozjs!JSObject::TradeGuts
mozjs!JSObject::swap
mozjs!JS_TransplantObject
xul!xpc::TransplantObject
xul!mozilla::dom::ReparentWrapper
xul!nsNodeUtils::CloneAndAdopt

Oddly enough this only occurs on 32 beta, but it is the #3 crash there. Is there something about our GC/trains configuration that would account for that?

Comment 1

[(Away)]

In most reports, the StoreBuffer pointer is poison.

Comment 2

[Andrew McCreight [:mccr8]]

Bug 1029299 is another UAF involving TradeGuts, with a test case.

Comment 3

[Terrence Cole [:terrence]]

Attachment: dont_touch_object_guts_while_tradeguts-v0.diff

The access path to get to |this| for the storeBuffer is:
a->getSlot(i)->chunk()->info.trailer.storeBuffer

The crash line is on isOkayToUseBuffer, which is presumably inlined. The accesses it performs are:
this->enabled_
this->runtime_
this->runtime_->ownerThread
PR_GetCurrentThread (presumably a TLS access)
InParallelSection (another TLS access)

The ultra-weird thing here is that the crash address is for a /jemalloc/ poison value. The jemalloc memory here is |this| and |runtime_| (through this). This would imply that |b| is from a /dead/ runtime. This is clearly insane.

On more thought however, it occurs to me that we don't poison the GC heap on Beta, so this could also be jemalloc freed memory that has since been grabbed by the GC and re-used. In that case, it would imply that one of the accesses is simply /uninitialized/. Looking at it from that direction:

First, a->getSlot(i) has a branch to filter out non-objects, which means that it must be ObjectValue() to get to this crash. Since Value's object header does not look like 0x5a, we can pretty much rule this case out. The other GC accesses are initialized by Chunk::init out of Chunk::allocate.

This doesn't leave much room for errors in TradeGuts, unfortunately. This would imply then that before the TradeGuts, someone set b to ObjectValue(dead), or perhaps a torn value write. Nothing pretty.

I think for now maybe let's switch to using the whole-object buffer and see if the topcrasher moves. If the topcrasher moves, that would indicate real corruption elsewhere. If the crash just disappears, I'm really not sure what that means, but I wouldn't complain.

Comment 4

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

I might as well just close the whole bug.

Comment 5

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Comment on attachment 8464863 [details] [diff]
dont_touch_object_guts_while_tradeguts-v0.diff

Terrence figured out the bug here! We're barriering the slots up to numFixedSlots(), but the slots after slotSpan() are uninitialized. We'll need to backport this.

Comment 6

[Terrence Cole [:terrence]]

We should probably land this under bug 1029299, given that it's sec-bounty? and the test case the contributor submitted led directly to the fix.

Comment 8

[Lawrence Mandel [:lmandel] (use needinfo)]

Clearing tracking and status flags.