Closed Bug 1739454 Opened 3 years ago Closed 3 years ago

Crash [@ PremultiplyChunk_SSE2<false, false>]

Categories

(Core :: Graphics, defect, P5)

x86_64
Linux
defect

Tracking

()

RESOLVED FIXED
96 Branch
Tracking Status
firefox-esr91 --- wontfix
firefox94 --- wontfix
firefox95 --- wontfix
firefox96 --- fixed

People

(Reporter: tsmith, Assigned: lsalzman)

References

(Blocks 1 open bug, Regression)

Details

(Keywords: regression, testcase, Whiteboard: [bugmon:confirm])

Crash Data

Attachments

(3 files, 1 obsolete file)

Testcase found while fuzzing mozilla-central rev c97b17652863 (built with: --enable-debug --enable-fuzzing).

Testcase can be reproduced using the following commands:

$ pip install fuzzfetch grizzly-framework
$ python -m fuzzfetch --build c97b17652863 --debug --fuzzing -n firefox
$ python -m grizzly.replay ./firefox/firefox testcase.zip
[@ PremultiplyChunk_SSE2<false, false>]

    ==9344==ERROR: UndefinedBehaviorSanitizer: SEGV on unknown address 0x7f9a270e0000 (pc 0x7f9a30b79ca6 bp 0x7ffc11a47fa0 sp 0x7ffc11a47f98 T9344)
    ==9344==The signal is caused by a WRITE memory access.
        #0 0x7f9a30b79ca6 in PremultiplyChunk_SSE2<false, false> /gfx/2d/SwizzleSSE2.cpp:100:5
        #1 0x7f9a30b79ca6 in void mozilla::gfx::Premultiply_SSE2<false, false>(unsigned char const*, int, unsigned char*, int, mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits>) /gfx/2d/SwizzleSSE2.cpp:132:5
        #2 0x7f9a30bdfe3b in mozilla::gfx::PremultiplyData(unsigned char const*, int, mozilla::gfx::SurfaceFormat, unsigned char*, int, mozilla::gfx::SurfaceFormat, mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits> const&) /gfx/2d/Swizzle.cpp
        #3 0x7f9a328092c9 in mozilla::dom::ImageBitmap::PrepareForDrawTarget(mozilla::gfx::DrawTarget*) /dom/canvas/ImageBitmap.cpp:667:5
        #4 0x7f9a3280874d in mozilla::dom::CanvasRenderingContext2D::CreatePattern(mozilla::dom::HTMLImageElementOrSVGImageElementOrHTMLCanvasElementOrHTMLVideoElementOrImageBitmap const&, nsTSubstring<char16_t> const&, mozilla::ErrorResult&) /dom/canvas/CanvasRenderingContext2D.cpp:2164:47
        #5 0x7f9a31d2ac0d in mozilla::dom::CanvasRenderingContext2D_Binding::createPattern(JSContext*, JS::Handle<JSObject*>, void*, JSJitMethodCallArgs const&) /builds/worker/workspace/obj-build/dom/bindings/CanvasRenderingContext2DBinding.cpp:3997:80
        #6 0x7f9a327718f8 in bool mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::NormalThisPolicy, mozilla::dom::binding_detail::ThrowExceptions>(JSContext*, unsigned int, JS::Value*) /dom/bindings/BindingUtils.cpp:3300:13
        #7 0x7f9a35f4722f in CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), js::CallReason, JS::CallArgs const&) /js/src/vm/Interpreter.cpp:385:13
        #8 0x7f9a35f4692b in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) /js/src/vm/Interpreter.cpp:472:12
        #9 0x7f9a35f4832e in InternalCall(JSContext*, js::AnyInvokeArgs const&, js::CallReason) /js/src/vm/Interpreter.cpp:532:10
        #10 0x7f9a35f3dac3 in CallFromStack /js/src/vm/Interpreter.cpp:536:10
        #11 0x7f9a35f3dac3 in Interpret(JSContext*, js::RunState&) /js/src/vm/Interpreter.cpp:3240:16
        #12 0x7f9a35f34855 in js::RunScript(JSContext*, js::RunState&) /js/src/vm/Interpreter.cpp:354:13
        #13 0x7f9a35f46826 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) /js/src/vm/Interpreter.cpp:504:13
        #14 0x7f9a35f4832e in InternalCall(JSContext*, js::AnyInvokeArgs const&, js::CallReason) /js/src/vm/Interpreter.cpp:532:10
        #15 0x7f9a35f48531 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>, js::CallReason) /js/src/vm/Interpreter.cpp:549:8
        #16 0x7f9a36325d00 in js::CallSelfHostedFunction(JSContext*, JS::Handle<js::PropertyName*>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /js/src/vm/SelfHosting.cpp:1538:10
        #17 0x7f9a360dcabb in AsyncFunctionResume(JSContext*, JS::Handle<js::AsyncFunctionGeneratorObject*>, ResumeKind, JS::Handle<JS::Value>) /js/src/vm/AsyncFunction.cpp:152:8
        #18 0x7f9a3621b59e in AsyncFunctionPromiseReactionJob /js/src/builtin/Promise.cpp:2000:12
        #19 0x7f9a3621b59e in PromiseReactionJob(JSContext*, unsigned int, JS::Value*) /js/src/builtin/Promise.cpp:2160:12
        #20 0x7f9a35f4722f in CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), js::CallReason, JS::CallArgs const&) /js/src/vm/Interpreter.cpp:385:13
        #21 0x7f9a35f4692b in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) /js/src/vm/Interpreter.cpp:472:12
        #22 0x7f9a35f4832e in InternalCall(JSContext*, js::AnyInvokeArgs const&, js::CallReason) /js/src/vm/Interpreter.cpp:532:10
        #23 0x7f9a35f48531 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>, js::CallReason) /js/src/vm/Interpreter.cpp:549:8
        #24 0x7f9a361008c1 in JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /js/src/vm/CallAndConstruct.cpp:117:10
        #25 0x7f9a31a5e70c in mozilla::dom::PromiseJobCallback::Call(mozilla::dom::BindingCallContext&, JS::Handle<JS::Value>, mozilla::ErrorResult&) /builds/worker/workspace/obj-build/dom/bindings/PromiseBinding.cpp:35:8
        #26 0x7f9a2f4fc055 in mozilla::dom::PromiseJobCallback::Call(mozilla::ErrorResult&, char const*, mozilla::dom::CallbackObject::ExceptionHandling, JS::Realm*) /builds/worker/workspace/obj-build/dist/include/mozilla/dom/PromiseBinding.h:89:12
        #27 0x7f9a2f4fb38b in Call /builds/worker/workspace/obj-build/dist/include/mozilla/dom/PromiseBinding.h:102:12
        #28 0x7f9a2f4fb38b in mozilla::PromiseJobRunnable::Run(mozilla::AutoSlowOperation&) /xpcom/base/CycleCollectedJSContext.cpp:213:18
        #29 0x7f9a2f4e5f78 in mozilla::CycleCollectedJSContext::PerformMicroTaskCheckPoint(bool) /xpcom/base/CycleCollectedJSContext.cpp:674:17
        #30 0x7f9a2f4e6d9c in mozilla::CycleCollectedJSContext::AfterProcessTask(unsigned int) /xpcom/base/CycleCollectedJSContext.cpp:463:3
        #31 0x7f9a308e4785 in XPCJSContext::AfterProcessTask(unsigned int) /js/xpconnect/src/XPCJSContext.cpp:1499:28
        #32 0x7f9a2f61135a in nsThread::ProcessNextEvent(bool, bool*) /xpcom/threads/nsThread.cpp:1212:24
        #33 0x7f9a2f61816a in NS_ProcessNextEvent(nsIThread*, bool) /xpcom/threads/nsThreadUtils.cpp:467:10
        #34 0x7f9a300a5134 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /ipc/glue/MessagePump.cpp:107:5
        #35 0x7f9a2ffc4217 in MessageLoop::RunInternal() /ipc/chromium/src/base/message_loop.cc:331:10
        #36 0x7f9a2ffc4122 in RunHandler /ipc/chromium/src/base/message_loop.cc:324:3
        #37 0x7f9a2ffc4122 in MessageLoop::Run() /ipc/chromium/src/base/message_loop.cc:306:3
        #38 0x7f9a33ef6d98 in nsBaseAppShell::Run() /widget/nsBaseAppShell.cpp:137:27
        #39 0x7f9a35dca703 in XRE_RunAppShell() /toolkit/xre/nsEmbedFunctions.cpp:917:20
        #40 0x7f9a300a607a in mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) /ipc/glue/MessagePump.cpp:235:9
        #41 0x7f9a2ffc4217 in MessageLoop::RunInternal() /ipc/chromium/src/base/message_loop.cc:331:10
        #42 0x7f9a2ffc4122 in RunHandler /ipc/chromium/src/base/message_loop.cc:324:3
        #43 0x7f9a2ffc4122 in MessageLoop::Run() /ipc/chromium/src/base/message_loop.cc:306:3
        #44 0x7f9a35dc9d3b in XRE_InitChildProcess(int, char**, XREChildData const*) /toolkit/xre/nsEmbedFunctions.cpp:749:34
        #45 0x55a8dd336d79 in content_process_main /browser/app/../../ipc/contentproc/plugin-container.cpp:57:28
        #46 0x55a8dd336d79 in main /browser/app/nsBrowserApp.cpp:327:18
        #47 0x7f9a44edf0b2 in __libc_start_main /build/glibc-eX1tMB/glibc-2.31/csu/../csu/libc-start.c:308:16
        #48 0x55a8dd31250c in _start (/home/worker/builds/m-c-20211029213029-fuzzing-debug/firefox-bin+0x1550c)
    
    UndefinedBehaviorSanitizer can not provide additional info.
    SUMMARY: UndefinedBehaviorSanitizer: SEGV /gfx/2d/SwizzleSSE2.cpp:100:5 in PremultiplyChunk_SSE2<false, false>
    ==9344==ABORTING
Attached file testcase.zip
Group: core-security → gfx-core-security
Flags: in-testsuite?

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

Blocks: domino

Bugmon Analysis
Bugmon was unable to identify a testcase that reproduces this issue.
Removing bugmon keyword as no further action possible. Please review the bug and re-add the keyword for further analysis.

Keywords: bugmon

This looks like a crash on a weird value in graphics code, so I'll mark it sec-high, but that might be too high depending on what the issue is.

Keywords: sec-high
Blocks: gfx-triage
Flags: needinfo?(lsalzman)

In the pernosco session, it seems like ubsan is being triggered on a write to the very first SSE2 chunk in the image, that is definitely within the mapped bounds of the data - 32x32, aligned stride of 128, so no funny business with invalid bounds. It seems like as soon as we get the ImageBitmap in CanvasRenderingContext2D here (https://searchfox.org/mozilla-central/source/dom/canvas/CanvasRenderingContext2D.cpp#2177), that the SourceSurfaceSharedData stored inside the ImageBitmap itself just doesn't point to good memory, since when mapped it directly gives us the pointer to the start of the memory we are ultimately trying to write to downwind and crashing on...

Andrew, ideas? Something screwy going on with the BMP decoder, or just the ImageBitmap code itself, or...?

Flags: needinfo?(lsalzman) → needinfo?(aosmond)

I replaced the included bmp from the testcase with a handful of different normal jpgs I had around (they are not testcases, so should be normal) and the tab still crashes for me (in the premultiply code, looks like in a similar place to the pernosco). I also tried changing the testcase to fetch the empty string instead of a proper file name and the tab still crashes. So it seems like the problem does not lie in imagelib.

It seems like the problem is that SourceSurfaceSharedData is actually mapped as read-only as a consequence of a change made in bug 1432375. The offending bits happen here: https://searchfox.org/mozilla-central/source/gfx/layers/SourceSurfaceSharedData.cpp#19

So this begs the question of whether this is strictly only a bug in debug mode, and we should allow PremultiplyData to proceed on the shmem as it always has in non-debug modes, or do we want to disallow that in any case?

The solution seems to be to make SourceSurfaceSharedData::Map not allow WRITE or READ_WRITE modes, but again, is this only for the debug case, or do we want this to be the law all the time?

Regardless, I don't think this represents anything approaching a sec-high or even a sec bug for that matter, since it's not actually writing out of bounds and the memory it is writing to is still properly allocated, it merely happens to be marked read-only as a result of SHARED_SURFACE_PROTECT_FINALIZED.

(In reply to Lee Salzman [:lsalzman] from comment #7)

Regardless, I don't think this represents anything approaching a sec-high or even a sec bug for that matter, since it's not actually writing out of bounds and the memory it is writing to is still properly allocated, it merely happens to be marked read-only as a result of SHARED_SURFACE_PROTECT_FINALIZED.

Thanks for the analysis Lee.

Keywords: sec-high
Group: gfx-core-security

The surface need only be finalized if it is put into the SurfaceCache and something else might pull it out. We don't need to cache cases which are decoded through imgTools because we just create temporary anonymous images that go away when the operation is completed. I'm not sure the best way to fix it offhand, but perhaps all anonymous images should set a special decode flag...

Flags: needinfo?(aosmond)

Debug build only. We can patch this up when we get some free time.

No longer blocks: gfx-triage
Severity: -- → S3
Flags: needinfo?(aosmond)
Priority: -- → P5
Regressed by: 1239752
Has Regression Range: --- → yes

ImageBitmap::PrepareForDrawTarget overwrites its surface's contents when premultiplying the
data. This code pattern seems to stem all the way back to bug 1239752. It doesn't seem like
we have much control over where this surface comes from, and we have no strong guarantees
that this surface isn't shared by multiple consumers. As such, overwriting the data could
result in another consumer getting premultiplied data in the surface that it didn't expect.
We should just always be cloning the surface first before premultiplying unless we can
otherwise prove the surface isn't shared.

Assignee: nobody → lsalzman
Status: NEW → ASSIGNED

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

Flags: needinfo?(aosmond)
Pushed by lsalzman@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1de7d723923a
Don't overwrite the ImageBitmap's surface in PrepareForDrawTarget. r=emilio
https://hg.mozilla.org/integration/autoland/rev/18fdb7a13e63
Don't allow write-mapping of SourceSurfaceSharedData after Finalize. r=aosmond
Pushed by lsalzman@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3accfa522abb
Remove reftest-wait in reftest. r=emilio

Backed out 3 changesets (Bug 1739454) for causing crashtest failures in 1739454-1.html
Backout link: https://hg.mozilla.org/integration/autoland/rev/2252db5b63f6764b8476c308d8a21377dd75eb1e
Push with failures, failure log.

Flags: needinfo?(lsalzman)
Flags: needinfo?(lsalzman)
Attachment #9251600 - Attachment is obsolete: true
Pushed by lsalzman@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d985b98fa014
Don't overwrite the ImageBitmap's surface in PrepareForDrawTarget. r=emilio
https://hg.mozilla.org/integration/autoland/rev/1eafd70b4049
Don't allow write-mapping of SourceSurfaceSharedData after Finalize. r=aosmond
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 96 Branch

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)
Flags: needinfo?(lsalzman)
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: