1966754 - feGaussianBlur crashes firefox with [@ webrender::render_task::render_task_sanity_check ]

Status: RESOLVED FIXED

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

Description

[Philip Rogers]

Attachment: 41483741.html

Steps to reproduce:

When triaging a Chromium bug, I found a case that seems to crash firefox (see attached). This crash has not been observed on real user content, just a minimized testcase from a browser engineer.

Actual results:

Crash

Comment 1

[Francesco Lodolo [:flod]]

Limiting access to MoCo employees for now.

Here's a bug report: https://crash-stats.mozilla.org/report/index/6de61d27-8092-40e9-b750-e47dc0250515#tab-bugzilla

Moving based on the bugs showing up as possible matches.

Comment 2

[Daniel Veditz [:dveditz]]

Based on the panic, this testcase causes a render where either the width or height exceeds 16384 (MAX_RENDER_TASK_SIZE). Ideally this should be caught earlier in the code and be handled by returning an appropriate error, but webrender::render_task::render_task_sanity_check() is a backstop that will crash the browser if it's not rather than risk over- (or under-) flowing buffers later. Given the sizes I see in the testcase I don't know how the render gets that big.

The feDisplacementMap doesn't seem to matter: I can remove it and it still crashes.

Both the feColorMatrix and feGaussianBlur seem to be required: it doesn't crash if I remove either of those. It also doesn't crash if I reduce the size of the filter (drastically!): if I set the width to 5px and height to 10px it doesn't crash. If I increase the width to 7px it does crash, and if I set the width to 6px it beachballs on macos and i have to manually kill the process.

Since the crash is happening while rendering the blur I'm taking the liberty of changing the summary. Frame 8 of the crash stack is
https://hg-edge.mozilla.org/mozilla-central/file/907a3d528f5eef73b26914c19fac7c0b0d79e190/gfx/wr/webrender/src/render_task.rs#l2330

Comment 3

[Glenn Watson [:gw]]

Ashley, sounds related to your svg filter work?

Comment 4

[Ashley Hale [:ahale]]

I'm not so sure about this being related to my svg filter work - the shader for feDisplacementMap is not implemented in WebRender currently and hence gfx.webrender.svg-filter-effects.fedisplacementmap is false so this goes to blob fallback, which implies this is not properly tiling the blob fallback perhaps?

Comment 5

[Ashley Hale [:ahale]]

I'll take a look at this, on the assumption that feDisplacementMap is a distraction, if feColorMatrix and feGaussianBlur in combination on a very large image is causing a panic then that needs to be fixed, I'm assuming the real culprit is feGaussianBlur and I'll need to take a look at how the subregions are being generated in this case and what the parameters are on these nodes.

Comment 6

[Jim Mathies [:jimm]]

Maybe an S2? Not sure.

Comment 7

[Bob Hood [:bhood]]

I will discuss this with Ashley.

Comment 8

[Ashley Hale [:ahale]]

I consider this an S3 unless demonstrated in the wild, it's an S2 in the sense that it brings down the GPU process in a crash loop (what SRE folks call task flapping), so if encountered in the wild it makes the browser entirely unusable (and potentially across browser restarts if restoring pages is enabled). Unfortunately this is the general failure mode of WebRender when RenderTask size is excessive, I don't entirely agree with that behavior because we do get this kind of thing wrong from time to time and having it just crash-loop the GPU process is a really bad behavior.

In terms of demonstrating this in the wild, I think https://bugzilla.mozilla.org/show_bug.cgi?id=1961176 may be the same issue (and again feGaussianBlur with excessive size is the theme).

Comment 9

[Ashley Hale [:ahale]]

I'm less convinced that Bug 1961176 is the same issue because while sizes are getting excessive there, it's a matter of memory exhaustion caused by the cumulative sizes of blurs rather than this where a single item is too big and crashes, similar root cause but this one is way more extreme.

Comment 10

[Ashley Hale [:ahale]]

Attachment: Bug 1966754 - reduce SVGFE render task resolution for excessively large inputs r?#gfx-reviewers

Comment 11

[Daniel Veditz [:dveditz]]

(In reply to Philip Rogers from comment #0)

When triaging a Chromium bug, I found a case that seems to crash firefox [...]

Philip: Can we get a URL for the chromium bug for reference? If it's not a public bug, do you need us to keep this bug hidden for a time? Having the link is handy even if it's not currently public: if you want us to keep ours hidden we can then use the link to periodically check when it would be OK to make ours public

Comment 12

[Ashley Hale [:ahale]]

When patching this bug I am including a WPT crashtest, so if there's some reason to land only the code fix at first and omit the crashtest for now, I'd appreciate knowing why :)

Comment 13

[Daniel Holbert [:dholbert]]

(In reply to Daniel Veditz [:dveditz] from comment #11)

do you need us to keep this bug hidden for a time?

FWIW looking at the history here, it seems there's no reason for concern on this^ -- Philip filed this as a public bug (it was only made private in comment 1 by flod as a precaution while we investigated the crash); and the only crash mentioned here was a Firefox crash (the Chrome bug may have just been a correctness issue). So, in the absence of a signal to the contrary, I think we're fine to publish stuff here.

(In reply to Ashley Hale [:ahale] from comment #12)

if there's some reason to land only the code fix at first and omit the crashtest for now

Per above, probably no need to worry about splitting it up.

Comment 14

[Ashley Hale [:ahale]]

(In reply to Daniel Holbert [:dholbert] from comment #13)

Per above, probably no need to worry about splitting it up.

I appreciate the clarification, thanks!

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/c9c5e7ddb932
https://hg.mozilla.org/integration/autoland/rev/fa53eb0659e5
reduce SVGFE render task resolution for excessively large inputs r=gfx-reviewers,bradwerth

Comment 16

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/53709 for changes under testing/web-platform/tests

Comment 17

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/fa53eb0659e5

Comment 18

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 19

[BugBot [:suhaib / :marco/ :calixte]]

Since nightly and release are affected, beta will likely be affected too.
For more information, please visit BugBot documentation.

Comment 20

[Ryan VanderMeulen [:RyanVM]]

Please nominate this for ESR140 uplift when you get a chance. It cherry-picks cleanly.

Comment 21

[Ashley Hale [:ahale]]

Attachment: Bug 1966754 - reduce SVGFE render task resolution for excessively large inputs r?#gfx-reviewers

Original Revision: https://phabricator.services.mozilla.com/D256388

Comment 22

[Ashley Hale [:ahale]]

Comment on attachment 9505839 [details]
Bug 1966754 - reduce SVGFE render task resolution for excessively large inputs r?#gfx-reviewers

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: This prevents a rare crash with SVG filters on very large source images (image in the CSS sense - can be a block of text, etc), it has reduced crash volume in nightly.
• User impact if declined: Crashes continue...?
• Fix Landed on Version: 141
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This code change hasn't caused problems so far.

Comment 23

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr140/rev/eeac337d55b2