1287515 - int-overflow: 0xFFFFFFFFFFFFFFFF bytes requested in [@SkDashPathEffect::asPoints]

Status: RESOLVED FIXED

(Core :: Graphics: Canvas2D, defect)

Description

[Tyson Smith [:tsmith]]

Attachment: log.txt

Repros on non ASan and non debug builds.

Log from an ASan build:
==20174==WARNING: AddressSanitizer failed to allocate 0xffffffffffffffff bytes
==20174==AddressSanitizer's allocator is terminating the process instead of returning 0
==20174==If you don't like this behavior set allocator_may_return_null=1
==20174==AddressSanitizer CHECK failed: /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/sanitizer_common/sanitizer_allocator.cc:147 "((0)) != (0)" (0x0, 0x0)
    #0 0x4bb54d in __asan::AsanCheckFailed(char const*, int, char const*, unsigned long long, unsigned long long) /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_rtl.cc:67:3
    #1 0x4bf3e1 in __sanitizer::CheckFailed(char const*, int, char const*, unsigned long long, unsigned long long) /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/sanitizer_common/sanitizer_common.cc:159:5
    #2 0x4bdc53 in __sanitizer::ReportAllocatorCannotReturnNull() /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/sanitizer_common/sanitizer_allocator.cc:147:3
    #3 0x41d238 in ReturnNullOrDie /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/../sanitizer_common/sanitizer_allocator.h:1317:5
    #4 0x41d238 in __asan::Allocator::Allocate(unsigned long, unsigned long, __sanitizer::BufferedStackTrace*, __asan::AllocType, bool) /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_allocator.cc:359
    #5 0x4b2534 in malloc /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:53:10
    #6 0x4e06bd in moz_xmalloc /builds/slave/m-in-l64-asan-0000000000000000/build/src/memory/mozalloc/mozalloc.cpp:83:17
    #7 0x7fa080257000 in operator new[] /builds/slave/m-in-l64-asan-0000000000000000/build/src/obj-firefox/dist/include/mozilla/mozalloc.h:205:12
    #8 0x7fa080257000 in SkDashPathEffect::asPoints(SkPathEffect::PointData*, SkPath const&, SkStrokeRec const&, SkMatrix const&, SkRect const*) const /builds/slave/m-in-l64-asan-0000000000000000/build/src/gfx/skia/skia/src/effects/SkDashPathEffect.cpp:264
    #9 0x7fa0804d46fa in SkDraw::drawPoints(SkCanvas::PointMode, unsigned long, SkPoint const*, SkPaint const&, bool) const /builds/slave/m-in-l64-asan-0000000000000000/build/src/gfx/skia/skia/src/core/SkDraw.cpp:647:25
    #10 0x7fa08021ae8a in SkCanvas::onDrawPoints(SkCanvas::PointMode, unsigned long, SkPoint const*, SkPaint const&) /builds/slave/m-in-l64-asan-0000000000000000/build/src/gfx/skia/skia/src/core/SkCanvas.cpp:2082:9
    #11 0x7fa08022b7d7 in drawPoints /builds/slave/m-in-l64-asan-0000000000000000/build/src/gfx/skia/skia/src/core/SkCanvas.cpp:1902:5
    #12 0x7fa08022b7d7 in SkCanvas::drawLine(float, float, float, float, SkPaint const&) /builds/slave/m-in-l64-asan-0000000000000000/build/src/gfx/skia/skia/src/core/SkCanvas.cpp:2827
    #13 0x7fa078a2730e in mozilla::gfx::DrawTargetSkia::StrokeLine(mozilla::gfx::PointTyped<mozilla::gfx::UnknownUnits, float> const&, mozilla::gfx::PointTyped<mozilla::gfx::UnknownUnits, float> const&, mozilla::gfx::Pattern const&, mozilla::gfx::StrokeOptions const&, mozilla::gfx::DrawOptions const&) /builds/slave/m-in-l64-asan-0000000000000000/build/src/gfx/2d/DrawTargetSkia.cpp:577:3
    #14 0x7fa07b6486a4 in mozilla::dom::CanvasRenderingContext2D::StrokeRect(double, double, double, double) /builds/slave/m-in-l64-asan-0000000000000000/build/src/dom/canvas/CanvasRenderingContext2D.cpp:2872:5
...
see log.txt for full log.

Comment 1

[Tyson Smith [:tsmith]]

Attachment: test_case.html

Comment 2

[Peter Chang[:pchang]]

Based on log, I think this was reproduced with asan build on linux.
Vincent, are you able to reproduce this?

Comment 3

[Lee Salzman [:lsalzman]]

Attachment: limit the number of points in SkDashPathEffect::asPoints

SkScalarFloorToInt(len2 / fIntervalLength) was overflowing the resulting int, causing it to go negative. When that int is cast to a size_t, further havoc ensues. Since this is a fast-path for dash rendering, we just want to bail out here when the conditions aren't sane.

Comment 4

[Mason Chang [Inactive] [:mchang]]

Comment on attachment 8772854 [details] [diff] [review]
limit the number of points in SkDashPathEffect::asPoints

Review of attachment 8772854 [details] [diff] [review]:
-----------------------------------------------------------------

Can we also check in the test case here as a crash test please?

Comment 5

[Lee Salzman [:lsalzman]]

Attachment: limit the number of points in SkDashPathEffect::asPoints

Now with more crashtestiness...

Comment 6

[Mason Chang [Inactive] [:mchang]]

Comment on attachment 8772913 [details] [diff] [review]
limit the number of points in SkDashPathEffect::asPoints

Review of attachment 8772913 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!

Comment 7

[Lee Salzman [:lsalzman]]

Upstream Skia bug report: https://codereview.chromium.org/2165013002/

Comment 8

[Peter Chang[:pchang]]

Clear ni since lee is working on this.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4bc6b6fe87b6
limit the number of points in SkDashPathEffect::asPoints. r=mchang

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0bfa680a22ba
fix crashtest for d2d. r=me

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/00833f7ba134
fix android crashtest bustage. r=me

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/4bc6b6fe87b6
https://hg.mozilla.org/mozilla-central/rev/0bfa680a22ba
https://hg.mozilla.org/mozilla-central/rev/00833f7ba134