1487020 - AddressSanitizer: heap-use-after-free [@ nsDisplayRemote::CreateWebRenderCommands] with READ of size 8

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, defect, P2)

Description

[Christian Holler (:decoder)]

The attached crash information was submitted via the ASan Nightly Reporter on mozilla-central-asan-nightly revision 63.0a1-20180827100129-https://hg.mozilla.org/mozilla-central/rev/768eef11f5ff119fc3b63221e326db16986900cb.

For detailed crash information, see attachment.

Comment 1

[Christian Holler (:decoder)]

Attachment: Detailed Crash Information

Comment 3

[Ryan VanderMeulen [:RyanVM]]

Jeff, can you please help triage this?

Comment 4

[Jeff Muizelaar [:jrmuizel]]

We're not shipping this so it's probably not bounty eligible unless it's triggered without flipping the webrender pref.

Comment 5

[Jeff Muizelaar [:jrmuizel]]

So it looks like we're trying to use the RenderFrameParent after it's gone away.

Comment 6

[Jeff Muizelaar [:jrmuizel]]

Matt do you know what mechanism nsDisplayRemote uses to make sure it's RenderFrameParent is still alive?

Comment 7

[Matt Woodrow (:mattwoodrow)]

There is a known issue with nsDisplayRemote not getting invalidated when the RenderFrameParent is deleted, Miko is looking into fixing it in bug 1413546.

However, that should only affect configurations with RDL enabled, which should not be true for the parent process, and we should never have nsDisplayRemote in the content process (until Fission is done).

Does this crash require any pref changes?

Comment 8

[Christian Holler (:decoder)]

We have an email address for the original reporter, so if you want me to reach out and ask for any specific prefs, that would be possible if you let me know which ones that would be.

Comment 9

[Daniel Veditz [:dveditz]]

changing fx63 status to "disabled" because we have this turned off in release builds and don't plan to enable it for at least another two cycles afaik.

Comment 10

[Christian Holler (:decoder)]

Fwiw, I reached out to the original reporter, asking if any of the "gfx.webrender" prefs were in a non-default state, but I haven't received a response yet. I will update the bug, once I have it.

Comment 11

[Christian Holler (:decoder)]

The original reporter told me that they have modified the pref "gfx.webrender.all" and set it to "true". No other gfx.webrender prefs where changed, I can ask for other prefs if you need them, just let me know which.

Matt, does that help you here?

Comment 12

[Matt Woodrow (:mattwoodrow)]

I guess it would need layout.display-list.retain.chrome to be true for this to happen.

Comment 13

[Miko Mynttinen]

(In reply to Matt Woodrow (:mattwoodrow) from comment #12)
> I guess it would need layout.display-list.retain.chrome to be true for this
> to happen.

This is not needed, just enabling WebRender crashes the same test that caused backout of bug 1413546. This test is not run by default with WR enabled.

Comment 14

[Miko Mynttinen]

Attachment: Bug 1487020 - Do not reuse nsDisplayRemote if RenderFrameParent has been destroyed

Comment 15

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 9006079 [details]
Bug 1487020 - Do not reuse nsDisplayRemote if RenderFrameParent has been destroyed

Matt Woodrow (:mattwoodrow) has approved the revision.

Comment 16

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/99bfc4e1be4d
https://hg.mozilla.org/mozilla-central/rev/99bfc4e1be4d

Comment 17

[Daniel Veditz [:dveditz]]

(In reply to Miko Mynttinen [:miko] from comment #13)
> This is not needed, just enabling WebRender crashes the same test that
> caused backout of bug 1413546. This test is not run by default with WR
> enabled.

Does that mean we already knew about this crash? Or that we disabled it for WebRender for other reasons and that it _could_ have caught this bug if we had remembered to re-enable it?

Comment 18

[Miko Mynttinen]

(In reply to Daniel Veditz [:dveditz] from comment #17)
> (In reply to Miko Mynttinen [:miko] from comment #13)
> > This is not needed, just enabling WebRender crashes the same test that
> > caused backout of bug 1413546. This test is not run by default with WR
> > enabled.
> 
> Does that mean we already knew about this crash? Or that we disabled it for
> WebRender for other reasons and that it _could_ have caught this bug if we
> had remembered to re-enable it?

The root cause of this problem was RenderFrameParent being used through a dangling pointer in nsDisplayRemote display item. This crash was blocking bug 1413546 from landing, and it was triggered by one Windows specific Firefox UI functional test, that does not seem to run by default on WebRender CI.

Comment 19

[Miko Mynttinen]

And to further clarify: the crash was not WebRender specific, it was also triggered by enabling parent process retained display lists.

Comment 20

[Daniel Veditz [:dveditz]]

WebRender was prefed-off for a reason -- it's a huge and incomplete project that needs testing over several cycles. It is not eligible for a bounty at this time.