Skip to content

Commit e262e68

Browse files
brianosmanSkCQ
authored andcommitted
Limit surfaces/images to 2GB in size
The CPU blitters use signed 32-bit offsets when gathering source pixels, so any image larger than that can't be fully indexed. Instead, we'd wrap around and sample from invalid memory. This does put a new (smaller) limit on valid image sizes, but it seems unlikely to impact any client. Bug: chromium:1264705 Change-Id: I21b088c11b49e6410b1f237cbfb6b1639cb22ca0 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/471764 Reviewed-by: Herb Derby <[email protected]> Reviewed-by: Brian Salomon <[email protected]> Commit-Queue: Brian Osman <[email protected]>
1 parent fe5b133 commit e262e68

File tree

3 files changed

+20
-1
lines changed

3 files changed

+20
-1
lines changed

RELEASE_NOTES.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ Milestone 98
1212
* GrBackendSemaphore only includes methods that match the GPU backend that Skia was compiled for.
1313
For example, initVulkan and vkSemaphore are not defined unless the Vulkan backend is compiled
1414
into Skia.
15+
* Surfaces and images are now limited to just under 2GB of total size. Previously, larger images
16+
could be created, but the CPU backend would fail to index them correctly.
1517

1618

1719
Milestone 97

src/core/SkImageInfo.cpp

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,12 @@ size_t SkImageInfo::computeByteSize(size_t rowBytes) const {
6363
SkSafeMath safe;
6464
size_t bytes = safe.add(safe.mul(safe.addInt(this->height(), -1), rowBytes),
6565
safe.mul(this->width(), this->bytesPerPixel()));
66-
return safe.ok() ? bytes : SIZE_MAX;
66+
67+
// The CPU backend implements some memory operations on images using instructions that take a
68+
// signed 32-bit offset from the base. If we ever make an image larger than that, overflow can
69+
// cause us to read/write memory that starts 2GB *before* the buffer. (crbug.com/1264705)
70+
constexpr size_t kMaxSigned32BitSize = SK_MaxS32;
71+
return (safe.ok() && (bytes <= kMaxSigned32BitSize)) ? bytes : SIZE_MAX;
6772
}
6873

6974
SkImageInfo SkImageInfo::MakeS32(int width, int height, SkAlphaType at) {

tests/ImageFilterTest.cpp

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2057,3 +2057,15 @@ DEF_TEST(PictureImageSourceBounds, reporter) {
20572057
SkImageFilter::kReverse_MapDirection,
20582058
&input));
20592059
}
2060+
2061+
DEF_TEST(DropShadowImageFilter_Huge, reporter) {
2062+
// Successful if it doesn't crash or trigger ASAN. (crbug.com/1264705)
2063+
auto surf = SkSurface::MakeRasterN32Premul(300, 150);
2064+
2065+
SkPaint paint;
2066+
paint.setImageFilter(SkImageFilters::DropShadowOnly(
2067+
0.0f, 0.437009f, 14129.6f, 14129.6f, SK_ColorGRAY, nullptr));
2068+
2069+
surf->getCanvas()->saveLayer(nullptr, &paint);
2070+
surf->getCanvas()->restore();
2071+
}

0 commit comments

Comments
 (0)