Consistently fail writePixels when rowbytes not a multiple of bpp

With skia-review.googlesource.com/c/skia/+/363782, both cpu and gpu
backends would gracefully fail readPixel requests where dst row bytes
wasn't a multiple of dst bpp. It also updated the cpu backend's
writePixels behavior to gracefully reject writePixels requests where
the src row bytes wasn't a multiple of src bpp.

GPU writePixels would not detect this and later trigger an assert
in debug builds in GrConvertPixels (caught by the linked fuzzer bug).

This adds tests to mirror the read pixels bad-row-bytes tests and
updates GrSurfaceContext::writePixels to check src row bytes vs. bpp.
I confirmed it fixes the fuzzer crash.

Bug: chromium:1185266
Change-Id: I7cd8406c65a9ba35a55d695b2f65410a1edd2a19
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/382276
Auto-Submit: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
This commit is contained in:
Michael Ludwig 2021-03-09 15:35:32 -05:00 committed by Skia Commit-Bot
parent 7d195fff45
commit a5c90588b1
3 changed files with 43 additions and 0 deletions

View File

@ -372,6 +372,10 @@ bool GrSurfaceContext::writePixels(GrDirectContext* dContext, GrPixmap src, SkIP
return false;
}
if (src.rowBytes() % src.info().bpp()) {
return false;
}
src = src.clip(this->dimensions(), &pt);
if (!src.hasPixels()) {
return false;

View File

@ -517,6 +517,25 @@ DEF_GPUTEST_FOR_ALL_CONTEXTS(ReadPixels_InvalidRowBytes_Gpu, reporter, ctxInfo)
}
}
DEF_GPUTEST_FOR_ALL_CONTEXTS(WritePixels_InvalidRowBytes_Gpu, reporter, ctxInfo) {
auto dstII = SkImageInfo::Make({10, 10}, kRGBA_8888_SkColorType, kPremul_SkAlphaType);
auto surf = SkSurface::MakeRenderTarget(ctxInfo.directContext(), SkBudgeted::kYes, dstII);
for (int ct = 0; ct < kLastEnum_SkColorType + 1; ++ct) {
auto colorType = static_cast<SkColorType>(ct);
size_t bpp = SkColorTypeBytesPerPixel(colorType);
if (bpp <= 1) {
continue;
}
auto srcII = dstII.makeColorType(colorType);
size_t badRowBytes = (surf->width() + 1)*bpp - 1;
auto storage = std::make_unique<char[]>(badRowBytes*surf->height());
memset(storage.get(), 0, badRowBytes * surf->height());
// SkSurface::writePixels doesn't report bool, SkCanvas's does.
REPORTER_ASSERT(reporter,
!surf->getCanvas()->writePixels(srcII, storage.get(), badRowBytes, 0, 0));
}
}
namespace {
struct AsyncContext {
bool fCalled = false;

View File

@ -570,3 +570,23 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(WritePixelsPendingIO, reporter, ctxInfo) {
REPORTER_ASSERT(reporter, isCorrect);
}
DEF_TEST(WritePixels_InvalidRowBytes, reporter) {
auto dstII = SkImageInfo::Make({10, 10}, kRGBA_8888_SkColorType, kPremul_SkAlphaType);
auto surf = SkSurface::MakeRaster(dstII);
for (int ct = 0; ct < kLastEnum_SkColorType + 1; ++ct) {
auto colorType = static_cast<SkColorType>(ct);
size_t bpp = SkColorTypeBytesPerPixel(colorType);
if (bpp <= 1) {
continue;
}
auto srcII = dstII.makeColorType(colorType);
size_t badRowBytes = (surf->width() + 1)*bpp - 1;
auto storage = std::make_unique<char[]>(badRowBytes*surf->height());
memset(storage.get(), 0, badRowBytes * surf->height());
// SkSurface::writePixels doesn't report bool, SkCanvas's does.
REPORTER_ASSERT(reporter,
!surf->getCanvas()->writePixels(srcII, storage.get(), badRowBytes, 0, 0));
}
}