1461704 - Crash in nsINode::GetComposedDocInternal

Status: RESOLVED WORKSFORME

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

Description

[Calixte Denizet (:calixte)]

This bug was filed from the Socorro interface and is
report bp-a33f8a88-7158-454a-962d-e5b100180515.
=============================================================

Top 10 frames of crashing thread:

0 xul.dll nsINode::GetComposedDocInternal dom/base/nsINode.cpp:447
1 xul.dll mozilla::dom::FragmentOrElement::CanSkip dom/base/FragmentOrElement.cpp:1753
2 xul.dll mozilla::dom::CharacterData::cycleCollection::CanSkipReal dom/base/CharacterData.cpp:80
3 xul.dll mozilla::CycleCollectedJSRuntime::UnmarkSkippableJSHolders xpcom/base/CycleCollectedJSRuntime.cpp:628
4 xul.dll nsCCUncollectableMarker::Observe dom/base/nsCCUncollectableMarker.cpp:441
5 xul.dll nsObserverList::NotifyObservers xpcom/ds/nsObserverList.cpp:112
6 xul.dll nsObserverService::NotifyObservers xpcom/ds/nsObserverService.cpp:297
7 xul.dll XPCJSRuntime::PrepareForForgetSkippable js/xpconnect/src/XPCJSRuntime.cpp:676
8 xul.dll nsCycleCollector::ForgetSkippable xpcom/base/nsCycleCollector.cpp:2938
9 xul.dll nsCycleCollector_forgetSkippable xpcom/base/nsCycleCollector.cpp:4268

=============================================================

There is 1 crash in nightly 62 with buildid 20180514220126. In analyzing the backtrace, the regression may have been introduced by patch [1] to fix bug 1001469.
The crash address is 0xffffffffe5e5e60d so a UAF.

[1] https://hg.mozilla.org/mozilla-central/rev?node=99e338bc3709

Comment 1

[Olli Pettay [:smaug][[e-mail]](vacation until Nov 16)]

I don't understand how that crash could happen, unless we have deleted CharacterData object in mJSHolders.
If that is the case, we would have crashed just in different place before bug 1001469.

Comment 2

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

(In reply to Olli Pettay [:smaug] from comment #1)
> I don't understand how that crash could happen, unless we have deleted
> CharacterData object in mJSHolders.
> If that is the case, we would have crashed just in different place before
> bug 1001469.

Where would it have crashed before that landing?

Comment 3

[Olli Pettay [:smaug][[e-mail]](vacation until Nov 16)]

Anywhere where cycle collector touches objects.

Comment 4

[Daniel Veditz [:dveditz]]

(In reply to Olli Pettay [:smaug] from comment #1)
> I don't understand how that crash could happen, unless we have deleted
> CharacterData object in mJSHolders.

Socorro says the crash _did_ happen, so who investigates having deleted CharacterData objects in mJSHolders?

Comment 5

[Olli Pettay [:smaug][[e-mail]](vacation until Nov 16)]

Stack traces in bug 1462548 smell a bit same. Hopefully we'll get a testcase there.

Comment 6

[Daniel Veditz [:dveditz]]

Isn't that bug related to shadow DOM? these crashes are in versions that don't have that.

On the other hand we don't see crashes in 62 beta so maybe something fixed it? but it's a really low volume crash (only 2-4 crashes in earlier betas over 3 months).

Calixte: why do you think this is a regression? Seems to have been around at a low level a long time.

Comment 7

[Calixte Denizet (:calixte)]

:dvetditz, there was a crash in 20180416220315 with this signature and the next one was in 20180514220126 (the one in comment #0) so no crash during one month and suddenly reappeared with a just touched line on the top of the backtrace: I considered it was a regression for these reason.
Of course it may be a coincidence so feel free to remove the keyword regression if you think it isn't.

Comment 8

[Daniel Veditz [:dveditz]]

I think it was just rare -- especially since it involves the Shadow DOM and that feature is supposed to be disabled in the older releases that crash here. Are people playing around with it, or is it not as disabled as we thought?

From comment 5 this might be bug 1462548 which was fixed by bug 1463116 (landed on May 22).

Most (not all) of the older-version crashes are near null and that was fixed by Bug 1428393.

The most-recent crashing nightly is build 20180518222751 -- older than the likely fix and nearly a month ago. Going to close this "worksforme" assuming that was really the fix.