step carefully in decal_nofilter_scale can_truncate_to_fixed_for_decal() is correct to check that we can step fx by dx count-1 times without overflowing, but as written we actually step count times, and sometimes the final fx does overflow. Fuzzers equipped with UBSAN have noticed. There's no real bug here, as we ignore that final fx, but we can step carefully to avoid this with very little impact on code complexity or performance. Bug: chromium:951264 Cq-Include-Trybots: luci.chromium.try:linux-blink-rel Change-Id: I5446dbdffdc5fa873c843c26654894bd79202971 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/207921 Commit-Queue: Mike Klein <mtklein@google.com> Reviewed-by: Florin Malita <fmalita@chromium.org> Auto-Submit: Mike Klein <mtklein@google.com>
diff --git a/src/core/SkBitmapProcState_matrixProcs.cpp b/src/core/SkBitmapProcState_matrixProcs.cpp index bf53d88..aefcd96 100644 --- a/src/core/SkBitmapProcState_matrixProcs.cpp +++ b/src/core/SkBitmapProcState_matrixProcs.cpp
@@ -56,23 +56,24 @@ return SkTFitsIn<int32_t>(lastFx) && (unsigned)SkFixedFloorToInt(SkTo<int32_t>(lastFx)) < max; } - // When not filtering, we store 32-bit y, 16-bit x, 16-bit x, 16-bit x, ... // When filtering we write out 32-bit encodings, pairing 14.4 x0 with 14-bit x1. // The clamp routines may try to fall into one of these unclamped decal fast-paths. // (Only clamp works in the right coordinate space to check for decal.) static void decal_nofilter_scale(uint32_t dst[], SkFixed fx, SkFixed dx, int count) { - for (; count >= 2; count -= 2) { + // can_truncate_to_fixed_for_decal() checked only that stepping fx+=dx count-1 + // times doesn't overflow fx, so we take unusual care not to step count times. + for (; count > 2; count -= 2) { *dst++ = pack_two_shorts( (fx + 0) >> 16, (fx + dx) >> 16); fx += dx+dx; } - auto xx = (uint16_t*)dst; - while (count --> 0) { - *xx++ = SkToU16(fx >> 16); - fx += dx; + SkASSERT(count <= 2); + switch (count) { + case 2: ((uint16_t*)dst)[1] = SkToU16((fx + dx) >> 16); + case 1: ((uint16_t*)dst)[0] = SkToU16((fx + 0) >> 16); } }