848153 - Bluetooth: CycleCollection Crash in BluetoothAdapter.cpp with mJsDeviceAddresses

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

[Gregor Wagner [:gwagner]]

Main process dangling pointer crash on b2g18v1.0.1:

(gdb) bt
#0  0x40c08c3a in js::gc::ArenaHeader::getAllocKind (this=0x4bf81000) at ../../../dist/include/gc/Heap.h:513
#1  0x426d4a16 in js::gc::Cell::getAllocKind (this=0x4bf810d0) at /Volumes/mac/moz/b2g18v101/js/src/gc/Heap.h:989
#2  0x42763c92 in js::gc::GetGCThingTraceKind (thing=0x4bf810d0) at /Volumes/mac/moz/b2g18v101/js/src/jsgcinlines.h:33
#3  0x42775eca in js_GetGCThingTraceKind (thing=0x4bf810d0) at /Volumes/mac/moz/b2g18v101/js/src/jsgc.cpp:1880
#4  0x41949bf2 in xpc_GCThingIsGrayCCThing (thing=0x4bf810d0) at /Volumes/mac/moz/b2g18v101/js/xpconnect/src/nsXPConnect.cpp:656
#5  0x420f183e in GCGraphBuilder::NoteJSChild (this=0x4410cd28, child=0x4bf810d0) at /Volumes/mac/moz/b2g18v101/xpcom/base/nsCycleCollector.cpp:1939
#6  0x4207d162 in nsScriptObjectTracer::NoteJSChild (aScriptThing=0x4bf810d0, name=0x42ead154 "mJsDeviceAddresses", aClosure=0x4410cd28)
    at /Volumes/mac/moz/b2g18v101/debunagibuild/xpcom/build/nsCycleCollectionParticipant.cpp:16
#7  0x41739598 in mozilla::dom::bluetooth::BluetoothAdapter::cycleCollection::TraceImpl (p=0x495f1a80, 
    aCallback=0x4207d11d <nsScriptObjectTracer::NoteJSChild(void*, char const*, void*)>, aClosure=0x4410cd28)
    at /Volumes/mac/moz/b2g18v101/dom/bluetooth/BluetoothAdapter.cpp:42
#8  0x41264ea2 in nsDOMEventTargetHelper::cycleCollection::TraverseImpl (that=0x438183e8, p=0x495f1a80, cb=...)
    at /Volumes/mac/moz/b2g18v101/content/events/src/nsDOMEventTargetHelper.cpp:37
#9  0x4173960e in mozilla::dom::bluetooth::BluetoothAdapter::cycleCollection::TraverseImpl (that=0x438183e8, p=0x495f1a80, cb=...)
    at /Volumes/mac/moz/b2g18v101/dom/bluetooth/BluetoothAdapter.cpp:45
#10 0x41094ab0 in nsCycleCollectionParticipant::Traverse (this=0x438183e8, p=0x495f1a80, cb=...) at ../../../dist/include/nsCycleCollectionParticipant.h:254
#11 0x420f13c6 in GCGraphBuilder::Traverse (this=0x4410cd28, aPtrInfo=0x4a121e94) at /Volumes/mac/moz/b2g18v101/xpcom/base/nsCycleCollector.cpp:1810
#12 0x420f1e36 in nsCycleCollector::MarkRoots (this=0x4034f000, builder=...) at /Volumes/mac/moz/b2g18v101/xpcom/base/nsCycleCollector.cpp:2122
#13 0x420f2d68 in nsCycleCollector::BeginCollection (this=0x4034f000, aMergeCompartments=false, aListener=0x0)
    at /Volumes/mac/moz/b2g18v101/xpcom/base/nsCycleCollector.cpp:2803
#14 0x420f3684 in nsCycleCollectorRunner::Run (this=0x4034e0b0) at /Volumes/mac/moz/b2g18v101/xpcom/base/nsCycleCollector.cpp:3083
#15 0x420dbb86 in nsThread::ProcessNextEvent (this=0x40304390, mayWait=true, result=0x4410ce87) at /Volumes/mac/moz/b2g18v101/xpcom/threads/nsThread.cpp:620
#16 0x4207ccde in NS_ProcessNextEvent_P (thread=0x40304390, mayWait=true) at /Volumes/mac/moz/b2g18v101/debunagibuild/xpcom/build/nsThreadUtils.cpp:237
#17 0x420dac7a in nsThread::ThreadFunc (arg=0x40304390) at /Volumes/mac/moz/b2g18v101/xpcom/threads/nsThread.cpp:258
#18 0x405d8574 in _pt_root (arg=0x4031c200) at /Volumes/mac/moz/b2g18v101/nsprpub/pr/src/pthreads/ptthread.c:191
#19 0x40094e18 in __thread_entry (func=0x405d8455 <_pt_root>, arg=0x4031c200, tls=<value optimized out>) at bionic/libc/bionic/pthread.c:217
#20 0x4009496c in pthread_create (thread_out=<value optimized out>, attr=0xbeeb4708, start_routine=0x405d8455 <_pt_root>, arg=0x4031c200)
    at bionic/libc/bionic/pthread.c:357
#21 0x00000000 in ?? ()


We are tracing a dead object:
(gdb) p *tmp->mJsDeviceAddresses
$5 = {<js::ObjectImpl> = {<js::gc::Cell> = {static CellShift = 3, static CellSize = 8, static CellMask = 7}, shape_ = {<js::EncapsulatedPtr<js::Shape, unsigned int>> = {{
          value = 0xdadadada, other = 3671775962}}, <No data fields>}, type_ = {<js::EncapsulatedPtr<js::types::TypeObject, unsigned int>> = {{value = 0xdadadada, 
          other = 3671775962}}, <No data fields>}, slots = 0xdadadada, elements = 0xdadadada, static SLOT_CAPACITY_MIN = 8}, static NELEMENTS_LIMIT = 268435456, 
  static MAX_FIXED_SLOTS = <optimized out>, static JSSLOT_DATE_UTC_TIME = 0, static JSSLOT_DATE_TZA = <optimized out>, static JSSLOT_DATE_COMPONENTS_START = 2, 
  static JSSLOT_DATE_LOCAL_TIME = <optimized out>, static JSSLOT_DATE_LOCAL_YEAR = <optimized out>, static JSSLOT_DATE_LOCAL_MONTH = <optimized out>, 
  static JSSLOT_DATE_LOCAL_DATE = <optimized out>, static JSSLOT_DATE_LOCAL_DAY = <optimized out>, static JSSLOT_DATE_LOCAL_HOURS = <optimized out>, 
  static JSSLOT_DATE_LOCAL_MINUTES = <optimized out>, static JSSLOT_DATE_LOCAL_SECONDS = 9, static DATE_CLASS_RESERVED_SLOTS = <optimized out>, 
  static ITER_CLASS_NFIXED_SLOTS = <optimized out>, static JSSLOT_NAME_PREFIX = 0, static JSSLOT_NAME_URI = 1, static JSSLOT_NAMESPACE_DECLARED = 2, 
  static JSSLOT_QNAME_LOCAL_NAME = 2, static NAMESPACE_CLASS_RESERVED_SLOTS = <optimized out>, static QNAME_CLASS_RESERVED_SLOTS = <optimized out>}
(gdb) 

It should be fixed with bug 811206.

Comment 1

[Blake Kaplan (:mrbkap) (inactive)]

This is a crash in the parent process.

Comment 2

[Andrew McCreight [:mccr8]]

This is a UAF, so marking sec-critical.  I guess it could be mitigated a bit by being related to Bluetooth, which presumably isn't under control of random web pages, or maybe even random apps.  This was fixed on trunk by bug 811206 in 19.  I guess we should have backported that to Aurora after all!  Presumably we don't care about Bluetooth on ESR17, so marking that wontfix.

Comment 3

[Andrew McCreight [:mccr8]]

Attachment: backport part 3 of bug 811206

It might be interesting to know why we're getting a double-unlink in this case, but this should fix this problem in general.

Comment 4

[Andrew McCreight [:mccr8]]

Attachment: backport part 2 of bug 811206, debug only

This part is debug-only, and checks that when we DROP cycle collected objects, we don't hold onto dangling pointers, in case the object is not as dead as we think it is.

Comment 5

[Andrew McCreight [:mccr8]]

Comment on attachment 721927 [details] [diff] [review]
backport part 3 of bug 811206

Carrying forward the r+ on these patches from the original landing.

Comment 6

[Andrew McCreight [:mccr8]]

Comment on attachment 721929 [details] [diff] [review]
backport part 2 of bug 811206, debug only

I'm not sure what should happen here next.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/9b2c97a2aec8
https://hg.mozilla.org/releases/mozilla-b2g18/rev/a5ab028edf86

https://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/daeb554aa09f
https://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/f611f6c2cf43