Closed Bug 1706441 Opened 3 years ago Closed 2 years ago

crash in [@ blend_span]

Categories

(Core :: Graphics: WebRender, defect)

defect

Tracking

()

VERIFIED FIXED
101 Branch
Tracking Status
firefox-esr78 --- unaffected
firefox-esr91 100+ fixed
firefox88 --- wontfix
firefox89 --- wontfix
firefox90 --- wontfix
firefox98 --- wontfix
firefox99 --- wontfix
firefox100 + fixed
firefox101 + verified

People

(Reporter: tsmith, Assigned: lsalzman)

References

(Blocks 1 open bug, Regression)

Details

(5 keywords, Whiteboard: [bugmon:bisected,confirmed][adv-main100+r][adv-esr91.9+r])

Attachments

(3 files)

Attached file testcase.html

First found while fuzzing m-c 20210418-78764895b2a6 (--enable-coverage --enable-fuzzing)

I can only reproduce this with a fuzzing-ccov build. No idea why. No repro on fuzzing-asan-opt, fuzzing-debug, ccov-opt or opt.

I'm happy to test patches if you have guesses.

==179379==ERROR: UndefinedBehaviorSanitizer: SEGV on unknown address 0x7f9fe27a2460 (pc 0x7fa156e36cc8 bp 0x000000000000 sp 0x7fa0b7e96830 T179430)
==179379==The signal is caused by a READ memory access.
    #0 0x7fa156e36cc8 in memcpy /builds/worker/fetches/sysroot/usr/include/x86_64-linux-gnu/bits/string3.h:51:10
    #1 0x7fa156e36cc8 in load<unsigned int> src/gfx/wr/swgl/src/vector_type.h:503:5
    #2 0x7fa156e36cc8 in unaligned_load<unsigned char __attribute__((ext_vector_type(16))), unsigned int> src/gfx/wr/swgl/src/vector_type.h:532:10
    #3 0x7fa156e36cc8 in blend_span src/gfx/wr/swgl/src/blend.h:732:28
    #4 0x7fa156e36cc8 in commit_blend_span<true, unsigned int, unsigned short __attribute__((ext_vector_type(16)))> src/gfx/wr/swgl/src/blend.h:792:22
    #5 0x7fa156e36cc8 in unsigned int* blendTextureLinearFallback<true, glsl::sampler2D_impl*, NoColor, unsigned int>(glsl::sampler2D_impl*, glsl::vec2, int, glsl::vec2_scalar, glsl::vec2_scalar, glsl::vec2_scalar, NoColor, unsigned int*) src/gfx/wr/swgl/src/swgl_ext.h:177:5
    #6 0x7fa156f0906a in int blendTextureLinear<true, glsl::sampler2D_impl*, NoColor, unsigned int>(glsl::sampler2D_impl*, glsl::vec2, int, glsl::vec4_scalar const&, NoColor, unsigned int*, LinearFilter) src/gfx/wr/swgl/src/swgl_ext.h:456:3
    #7 0x7fa156f83f3f in brush_image_ALPHA_PASS_TEXTURE_2D_frag::swgl_drawSpanRGBA8() /builds/worker/workspace/obj-build/x86_64-unknown-linux-gnu/release/build/swgl-c0a134755c64a46a/out/brush_image_ALPHA_PASS_TEXTURE_2D.h:895:2
    #8 0x7fa156f7f8fb in brush_image_ALPHA_PASS_TEXTURE_2D_frag::draw_span_RGBA8(brush_image_ALPHA_PASS_TEXTURE_2D_frag*) /builds/worker/workspace/obj-build/x86_64-unknown-linux-gnu/release/build/swgl-c0a134755c64a46a/out/brush_image_ALPHA_PASS_TEXTURE_2D.h:938:42
    #9 0x7fa157144c48 in draw_span src/gfx/wr/swgl/src/program.h:149:12
    #10 0x7fa157144c48 in draw_depth_span<unsigned int> src/gfx/wr/swgl/src/rasterize.h:597:38
    #11 0x7fa157144c48 in void draw_quad_spans<unsigned int>(int, glsl::vec2_scalar*, unsigned int, glsl::vec3*, Texture&, Texture&, ClipRect const&) src/gfx/wr/swgl/src/rasterize.h:999:13
    #12 0x7fa156da9dcf in draw_quad(int, Texture&, Texture&) src/gfx/wr/swgl/src/rasterize.h:1592:5
    #13 0x7fa156da6e93 in void draw_elements<unsigned short>(int, int, unsigned long, VertexArray&, Texture&, Texture&) src/gfx/wr/swgl/src/rasterize.h:1622:5
    #14 0x7fa156da6c68 in DrawElementsInstanced src/gfx/wr/swgl/src/gl.cc:2699:7
    #15 0x7fa155a2ce24 in webrender::device::gl::Device::draw_indexed_triangles_instanced_u16::he541a299179565db src/gfx/wr/webrender/src/device/gl.rs:3556:9
    #16 0x7fa155a2ce24 in webrender::renderer::Renderer::draw_instanced_batch::h36364c6060e0f56b src/gfx/wr/webrender/src/renderer/mod.rs:2561:17
    #17 0x7fa155a2a967 in webrender::renderer::Renderer::draw_alpha_batch_container::h196dfcc56eac88f7 src/gfx/wr/webrender/src/renderer/mod.rs:3045:17
    #18 0x7fa155a2415e in webrender::renderer::Renderer::draw_picture_cache_target::ha4153bd6409179a8 src/gfx/wr/webrender/src/renderer/mod.rs:2868:9
    #19 0x7fa155a20936 in webrender::renderer::Renderer::draw_frame::h406fc00bfb194484 src/gfx/wr/webrender/src/renderer/mod.rs:4683:21
    #20 0x7fa155a57937 in webrender::renderer::Renderer::render_impl::h0f98e3578feb63ca src/gfx/wr/webrender/src/renderer/mod.rs:2159:17
    #21 0x7fa155a64899 in webrender::renderer::Renderer::render::h3e3d3b3a9b7c29e2 src/gfx/wr/webrender/src/renderer/mod.rs:1894:30
    #22 0x7fa156188f7c in wr_renderer_render src/gfx/webrender_bindings/src/bindings.rs:636:11
    #23 0x7fa148d49881 in mozilla::wr::RendererOGL::UpdateAndRender(mozilla::Maybe<mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits> > const&, mozilla::Maybe<mozilla::wr::ImageFormat> const&, mozilla::Maybe<mozilla::Range<unsigned char> > const&, bool*, mozilla::wr::RendererStats*) src/gfx/webrender_bindings/RendererOGL.cpp:186:8
    #24 0x7fa148d4861c in mozilla::wr::RenderThread::UpdateAndRender(mozilla::wr::WrWindowId, mozilla::layers::BaseTransactionId<mozilla::VsyncIdType> const&, mozilla::TimeStamp const&, bool, mozilla::Maybe<mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits> > const&, mozilla::Maybe<mozilla::wr::ImageFormat> const&, mozilla::Maybe<mozilla::Range<unsigned char> > const&, bool*) src/gfx/webrender_bindings/RenderThread.cpp:486:31
    #25 0x7fa148d47dfe in mozilla::wr::RenderThread::HandleFrameOneDoc(mozilla::wr::WrWindowId, bool) src/gfx/webrender_bindings/RenderThread.cpp:341:3
    #26 0x7fa148d547ae in applyImpl<mozilla::wr::RenderThread, void (mozilla::wr::RenderThread::*)(mozilla::wr::WrWindowId, bool), StoreCopyPassByConstLRef<mozilla::wr::WrWindowId>, StoreCopyPassByConstLRef<bool> , 0, 1> /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:1148:12
    #27 0x7fa148d547ae in apply<mozilla::wr::RenderThread, void (mozilla::wr::RenderThread::*)(mozilla::wr::WrWindowId, bool)> /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:1154:12
    #28 0x7fa148d547ae in mozilla::detail::RunnableMethodImpl<mozilla::wr::RenderThread*, void (mozilla::wr::RenderThread::*)(mozilla::wr::WrWindowId, bool), true, (mozilla::RunnableKind)0, mozilla::wr::WrWindowId, bool>::Run() /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:1201:13
    #29 0x7fa1476f7df5 in MessageLoop::RunTask(already_AddRefed<nsIRunnable>) src/ipc/chromium/src/base/message_loop.cc:468:11
    #30 0x7fa1476f8514 in MessageLoop::DeferOrRunPendingTask(MessageLoop::PendingTask&&) src/ipc/chromium/src/base/message_loop.cc:477:5
    #31 0x7fa1476f880a in MessageLoop::DoWork() src/ipc/chromium/src/base/message_loop.cc:552:13
    #32 0x7fa1476f9450 in base::MessagePumpDefault::Run(base::MessagePump::Delegate*) src/ipc/chromium/src/base/message_pump_default.cc:35:31
    #33 0x7fa1476f7bbf in RunInternal src/ipc/chromium/src/base/message_loop.cc:335:10
    #34 0x7fa1476f7bbf in RunHandler src/ipc/chromium/src/base/message_loop.cc:328:3
    #35 0x7fa1476f7bbf in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:310:3
    #36 0x7fa14770bea1 in base::Thread::ThreadMain() src/ipc/chromium/src/base/thread.cc:191:16
    #37 0x7fa1477022e1 in ThreadFunc(void*) src/ipc/chromium/src/base/platform_thread_posix.cc:40:13
    #38 0x7fa16b594608 in start_thread /build/glibc-eX1tMB/glibc-2.31/nptl/pthread_create.c:477:8
    #39 0x7fa16b15d292 in clone /build/glibc-eX1tMB/glibc-2.31/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:95
Flags: in-testsuite?
Attached file prefs.js
Severity: -- → S4

Nothing I can really do here without being able to reliably catch this in the act, and the stack trace isn't saying much about what is really going on. At a minimum a pernosco session would be helpful.

No problem. I made a ccov-fuzzing-noopt build to run under rr and of course it does not repro :(

I'll keep an eye on our buckets and hopefully a more reliable test case comes in.

Bugmon Analysis:
Verified bug as reproducible on mozilla-central 20210430214504-b417d526e5fc.
The bug appears to have been introduced in the following build range:

Start: 6f3a9007793c5f356522b8d283e9e2bc4a4f6b4f (20210402094809)
End: c2f0b7fb408a5b29c04d68b633a8ccbc3e19a425 (20210402214149)
Pushlog: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=6f3a9007793c5f356522b8d283e9e2bc4a4f6b4f&tochange=c2f0b7fb408a5b29c04d68b633a8ccbc3e19a425

Whiteboard: [bugmon:bisected,confirmed]

just guessing, but the fix for bug 1701975 is in this range and touches this code.

CCov builds are slow so maybe that compile option makes some races more likely?

Flags: needinfo?(lsalzman)
Keywords: sec-high
Depends on: gfx-triage
No longer depends on: gfx-triage

Waiting on a more reliable reproduction so that it can be debugged.

Keywords: stalled

Still waiting on a better repro here...

Flags: needinfo?(lsalzman)

The severity field for this bug is set to S4. However, the bug is flagged with the sec-high keyword.
:bhood, could you consider increasing the severity of this security bug?

For more information, please visit auto_nag documentation.

Flags: needinfo?(bhood)

Tyson, does this still reproduce at all?

Flags: needinfo?(bhood) → needinfo?(twsmith)

I was able to reproduce the issue with the attached testcase and prefs.js with m-c 20220328-eaf3521f3b37.

On Ubuntu 20.04 (and 18.04) I used Grizzly Replay:

$ pip install fuzzfetch grizzly-framework
$ python -m fuzzfetch --cov --fuzzing -n firefox
$ python -m grizzly.replay ./firefox/firefox testcase.html --xvfb

A Pernosco session is available here: https://pernos.co/debug/zHlyVfNAfxnCSxAEbyg6iQ/index.html

Please let me know if you'd like any patches tested.

Flags: needinfo?(twsmith)
Keywords: stalled

Without debugging information it is hard to see much of anything about what's going on in that trace...

Here is another attempt: https://pernos.co/debug/UnanKBsMaXGNfZEa6fR9Mg/index.html

This is tricky because for some reason this crash requires a ccov build. So I need to create a ccov build with --enable-debug and with the lowest debug level possible. Of course when I build with --disable-optimize I am no longer able to reproduce the issue. The build used to collect this trace is using -O1. In the trace some information seems to be available in shallower frames but not deeper frames, I'm not sure if there is enough to help you.

If this is potentially due to a compiler option like bug 1746545 send me a list of options to try and I'd be happy to test.

Flags: needinfo?(lsalzman)
Assignee: nobody → lsalzman
Status: NEW → ASSIGNED

It appears what's going on here is clang is being a bit too smart on its optimizations. Within the branch uv_step.x > 0.0f && insideDist >= uv_step.x it infers that both are non-zero, and further that int(insideDist / uv_step.x) * swgl_StepSize is >= 0, and so it elides the check for max(that, 0) = that > 0 ? that : 0 within the clamp since it determines that >= 0. Where we fall afoul of things is that the * swgl_StepSize operation overflows, which falls into "undefined behavior" land, and the compiler pretends it will never overflow as a result and that the operation is always predictably >= 0. This patch restructures the check so that inside is compared > 0, which the compiler can't guarantee or elide, so that the undefined behavior no longer matters...

Flags: needinfo?(lsalzman)

:lsalzman, since this bug contains a bisection range, could you fill (if possible) the regressed_by field?
For more information, please visit auto_nag documentation.

Flags: needinfo?(lsalzman)
Flags: needinfo?(lsalzman)
Regressed by: sw-wr-perf-linear

Comment on attachment 9269994 [details]
Bug 1706441 - Check for non-empty span. r?jrmuizel

Security Approval Request

  • How easily could an exploit be constructed based on the patch?: Not easily. Only seems to reproduce in fuzzing coverage builds.
  • Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: No
  • Which older supported branches are affected by this flaw?: 91+
  • If not all supported branches, which bug introduced the flaw?: None
  • Do you have backports for the affected branches?: No
  • If not, how different, hard to create, and risky will they be?: Not hard or risky.
  • How likely is this patch to cause regressions; how much testing does it need?: Unlikely.
Attachment #9269994 - Flags: sec-approval?

Set release status flags based on info from the regressing bug 1692731

Comment on attachment 9269994 [details]
Bug 1706441 - Check for non-empty span. r?jrmuizel

Approved to land and uplift

Attachment #9269994 - Flags: sec-approval? → sec-approval+
Group: gfx-core-security → core-security-release
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 101 Branch

Bugmon Analysis
Verified bug as fixed on rev mozilla-central 20220407092959-6ff2b7d52aa3.
Removing bugmon keyword as no further action possible. Please review the bug and re-add the keyword for further analysis.

Status: RESOLVED → VERIFIED
Keywords: bugmon

The patch landed in nightly and beta is affected.
:lsalzman, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Flags: needinfo?(lsalzman)

Comment on attachment 9269994 [details]
Bug 1706441 - Check for non-empty span. r?jrmuizel

Beta/Release Uplift Approval Request

  • User impact if declined: Potential buffer underread leading to crashes. Unknown if this affects release builds.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Just fixes a correctness issue related to compositing empty areas - it just skips them now. Risk should be very minimal.
  • String changes made/needed:

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration:
  • User impact if declined:
  • Fix Landed on Version:
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky):
Flags: needinfo?(lsalzman)
Attachment #9269994 - Flags: approval-mozilla-esr91?
Attachment #9269994 - Flags: approval-mozilla-beta?

Comment on attachment 9269994 [details]
Bug 1706441 - Check for non-empty span. r?jrmuizel

Approved for 100.0b5

Attachment #9269994 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Comment on attachment 9269994 [details]
Bug 1706441 - Check for non-empty span. r?jrmuizel

Approved for 91.9esr

Attachment #9269994 - Flags: approval-mozilla-esr91? → approval-mozilla-esr91+
Whiteboard: [bugmon:bisected,confirmed] → [bugmon:bisected,confirmed][adv-main100+r][adv-esr91.9+r]
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.