Closed Bug 848153 Opened 11 years ago Closed 11 years ago

Bluetooth: CycleCollection Crash in BluetoothAdapter.cpp with mJsDeviceAddresses


(Core :: DOM: Device Interfaces, defect)

Not set



B2G C4 (2jan on)
blocking-b2g tef+
Tracking Status
firefox19 --- unaffected
firefox20 --- unaffected
firefox21 --- unaffected
firefox22 --- unaffected
firefox-esr17 --- wontfix
b2g18 --- fixed
b2g18-v1.0.0 --- wontfix
b2g18-v1.0.1 --- fixed


(Reporter: gwagner, Assigned: mccr8)



(Keywords: csectype-uaf, sec-critical)


(2 files)

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>}

It should be fixed with bug 811206.
blocking-b2g: --- → tef?
Depends on: 811206
This is a crash in the parent process.
Severity: normal → critical
Group: core-security
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.
It might be interesting to know why we're getting a double-unlink in this case, but this should fix this problem in general.
Assignee: nobody → continuation
Attachment #721927 - Attachment description: backport part 3 of bug 811206, debug only → backport part 3 of bug 811206
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 on attachment 721927 [details] [diff] [review]
backport part 3 of bug 811206

Carrying forward the r+ on these patches from the original landing.
Attachment #721927 - Flags: review+
Comment on attachment 721929 [details] [diff] [review]
backport part 2 of bug 811206, debug only

I'm not sure what should happen here next.
Attachment #721929 - Flags: review+
blocking-b2g: tef? → tef+
Keywords: checkin-needed
Closed: 11 years ago
Resolution: --- → FIXED
Group: core-security → core-security-release
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.