Closed Bug 1715136 Opened 3 years ago Closed 8 months ago

Large allocation in [@ mozilla::gfx::ConvolutionFilter::ComputeResizeFilter]

Categories

(Core :: Graphics: ImageLib, defect)

defect

Tracking

()

RESOLVED FIXED
126 Branch
Tracking Status
firefox-esr115 --- wontfix
firefox90 --- wontfix
firefox91 --- wontfix
firefox124 --- wontfix
firefox125 --- wontfix
firefox126 --- fixed

People

(Reporter: tsmith, Assigned: lsalzman)

References

(Blocks 3 open bugs)

Details

(Keywords: csectype-oom, testcase)

Attachments

(2 files)

Attached file testcase.bmp

Found while fuzzing m-c 20210607-24938c537a55 (--enable-address-sanitizer --enable-fuzzing)

To help catch this issue ASAN_OPTIONS=max_allocation_size_mb=512 was used.

==23855==WARNING: AddressSanitizer failed to allocate 0x78000f0a bytes
=================================================================
==23855==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000001 (pc 0x55dfa6598b48 bp 0x7f0ad49b0670 sp 0x7f0ad49b0660 T23)
    #0 0x55dfa6598b48 in mozalloc_abort /gecko/memory/mozalloc/mozalloc_abort.cpp:33:3
    #1 0x55dfa6598cdd in mozalloc_handle_oom(unsigned long) /gecko/memory/mozalloc/mozalloc_oom.cpp:51:3
    #2 0x55dfa659920b in moz_xrealloc /gecko/memory/mozalloc/mozalloc.cpp:74:5
    #3 0x7f0ae90afa6b in SkTDArray<short>::resizeStorageToAtLeast(int) /gecko/gfx/skia/skia/include/private/SkTDArray.h:364:22
    #4 0x7f0ae90aebd8 in mozilla::gfx::ConvolutionFilter::ComputeResizeFilter(mozilla::gfx::ConvolutionFilter::ResizeMethod, int, int) /gecko/gfx/2d/ConvolutionFilter.cpp:122:12
    #5 0x7f0ae9e55d86 in nsresult mozilla::image::DownscalingFilter<mozilla::image::SurfaceSink>::Configure<mozilla::image::SurfaceConfig>(mozilla::image::DownscalingConfig const&, mozilla::image::SurfaceConfig const&) /gecko/image/DownscalingFilter.h:146:19
    #6 0x7f0ae9e1e4c1 in mozilla::Maybe<mozilla::image::SurfacePipe> mozilla::image::SurfacePipeFactory::MakePipe<mozilla::image::DownscalingConfig, mozilla::image::SurfaceConfig>(mozilla::image::DownscalingConfig const&, mozilla::image::SurfaceConfig const&) /gecko/image/SurfacePipeFactory.h:587:25
    #7 0x7f0ae9dbc42f in mozilla::image::SurfacePipeFactory::CreateSurfacePipe(mozilla::image::Decoder*, mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits> const&, mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits> const&, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&, mozilla::gfx::SurfaceFormat, mozilla::gfx::SurfaceFormat, mozilla::Maybe<mozilla::image::AnimationParams> const&, _qcms_transform*, mozilla::image::SurfacePipeFlags) /gecko/image/SurfacePipeFactory.h:543:22
    #8 0x7f0ae9dc7eec in mozilla::image::nsBMPDecoder::AllocateSurface() /gecko/image/decoders/nsBMPDecoder.cpp:897:29
    #9 0x7f0ae9e85562 in mozilla::image::nsBMPDecoder::DoDecode(mozilla::image::SourceBufferIterator&, mozilla::image::IResumable*)::$_0::operator()(mozilla::image::nsBMPDecoder::State, char const*, unsigned long) const /gecko/image/decoders/nsBMPDecoder.cpp:450:20
    #10 0x7f0ae9dc11db in BufferedRead<(lambda at /builds/worker/checkouts/gecko/image/decoders/nsBMPDecoder.cpp:432:7)> /gecko/image/StreamingLexer.h:605:11
    #11 0x7f0ae9dc11db in Lex<(lambda at /builds/worker/checkouts/gecko/image/decoders/nsBMPDecoder.cpp:432:7)> /gecko/image/StreamingLexer.h:470:26
    #12 0x7f0ae9dc11db in mozilla::image::nsBMPDecoder::DoDecode(mozilla::image::SourceBufferIterator&, mozilla::image::IResumable*) /gecko/image/decoders/nsBMPDecoder.cpp:430:17
    #13 0x7f0ae9c9abd7 in mozilla::image::Decoder::Decode(mozilla::image::IResumable*) /gecko/image/Decoder.cpp:177:19
    #14 0x7f0ae9ca8259 in mozilla::image::DecodedSurfaceProvider::Run() /gecko/image/DecodedSurfaceProvider.cpp:123:34
    #15 0x7f0ae9cd530c in mozilla::image::DecodingTask::Run() /gecko/image/DecodePool.cpp:146:12
    #16 0x7f0ae6d002b4 in mozilla::TaskController::RunPoolThread() /gecko/xpcom/threads/TaskController.cpp:268:33
    #17 0x7f0b04af90be in _pt_root /gecko/nsprpub/pr/src/pthreads/ptthread.c:201:5
    #18 0x7f0b08c19608 in start_thread /build/glibc-eX1tMB/glibc-2.31/nptl/pthread_create.c:477:8
    #19 0x7f0b087e2292 in clone /build/glibc-eX1tMB/glibc-2.31/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:95
Flags: in-testsuite?
Attachment #9225737 - Attachment mime type: image/bmp → application/octet-stream

We already check this value against a max at

https://searchfox.org/mozilla-central/rev/da5d08750e504f3710f7ea051327d9c311c39902/gfx/2d/ConvolutionFilter.cpp#119

Can you add this to an ignore list or something? I think the code is pretty much fine, even though for very large images it will allocate a lot of memory.

Flags: needinfo?(twsmith)

If we made this call site fallible, by giving a special reserveAdditional that is fallible, that would fix this in a way that appeases the fuzzers. Does that make sense?

Flags: needinfo?(lsalzman)
Blocks: wr-fuzz
Severity: -- → S3
Blocks: oom-fuzz
Flags: needinfo?(twsmith)
Flags: needinfo?(lsalzman)

This allows SkConvolutionFilter1D::AddFilter to simply return false on
an OOM rather than cause an unrecoverable error.

Assignee: nobody → lsalzman
Status: NEW → ASSIGNED
Pushed by lsalzman@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/96b5dabadd86 Make SkConvolutionFilter1D::AddFilter fallible. r=aosmond
Status: ASSIGNED → RESOLVED
Closed: 8 months ago
Resolution: --- → FIXED
Target Milestone: --- → 126 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: