1670521 - Crash in [@ nsTArray_Impl<T>::~nsTArray_Impl | mozilla::layers::AnimationInfo::~AnimationInfo]

Status: RESOLVED WORKSFORME

(Core :: Layout, defect, P4)

Description

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

UAF: I only see crashes in 82.0b7 -- could only exist there, or be a signature change. However all appear to be from the same user, but with different URLs and times they crashed -- perhaps an extension is involved?

Crash report: https://crash-stats.mozilla.org/report/index/9a855124-2c0c-4855-9d16-390cc0201006

Reason: EXCEPTION_ACCESS_VIOLATION_READ

Top 10 frames of crashing thread:

0 xul.dll nsTArray_Impl<mozilla::layers::Animation, nsTArrayInfallibleAllocator>::~nsTArray_Impl xpcom/ds/nsTArray.h:1011
1 xul.dll mozilla::layers::AnimationInfo::~AnimationInfo gfx/layers/AnimationInfo.cpp:28
2 xul.dll mozilla::layers::Layer::~Layer gfx/layers/Layers.cpp:228
3 xul.dll mozilla::layers::BasicPaintedLayer::~BasicPaintedLayer gfx/layers/basic/BasicPaintedLayer.h:42
4 xul.dll mozilla::layers::LayerPropertiesBase::~LayerPropertiesBase gfx/layers/LayerTreeInvalidation.cpp:215
5 xul.dll nsTArray_Impl<mozilla::UniquePtr<mozilla::layers::LayerPropertiesBase, mozilla::DefaultDelete<mozilla::layers::LayerPropertiesBase> >, nsTArrayInfallibleAllocator>::~nsTArray_Impl xpcom/ds/nsTArray.h:1012
6 xul.dll mozilla::layers::ContainerLayerProperties::~ContainerLayerProperties gfx/layers/LayerTreeInvalidation.cpp:373
7 xul.dll mozilla::InactiveLayerData::~InactiveLayerData layout/painting/FrameLayerBuilder.cpp:5555
8 xul.dll mozilla::FrameLayerBuilder::~FrameLayerBuilder layout/painting/FrameLayerBuilder.cpp:1859
9 xul.dll mozilla::FrameLayerBuilder::~FrameLayerBuilder layout/painting/FrameLayerBuilder.cpp:1855

Comment 1

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

Crashes go back to at least 77; usually random-ish addresses (some nullptr, some wildptr); the e5e5 clear UAFs were just from one user per comment 0.

Comment 2

[Neha Kochar [:neha]]

Boris, could you please find an assignee for this sec-high bug.

Comment 3

[Boris Chiou [:boris]]

Hiro, would you mind taking a look at this? Seems this happens only on windows.

Comment 4

[Hiroyuki Ikezoe (:hiro)]

Neha, though this is a sec-high issue, is this higher priority than FIssion M6c issues? I've currently been working on M6c issues, if this one is higher than those, I will take a look. That being said, this crash is very low crash volume rate and it's super unclear what's going on the time when the crash happened, so it might be possible I will just spent time to investigate and will result nothing.

(In reply to Randell Jesup [:jesup] (needinfo me) from comment #0)

UAF: I only see crashes in 82.0b7 -- could only exist there, or be a signature change. However all appear to be from the same user, but with different URLs and times they crashed -- perhaps an extension is involved?

Yeah, plausible. Extensions destroy the corresponding widget (thus it ends up destroying compositor stuff) every time when a popup window opens, I don't think it's a good manner. :/

Assigning to me (in case we just need an owner).

Comment 5

[Neha Kochar [:neha]]

Since this is pre-existing at least from 77, and we don't have all the clarity, please prioritize this after the Fission M6c (due Jan) and M7 (due Feb) work. I have been trying to make sure we try and close our sec crit and high bugs as soon as we can, that's why the NI to find an owner.
Thanks for checking with me.

Comment 6

[Andrew McCreight [:mccr8]]

This is still happening, for what it is worth. For instance, here's a crash in 86 where register r9 is a poison value: bp-44652ea6-acfe-4952-a97a-dff6b0210225

Comment 7

[Neha Kochar [:neha]]

Looks like more layout-related.

Comment 8

[Daniel Holbert [:dholbert]]

(In reply to Neha Kochar [:neha] from comment #7)

Looks like more layout-related.

Looks like this component reassignment caused the assignee field to be reset. --> Restoring that to hiro, to have this in his queue, when he can get to it.

(Also, changing platform to Windows rather than Win7, since it looks like we have reports from Win8.1 and Win10 as well.)

Comment 9

[Daniel Holbert [:dholbert]]

(As noted in comment 4, crash volume continues to be nonzero but very low).

Comment 10

[Maire Reavy [:mreavy]]

Hey Daniel, Is this bug actionable, or is it stalled (until we get more info)?

Comment 11

[Hiroyuki Ikezoe (:hiro)]

Note that this crash happens only on the Layers backend, i.e. it doesn't happen on WebRender, AFIACT. So once after we finished the migration to WebRender, we don't need to worry this.

Comment 12

[Daniel Holbert [:dholbert]]

(In reply to Maire Reavy [:mreavy] from comment #10)

Hey Daniel, Is this bug actionable, or is it stalled (until we get more info)?

I'll defer to hiro on that; but given the low crash volume, I think we can just let this be fixed-by-WebRender, per comment 11.

Comment 13

[Daniel Holbert [:dholbert]]

Closing as fixed-by-webrender, since I think we're starting to do that for bugs in that category.