From f90d03e8578054715c1f7b64d1fde3c38887c062 Mon Sep 17 00:00:00 2001 From: Lee Salzman Date: Wed, 28 Mar 2018 00:34:13 -0400 Subject: Bug 1441941 - Limit allocations in SkTDArray. r=jrmuizel, a=RyanVM --- gfx/skia/skia/include/core/SkTypes.h | 6 ++ gfx/skia/skia/include/private/SkTDArray.h | 8 ++- gfx/skia/skia/src/core/SkMallocPixelRef.cpp | 13 ++++ gfx/skia/skia/src/core/SkMath.cpp | 15 ++++ gfx/skia/skia/src/core/SkSafeMath.h | 106 ++++++++++++++++++++++++++++ 5 files changed, 145 insertions(+), 3 deletions(-) create mode 100644 gfx/skia/skia/src/core/SkSafeMath.h diff --git a/gfx/skia/skia/include/core/SkTypes.h b/gfx/skia/skia/include/core/SkTypes.h index 0cef8a125..f45c3c2b7 100644 --- a/gfx/skia/skia/include/core/SkTypes.h +++ b/gfx/skia/skia/include/core/SkTypes.h @@ -109,6 +109,11 @@ SK_API extern void* sk_calloc(size_t size); */ SK_API extern void* sk_calloc_throw(size_t size); +// Performs a safe multiply count * elemSize, checking for overflow +SK_API extern void* sk_calloc_throw(size_t count, size_t elemSize); +SK_API extern void* sk_malloc_throw(size_t count, size_t elemSize); +SK_API extern void* sk_realloc_throw(void* buffer, size_t count, size_t elemSize); + // bzero is safer than memset, but we can't rely on it, so... sk_bzero() static inline void sk_bzero(void* buffer, size_t size) { // Please c.f. sk_careful_memcpy. It's undefined behavior to call memset(null, 0, 0). @@ -295,6 +300,7 @@ template D SkTo(S s) { #define SK_MaxU32 0xFFFFFFFF #define SK_MinU32 0 #define SK_NaN32 ((int) (1U << 31)) +#define SK_MaxSizeT SIZE_MAX /** Returns true if the value can be represented with signed 16bits */ diff --git a/gfx/skia/skia/include/private/SkTDArray.h b/gfx/skia/skia/include/private/SkTDArray.h index f71d35700..a46a05e9d 100644 --- a/gfx/skia/skia/include/private/SkTDArray.h +++ b/gfx/skia/skia/include/private/SkTDArray.h @@ -21,7 +21,7 @@ public: fReserve = fCount = 0; fArray = NULL; if (count) { - fArray = (T*)sk_malloc_throw(count * sizeof(T)); + fArray = (T*)sk_malloc_throw(count, sizeof(T)); memcpy(fArray, src, sizeof(T) * count); fReserve = fCount = count; } @@ -346,7 +346,7 @@ public: void shrinkToFit() { fReserve = fCount; - fArray = (T*)sk_realloc_throw(fArray, fReserve * sizeof(T)); + fArray = (T*)sk_realloc_throw(fArray, fReserve, sizeof(T)); } private: @@ -359,6 +359,7 @@ private: * This is the same as calling setCount(count() + delta). */ void adjustCount(int delta) { + SkASSERT_RELEASE(fCount <= std::numeric_limits::max() - delta); this->setCount(fCount + delta); } @@ -372,9 +373,10 @@ private: */ void resizeStorageToAtLeast(int count) { SkASSERT(count > fReserve); + SkASSERT_RELEASE(count <= std::numeric_limits::max() - std::numeric_limits::max() / 5 - 4); fReserve = count + 4; fReserve += fReserve / 4; - fArray = (T*)sk_realloc_throw(fArray, fReserve * sizeof(T)); + fArray = (T*)sk_realloc_throw(fArray, fReserve, sizeof(T)); } }; diff --git a/gfx/skia/skia/src/core/SkMallocPixelRef.cpp b/gfx/skia/skia/src/core/SkMallocPixelRef.cpp index fffc04484..8db704fa4 100644 --- a/gfx/skia/skia/src/core/SkMallocPixelRef.cpp +++ b/gfx/skia/skia/src/core/SkMallocPixelRef.cpp @@ -8,8 +8,21 @@ #include "SkMallocPixelRef.h" #include "SkBitmap.h" #include "SkReadBuffer.h" +#include "SkSafeMath.h" #include "SkWriteBuffer.h" +void* sk_calloc_throw(size_t count, size_t elemSize) { + return sk_calloc_throw(SkSafeMath::Mul(count, elemSize)); +} + +void* sk_malloc_throw(size_t count, size_t elemSize) { + return sk_malloc_throw(SkSafeMath::Mul(count, elemSize)); +} + +void* sk_realloc_throw(void* buffer, size_t count, size_t elemSize) { + return sk_realloc_throw(buffer, SkSafeMath::Mul(count, elemSize)); +} + // assumes ptr was allocated via sk_malloc static void sk_free_releaseproc(void* ptr, void*) { sk_free(ptr); diff --git a/gfx/skia/skia/src/core/SkMath.cpp b/gfx/skia/skia/src/core/SkMath.cpp index 6eff790c8..84796c522 100644 --- a/gfx/skia/skia/src/core/SkMath.cpp +++ b/gfx/skia/skia/src/core/SkMath.cpp @@ -84,3 +84,18 @@ float SkScalarSinCos(float radians, float* cosValue) { } return sinValue; } + +/////////////////////////////////////////////////////////////////////////////////////////////////// + +size_t SkSafeMath::Add(size_t x, size_t y) { + SkSafeMath tmp; + size_t sum = tmp.add(x, y); + return tmp.ok() ? sum : SK_MaxSizeT; +} + +size_t SkSafeMath::Mul(size_t x, size_t y) { + SkSafeMath tmp; + size_t prod = tmp.mul(x, y); + return tmp.ok() ? prod : SK_MaxSizeT; +} + diff --git a/gfx/skia/skia/src/core/SkSafeMath.h b/gfx/skia/skia/src/core/SkSafeMath.h new file mode 100644 index 000000000..0bc0fbfac --- /dev/null +++ b/gfx/skia/skia/src/core/SkSafeMath.h @@ -0,0 +1,106 @@ +/* + * Copyright 2017 Google Inc. + * + * Use of this source code is governed by a BSD-style license that can be + * found in the LICENSE file. + */ + +#ifndef SkSafeMath_DEFINED +#define SkSafeMath_DEFINED + +#include "SkTypes.h" + +// SkSafeMath always check that a series of operations do not overflow. +// This must be correct for all platforms, because this is a check for safety at runtime. + +class SkSafeMath { +public: + SkSafeMath() = default; + + bool ok() const { return fOK; } + explicit operator bool() const { return fOK; } + + size_t mul(size_t x, size_t y) { + return sizeof(size_t) == sizeof(uint64_t) ? mul64(x, y) : mul32(x, y); + } + + size_t add(size_t x, size_t y) { + size_t result = x + y; + fOK &= result >= x; + return result; + } + + /** + * Return a + b, unless this result is an overflow/underflow. In those cases, fOK will + * be set to false, and it is undefined what this returns. + */ + int addInt(int a, int b) { + if (b < 0 && a < std::numeric_limits::min() - b) { + fOK = false; + return a; + } else if (b > 0 && a > std::numeric_limits::max() - b) { + fOK = false; + return a; + } + return a + b; + } + + size_t alignUp(size_t x, size_t alignment) { + SkASSERT(alignment && !(alignment & (alignment - 1))); + return add(x, alignment - 1) & ~(alignment - 1); + } + + template T castTo(size_t value) { + if (!SkTFitsIn(value)) { + fOK = false; + } + return static_cast(value); + } + + // These saturate to their results + static size_t Add(size_t x, size_t y); + static size_t Mul(size_t x, size_t y); + static size_t Align4(size_t x) { + SkSafeMath safe; + return safe.alignUp(x, 4); + } + +private: + uint32_t mul32(uint32_t x, uint32_t y) { + uint64_t bx = x; + uint64_t by = y; + uint64_t result = bx * by; + fOK &= result >> 32 == 0; + return result; + } + + uint64_t mul64(uint64_t x, uint64_t y) { + if (x <= std::numeric_limits::max() >> 32 + && y <= std::numeric_limits::max() >> 32) { + return x * y; + } else { + auto hi = [](uint64_t x) { return x >> 32; }; + auto lo = [](uint64_t x) { return x & 0xFFFFFFFF; }; + + uint64_t lx_ly = lo(x) * lo(y); + uint64_t hx_ly = hi(x) * lo(y); + uint64_t lx_hy = lo(x) * hi(y); + uint64_t hx_hy = hi(x) * hi(y); + uint64_t result = 0; + result = this->add(lx_ly, (hx_ly << 32)); + result = this->add(result, (lx_hy << 32)); + fOK &= (hx_hy + (hx_ly >> 32) + (lx_hy >> 32)) == 0; + + #if defined(SK_DEBUG) && defined(__clang__) && defined(__x86_64__) + auto double_check = (unsigned __int128)x * y; + SkASSERT(result == (double_check & 0xFFFFFFFFFFFFFFFF)); + SkASSERT(!fOK || (double_check >> 64 == 0)); + #endif + + return result; + } + } + bool fOK = true; +}; + +#endif//SkSafeMath_DEFINED -- cgit v1.2.3