Closed Bug 1339637 Opened 3 years ago Closed 3 years ago

skia: signed integer overflow in SkClampRange::init()

Categories

(Core :: Graphics, defect, P1)

defect

Tracking

()

RESOLVED FIXED
mozilla54
Tracking Status
firefox54 --- fixed

People

(Reporter: tsmith, Assigned: lsalzman)

Details

(Keywords: csectype-intoverflow, Whiteboard: [gfx-noted])

Attachments

(1 file)

This seems to fix a few assertions that I was coming across as well.

Logged as: https://bugs.chromium.org/p/skia/issues/detail?id=6219

Fixed upstream by: https://chromium.googlesource.com/skia/+/d9569664fe76068adcc87d7a029350f5dbdbc4a6

src/effects/gradients/SkClampRange.cpp:98:21: runtime error: signed integer overflow: -9223372036854775808 + -3368429952742457344 cannot be represented in type 'long'
    #0 0x28528c6 in SkClampRange::init(long, long, int, int, int) src/effects/gradients/SkClampRange.cpp:98:21
    #1 0x13f5066 in (anonymous namespace)::shadeSpan_linear_clamp(int (*)(int), long, long, unsigned int*, unsigned int const*, int, int) src/effects/gradients/SkLinearGradient.cpp:280:5
    #2 0x13f1ea9 in SkLinearGradient::LinearGradientContext::shadeSpan(int, int, unsigned int*, int) src/effects/gradients/SkLinearGradient.cpp:383:9
    #3 0x239829a in SkARGB32_Shader_Blitter::blitRect(int, int, int, int) src/core/SkBlitter_ARGB32.cpp:460:17
    #4 0x10391bb in SkScan::FillIRect(SkIRect const&, SkRegion const*, SkBlitter*) src/core/SkScan.cpp:50:13
    #5 0x103a276 in SkScan::FillIRect(SkIRect const&, SkRasterClip const&, SkBlitter*) src/core/SkScan.cpp:80:9
    #6 0xcc266f in SkDraw::drawPaint(SkPaint const&) const src/core/SkDraw.cpp:294:5
    #7 0x900fde in SkCanvas::internalDrawPaint(SkPaint const&) src/core/SkCanvas.cpp:2098:9
    #8 0x90086c in SkCanvas::onDrawPaint(SkPaint const&) src/core/SkCanvas.cpp:2091:5
    #9 0x5a123d in fuzzLinearGradient(Fuzz*) fuzz/FuzzGradients.cpp:83:9
    #10 0x5a5832 in fuzz_Gradients(Fuzz*) fuzz/FuzzGradients.cpp:205:13
    #11 0x5b1136 in fuzz_api(sk_sp<SkData>) fuzz/fuzz.cpp:147:13
    #12 0x5b1136 in fuzz_file(char const*) fuzz/fuzz.cpp:88
    #13 0x5af60e in main fuzz/fuzz.cpp:60:19
    #14 0x7fa4cbb6d82f in __libc_start_main /build/glibc-t3gR2i/glibc-2.23/csu/../csu/libc-start.c:291
    #15 0x49fd48 in _start (fuzz_ubsan+0x49fd48)
Could cherry pick this change
Assignee: nobody → lsalzman
Just a straight backport of upstream Skia fix: https://chromium.googlesource.com/skia/+/d9569664fe76068adcc87d7a029350f5dbdbc4a6
Attachment #8839574 - Flags: review?(jmuizelaar)
Priority: -- → P1
Whiteboard: [gfx-noted]
Attachment #8839574 - Flags: review?(jmuizelaar) → review+
Pushed by lsalzman@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8568a8c74f30
fix overflow in SkClampRange::init(). r=jrmuizel
https://hg.mozilla.org/mozilla-central/rev/8568a8c74f30
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Can you show some of the testcases that caused the assertion so we can assess quickly whether this would benefit from uplift?
Flags: needinfo?(twsmith)
The only test cases that I have are attached to the upstream bug report and the fix. The addition to tests/GradientTest.cpp (diff[1]) will likely be the most helpful.

[1] https://chromium.googlesource.com/skia/+/d9569664fe76068adcc87d7a029350f5dbdbc4a6%5E%21/#F1
Flags: needinfo?(twsmith)
(In reply to Tyson Smith [:tsmith] from comment #6)
> The only test cases that I have are attached to the upstream bug report and
> the fix. The addition to tests/GradientTest.cpp (diff[1]) will likely be the
> most helpful.
> 
> [1]
> https://chromium.googlesource.com/skia/+/
> d9569664fe76068adcc87d7a029350f5dbdbc4a6%5E%21/#F1

Unless we can prove this can actually be done via an html or javascript testcase, I am not sure this warrants an uplift at all.
Flags: needinfo?(ryanvm)
(In reply to Lee Salzman [:lsalzman] from comment #7)
> Unless we can prove this can actually be done via an html or javascript
> testcase, I am not sure this warrants an uplift at all.

Sounds like a reasonable position to me.
Flags: needinfo?(ryanvm)
You need to log in before you can comment on or make changes to this bug.