1317415 - Crash in PLDHashTable::Search | mozilla::dom::DOMIntersectionObserver::UnlinkTarget

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Daniel Holbert [:dholbert]]

This bug was filed from the Socorro interface and is 
report bp-3bf4bccf-93c8-42f4-aac8-6e5e92161114.
=============================================================

Just triggered a crash in IntersectionObserver code (null deref in DOMIntersectionObserver::UnlinkTarget), in a fresh profile with latest Nightly.

I was logged in to eBay (testing an unrelated bug), and I navigated forward and backwards a few times, after clicking on an entry in a search results page like this one:
http://www.ebay.com/sch/i.html?_from=R40&_trksid=p2050601.m570.l1313.TR11.TRC4.A0.H0.Xtblet.TRS1&_nkw=tblet&_sacat=0

I've only hit the crash once, so I don't have reliable STR. But hopefully the backtrace might help identify a potential problem.

Comment 1

[Andrew McCreight [:mccr8]]

This crash is new to the 11-14 Nightly.

Comment 2

[Andrew McCreight [:mccr8]]

[Tracking Requested - why for this release]: very frequently crash

This may more properly be a regression from bug 1315837.

This is 54% of all crashes on the latest Nightly, on Windows.

Comment 3

[Andrew McCreight [:mccr8]]

The variant crash appears to be a use-after-free, because about half of the crashes are on poison values.

Comment 4

[Marcia Knous [:marcia]]

Tracking 53+ for this top crash. I see another signature that might be related - [@ nsGenericHTMLElement::QueryInterface].

Comment 5

[Bogdan Maris, Desktop Test Engineering]

I also ran into this crash just a few minutes ago, I had only cnn.com homepage loaded in one tab, I locked the PC and after a while I logged back and saw the crash. I could not reproduce it a second time though.

bp-6269e7a0-248a-4b20-bbbd-5c1572161115

Comment 6

[Andrew McCreight [:mccr8]]

I can reproduce this by going to http://www.thedailybeast.com/cheats/2016/11/15/leaked-memo-britain-has-no-brexit-plan.html then scrolling up and down a few times, then closing the tab, and waiting for something like 5 to 10 seconds (presumably until the page is collected).

Comment 7

[Andrew McCreight [:mccr8]]

I backed out bug 1315837 for causing this crash.

Comment 8

[Tobias Schneider [:tobytailor]]

On it, still trying to reproduce.

Comment 9

[Ben Kelly [:bkelly, not reviewing]]

FWIW I just triggered this switching from twitter in one tab to linkedin in another tab.

Comment 11

[Joel Maher ( :jmaher ) (UTC -8)]

I have hit this 9 times today, here is my most recent crash on win10 with build id 20161116030212:
https://crash-stats.mozilla.com/report/index/c5d2b377-2a4c-47c3-bfcf-c530f2161116

I assume the backout mentioned in comment 7 didn't make it to nightly, or wasn't the root cause.

Comment 12

[Andrew McCreight [:mccr8]]

(In reply to Joel Maher ( :jmaher) from comment #11)
> I assume the backout mentioned in comment 7 didn't make it to nightly, or
> wasn't the root cause.

Yeah, it looks like the backout didn't make it into the 11-16 Nightly.

Comment 15

[Andrew McCreight [:mccr8]]

11-17 Nightly, which contains the backout, does not have these crashes.

Comment 16

[Nicholas Nethercote [inactive]]

(In reply to Andrew McCreight [:mccr8] from comment #15)
> 11-17 Nightly, which contains the backout, does not have these crashes.

I see five crashes with this signature in Nightly 20161118030222, but none after that. I'm not sure what to make of that.

Comment 17

[Nicholas Nethercote [inactive]]

> I see five crashes with this signature in Nightly 20161118030222, but none
> after that. I'm not sure what to make of that.

Sorry, my mistake. I was looking at crashes for a different signature.

Comment 18

[Randell Jesup [:jesup] (needinfo me)]

[Tracking Requested - why for this release]:

Thousands of crashes in 52 and 53 (including in versions after 11/16 -- 11/22 and 11/23)

Most crashes are UAFs

Comment 19

[Andrew McCreight [:mccr8]]

It looks like the branch date was 11-14 (according to https://wiki.mozilla.org/RapidRelease/Calendar ), and my backout didn't actually make it to central until 11-16, so Aurora is affected, as you marked it. I looked through every one of the signatures in this bug and I can't find any on Nightly after the 11-16 build (aside from one that looked unrelated). Randell, do you see any crashes on Nightly after the 11-16 build?

Comment 20

[Andrew McCreight [:mccr8]]

This UAF is very easy to trigger, so I'm going to mark this critical. It is also a stability issue because it is so frequent, so I'm not sure how useful it is to track it separately as a security issue, but it can't hurt.

Comment 21

[Brian Carpenter [:geeknik]]

(In reply to Andrew McCreight [:mccr8] from comment #19)

FWIW, I've been fuzzing Nightly non-stop and haven't encountered this crash since 15 November (Bug 1317807).

Comment 22

[Julien Cristau [:jcristau]]

Hopefully this will be resolved by the latest patches from bug 1315837 which just landed in aurora52.

Comment 23

[Marcia Knous [:marcia]]

(In reply to Andrew McCreight [:mccr8] from comment #19)
> It looks like the branch date was 11-14 (according to
> https://wiki.mozilla.org/RapidRelease/Calendar ), and my backout didn't
> actually make it to central until 11-16, so Aurora is affected, as you
> marked it. I looked through every one of the signatures in this bug and I
> can't find any on Nightly after the 11-16 build (aside from one that looked
> unrelated). Randell, do you see any crashes on Nightly after the 11-16 build?

Bug 1320367 which was just filed today has a crash report with 2 of the signatures using the 11-25 build: https://crash-stats.mozilla.com/report/index/93e9041f-95a1-4795-8f73-0b3f22161125 and https://crash-stats.mozilla.com/report/index/7be48be1-f0f1-4435-a5b1-f6e5a2161125#tab-details

Comment 24

[Brian Carpenter [:geeknik]]

(In reply to Marcia Knous [:marcia - use ni] from comment #23)

> Bug 1320367 which was just filed today has a crash report with 2 of the
> signatures using the 11-25 build:

FWIW, I have been unable to repro the crash in #1320367 with ASAN build ID 20161125144038 or Nightly built from https://hg.mozilla.org/mozilla-central/rev/bad312aefb42982f492ad2cf36f4c6c3d698f4f7. If I get a stack trace, I'll share it.

Comment 25

[Ludovic Hirlimann [:Usul]]

I just crashed with this on Android - just by visiting lemonde.fr they overlay ads on top of articles and if you close go back before the ad is display I think triggers the crash.

Comment 26

[Mats Palmgren (inactive)]

(In reply to Julien Cristau [:jcristau] from comment #22)
> Hopefully this will be resolved by the latest patches from bug 1315837 which
> just landed in aurora52.

Note that part 2 in that bug was backed out though, at least on Aurora.

Comment 27

[Marcia Knous [:marcia]]

Andrew: Can we get someone to own investigating the two signatures which are still crashing here (Reported in Bug 1320367) - nsTHashtable<T>::Contains | mozilla::dom::DOMIntersectionObserver::UnlinkTarget and NS_TableDrivenQI? Does something still need to be backed out (possibly intersection observer API related?)

Comment 28

[Andrew McCreight [:mccr8]]

These are all regressions from Tobias's intersection observer work.

Comment 29

[Bill Gianopoulos [:WG9s]]

I hope you already know, but maybe you do not, that navigating around the BBC website seems to trigger this issue especially if you navigate around http://www.bbc.com/sport

Comment 30

[Nicholas Nethercote [inactive]]

Bug 1320704 disables the intersection observer API and will hopefully fix this and all related bugs. It just landed on mozilla-inbound and has been nominated for Aurora uplift.

Comment 32

[Gary [:streetwolf52]]

(In reply to Nicholas Nethercote [:njn] from comment #30)
> Bug 1320704 disables the intersection observer API and will hopefully fix
> this and all related bugs. It just landed on mozilla-inbound and has been
> nominated for Aurora uplift.

I'm running with Bug 1320704 on inbound and it appears it fixed my crashes at cnn.com.

Comment 33

[Nicholas Nethercote [inactive]]

I have confirmed that disabling the API (bug 1320704) has made these crashes go away for both Nightly and Aurora. See bug 1320704 comment 14 for details.

Comment 34

[Tobias Schneider [:tobytailor]]

Attachment: Disconnect/Unlink in proper order

I finally was able to reproduce the crash. Whats causing it is the case where a script calls observe multiple times on an IntersectionObserver instance using the same target element. We don't check if the intersection observer is already added to the observing element and add a new entry to its internal array of observers every times it gets called. During deconstruction of the IntersectionObserver we then only remove the first entry we find, leaving all other entries in the array, with pointers now pointing to unlinked/freed memory. We than crash when we get to the point of collecting the target element, trying to unlink it from observing intersection observers. This scenario only leads to a crash when there IntersectionObserver gets collected before its targets (which usually only happens if an entire window/tab/iframe gets collected). That was the nondeterministic part of all this which made it quite a nightmare to reproduce. This patch is addressing all this by:

- using a nsDataHashtable instead of a nsTArray for the list of observers on Element instances
- additionally check if an intersection observer is already registered
- disconnecting from observing targets during unlinking instead of deconstruction
- adding a test

Comment 35

[Andrew McCreight [:mccr8]]

It might be better to file a new public bug with this fix

Comment 36

[Daniel Holbert [:dholbert]]

(...because this bug is already closed as FIXED [by backout], and we try to avoid landing patches for bugs after they've already been resolved, to be able to keep things straight & to have each bug tracking one main "action item".)

Comment 37

[Ryan VanderMeulen [:RyanVM]]

Updating the status flags here to better-clarify that this bug was fixed by disabling the feature in bug 1320704. Bug 1322717 covers fixing the underlying defect so that it can be re-enabled.

Comment 38

[Andrew McCreight [:mccr8]]

(I intentionally made this public because it was disabled everywhere so it wasn't worth keeping closed.)