[tor-bugs] #16771 [Tor Browser]: TBB crashes on Google Maps when creating markers/clicking

[Tor Bug Tracker & Wiki]

#16771: TBB crashes on Google Maps when creating markers/clicking
-------------------------+-------------------------------------------------
     Reporter:  tom      |      Owner:  arthuredelstein
         Type:  defect   |     Status:  needs_information
     Priority:  major    |  Milestone:
    Component:  Tor      |    Version:
  Browser                |   Keywords:  tbb-crash, tbb-5.0-regression,
   Resolution:           |  TorBrowserTeam201508R
Actual Points:           |  Parent ID:
       Points:           |
-------------------------+-------------------------------------------------

Comment (by arthuredelstein):

 Replying to [comment:19 mikeperry]:
 > Ok. Can you maybe set a gdb breakpoint on RemoveDataEntry() in your
 build with your test fix to ensure that blob URI removal succeeds with
 your patch at least once on maps?
 >
 https://trac.torproject.org/projects/tor/wiki/doc/TorBrowser/Hacking#AttachinganalreadyrunningTBB
 >
 > If you didn't strip your build, it should still have symbols.

 Sure. With Google maps, I saw many RevokeObjectURL calls. Here is an
 example:

 {{{
 Process 45613 stopped
 * thread #1: tid = 0x9602c6, 0x000000010248799e
 XUL`nsHostObjectProtocolHandler::RemoveDataEntry(aUri=0x00007fff5fbf7e70,
 aIsolationKey=0x00007fff5fbf7e38) + 46 at
 nsHostObjectProtocolHandler.cpp:354, queue = 'com.apple.main-thread, stop
 reason = breakpoint 2.1 3.1
     frame #0: 0x000000010248799e
 XUL`nsHostObjectProtocolHandler::RemoveDataEntry(aUri=0x00007fff5fbf7e70,
 aIsolationKey=0x00007fff5fbf7e38) + 46 at
 nsHostObjectProtocolHandler.cpp:354
    351  {
    352    if (gDataTable) {
    353      DataInfo* info = GetDataInfo(aUri);
 -> 354      if (info && info->mFirstPartyHost == aIsolationKey) {
    355        nsCString uriIgnoringRef;
    356        int32_t hashPos = aUri.FindChar('#');
    357        if (hashPos < 0) {
 (lldb) bt
 * thread #1: tid = 0x9602c6, 0x000000010248799e
 XUL`nsHostObjectProtocolHandler::RemoveDataEntry(aUri=0x00007fff5fbf7e70,
 aIsolationKey=0x00007fff5fbf7e38) + 46 at
 nsHostObjectProtocolHandler.cpp:354, queue = 'com.apple.main-thread, stop
 reason = breakpoint 2.1 3.1
     frame #0: 0x000000010248799e
 XUL`nsHostObjectProtocolHandler::RemoveDataEntry(aUri=0x00007fff5fbf7e70,
 aIsolationKey=0x00007fff5fbf7e38) + 46 at
 nsHostObjectProtocolHandler.cpp:354
     frame #1: 0x0000000102392e43
 XUL`mozilla::dom::URL::RevokeObjectURL(aGlobal=0x00007fff5fbf7fd0,
 aURL=0x00007fff5fbf7f40) + 515 at URL.cpp:210
 [...]
 (lldb) print info
 (DataInfo *) $1948 = 0x0000000120abfd30
 }}}

 Other RemoveDataEntry calls apparently come from garbage collection:

 {{{
 Process 19249 stopped
 * thread #1: tid = 0x951f13, 0x000000010248799e
 XUL`nsHostObjectProtocolHandler::RemoveDataEntry(aUri=0x000000012128ea08,
 aIsolationKey=0x00007fff5fbfc9b0) + 46 at
 nsHostObjectProtocolHandler.cpp:354, queue = 'com.apple.main-thread, stop
 reason = breakpoint 2.1 3.1
     frame #0: 0x000000010248799e
 XUL`nsHostObjectProtocolHandler::RemoveDataEntry(aUri=0x000000012128ea08,
 aIsolationKey=0x00007fff5fbfc9b0) + 46 at
 nsHostObjectProtocolHandler.cpp:354
    351  {
    352    if (gDataTable) {
    353      DataInfo* info = GetDataInfo(aUri);
 -> 354      if (info && info->mFirstPartyHost == aIsolationKey) {
    355        nsCString uriIgnoringRef;
    356        int32_t hashPos = aUri.FindChar('#');
    357        if (hashPos < 0) {
 (lldb) print info
 (DataInfo *) $1283 = 0x00000001215c8f10
 (lldb) n
 Process 19249 stopped
 * thread #1: tid = 0x951f13, 0x00000001024879d3
 XUL`nsHostObjectProtocolHandler::RemoveDataEntry(aUri=0x000000012128ea08,
 aIsolationKey=0x00007fff5fbfc9b0) + 99 at
 nsHostObjectProtocolHandler.cpp:355, queue = 'com.apple.main-thread, stop
 reason = step over
     frame #0: 0x00000001024879d3
 XUL`nsHostObjectProtocolHandler::RemoveDataEntry(aUri=0x000000012128ea08,
 aIsolationKey=0x00007fff5fbfc9b0) + 99 at
 nsHostObjectProtocolHandler.cpp:355
    352    if (gDataTable) {
    353      DataInfo* info = GetDataInfo(aUri);
    354      if (info && info->mFirstPartyHost == aIsolationKey) {
 -> 355        nsCString uriIgnoringRef;
    356        int32_t hashPos = aUri.FindChar('#');
    357        if (hashPos < 0) {
    358          uriIgnoringRef = aUri;
 (lldb) bt
 * thread #1: tid = 0x951f13, 0x00000001024879d3
 XUL`nsHostObjectProtocolHandler::RemoveDataEntry(aUri=0x000000012128ea08,
 aIsolationKey=0x00007fff5fbfc9b0) + 99 at
 nsHostObjectProtocolHandler.cpp:355, queue = 'com.apple.main-thread, stop
 reason = step over
     frame #0: 0x00000001024879d3
 XUL`nsHostObjectProtocolHandler::RemoveDataEntry(aUri=0x000000012128ea08,
 aIsolationKey=0x00007fff5fbfc9b0) + 99 at
 nsHostObjectProtocolHandler.cpp:355
     frame #1: 0x00000001023f7da0
 XUL`nsDocument::cycleCollection::Unlink(this=0x000000010885e040,
 p=0x0000000121324800) + 1344 at nsDocument.cpp:2169
     frame #2: 0x00000001036ad3cd
 XUL`nsHTMLDocument::cycleCollection::Unlink(this=0x000000010885e040,
 p=0x0000000121324800) + 61 at nsHTMLDocument.cpp:203
     frame #3: 0x000000010109382e
 XUL`nsCycleCollector::CollectWhite(this=0x0000000110596000) + 702 at
 nsCycleCollector.cpp:3297
 }}}

 In both cases, the object was removed from the gDataTable.

 I also saw examples where the `info` object was null:

 {{{
 Process 19249 stopped
 * thread #1: tid = 0x951f13, 0x000000010248799e
 XUL`nsHostObjectProtocolHandler::RemoveDataEntry(aUri=0x000000012128ea08,
 aIsolationKey=0x00007fff5fbfcb48) + 46 at
 nsHostObjectProtocolHandler.cpp:354, queue = 'com.apple.main-thread, stop
 reason = breakpoint 2.1
     frame #0: 0x000000010248799e
 XUL`nsHostObjectProtocolHandler::RemoveDataEntry(aUri=0x000000012128ea08,
 aIsolationKey=0x00007fff5fbfcb48) + 46 at
 nsHostObjectProtocolHandler.cpp:354
    351  {
    352    if (gDataTable) {
    353      DataInfo* info = GetDataInfo(aUri);
 -> 354      if (info && info->mFirstPartyHost == aIsolationKey) {
    355        nsCString uriIgnoringRef;
    356        int32_t hashPos = aUri.FindChar('#');
    357        if (hashPos < 0) {
 (lldb) print info
 (DataInfo *) $1300 = 0x0000000000000000
 (lldb) bt
 * thread #1: tid = 0x951f13, 0x000000010248799e
 XUL`nsHostObjectProtocolHandler::RemoveDataEntry(aUri=0x000000012128ea08,
 aIsolationKey=0x00007fff5fbfcb48) + 46 at
 nsHostObjectProtocolHandler.cpp:354, queue = 'com.apple.main-thread, stop
 reason = breakpoint 2.1
     frame #0: 0x000000010248799e
 XUL`nsHostObjectProtocolHandler::RemoveDataEntry(aUri=0x000000012128ea08,
 aIsolationKey=0x00007fff5fbfcb48) + 46 at
 nsHostObjectProtocolHandler.cpp:354
     frame #1: 0x00000001023f5418
 XUL`nsDocument::~nsDocument(this=0x0000000121324800) + 2200 at
 nsDocument.cpp:1785
 }}}

 And, as expected, the block in lines 355-368 was skipped, and nothing was
 removed from the gDataTable. Presumably the object had already been
 removed in an earlier call.

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/16771#comment:20>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online