From 595e3a152ff2912a950defd0ef4b5f659135b03a Mon Sep 17 00:00:00 2001 From: Nicolas Silva Date: Wed, 18 Nov 2015 16:59:11 +0100 Subject: [PATCH] Bug 1222809 - Don't try to allocate unreasonably large textures. r=Bas, a=sylvestre --- gfx/2d/2D.h | 25 ++++++++++-- gfx/2d/Factory.cpp | 67 ++++++++++++++++++++++++++++----- gfx/layers/ImageDataSerializer.cpp | 21 ++++++----- gfx/layers/YCbCrImageDataSerializer.cpp | 7 ++++ gfx/layers/client/TextureClient.cpp | 12 ++++++ gfx/thebes/gfxPlatform.cpp | 15 ++++++-- gfx/thebes/gfxPrefs.h | 3 ++ 7 files changed, 124 insertions(+), 26 deletions(-) diff --git a/gfx/2d/2D.h b/gfx/2d/2D.h index cf35bb2..b1e0e3e 100644 --- a/gfx/2d/2D.h +++ b/gfx/2d/2D.h @@ -1082,22 +1082,41 @@ struct TileSet size_t mTileCount; }; +struct Config { + LogForwarder* mLogForwarder; + int32_t mMaxTextureSize; + int32_t mMaxAllocSize; + + Config() + : mLogForwarder(nullptr) + , mMaxTextureSize(8192) + , mMaxAllocSize(52000000) + {} +}; + class GFX2D_API Factory { public: + static void Init(const Config& aConfig); + static void ShutDown(); + static bool HasSSE2(); /** Make sure that the given dimensions don't overflow a 32-bit signed int * using 4 bytes per pixel; optionally, make sure that either dimension * doesn't exceed the given limit. */ - static bool CheckSurfaceSize(const IntSize &sz, int32_t limit = 0); + static bool CheckSurfaceSize(const IntSize &sz, + int32_t limit = 0, + int32_t allocLimit = 0); /** Make sure the given dimension satisfies the CheckSurfaceSize and is * within 8k limit. The 8k value is chosen a bit randomly. */ static bool ReasonableSurfaceSize(const IntSize &aSize); + static bool AllowedSurfaceSize(const IntSize &aSize); + static TemporaryRef CreateDrawTargetForCairoSurface(cairo_surface_t* aSurface, const IntSize& aSize, SurfaceFormat* aFormat = nullptr); static TemporaryRef @@ -1171,10 +1190,10 @@ public: static uint32_t GetMaxSurfaceSize(BackendType aType); - static LogForwarder* GetLogForwarder() { return mLogForwarder; } + static LogForwarder* GetLogForwarder() { return sConfig ? sConfig->mLogForwarder : nullptr; } private: - static LogForwarder* mLogForwarder; + static Config* sConfig; public: #ifdef USE_SKIA_GPU diff --git a/gfx/2d/Factory.cpp b/gfx/2d/Factory.cpp index 948d3c3..6750c28 100644 --- a/gfx/2d/Factory.cpp +++ b/gfx/2d/Factory.cpp @@ -188,6 +188,35 @@ ID2D1Device *Factory::mD2D1Device; DrawEventRecorder *Factory::mRecorder; +mozilla::gfx::Config* Factory::sConfig = nullptr; + +void +Factory::Init(const Config& aConfig) +{ + MOZ_ASSERT(!sConfig); + sConfig = new Config(aConfig); + + // Make sure we don't completely break rendering because of a typo in the + // pref or whatnot. + const int32_t kMinAllocPref = 10000000; + const int32_t kMinSizePref = 2048; + if (sConfig->mMaxAllocSize < kMinAllocPref) { + sConfig->mMaxAllocSize = kMinAllocPref; + } + if (sConfig->mMaxTextureSize < kMinSizePref) { + sConfig->mMaxTextureSize = kMinSizePref; + } +} + +void +Factory::ShutDown() +{ + if (sConfig) { + delete sConfig; + sConfig = nullptr; + } +} + bool Factory::HasSSE2() { @@ -222,11 +251,25 @@ inline int LoggerOptionsBasedOnSize(const IntSize& aSize) bool Factory::ReasonableSurfaceSize(const IntSize &aSize) { - return Factory::CheckSurfaceSize(aSize,8192); + return Factory::CheckSurfaceSize(aSize, 8192); +} + +bool +Factory::AllowedSurfaceSize(const IntSize &aSize) +{ + if (sConfig) { + return Factory::CheckSurfaceSize(aSize, + sConfig->mMaxTextureSize, + sConfig->mMaxAllocSize); + } + + return CheckSurfaceSize(aSize); } bool -Factory::CheckSurfaceSize(const IntSize &sz, int32_t limit) +Factory::CheckSurfaceSize(const IntSize &sz, + int32_t extentLimit, + int32_t allocLimit) { if (sz.width <= 0 || sz.height <= 0) { gfxDebug() << "Surface width or height <= 0!"; @@ -234,8 +277,8 @@ Factory::CheckSurfaceSize(const IntSize &sz, int32_t limit) } // reject images with sides bigger than limit - if (limit && (sz.width > limit || sz.height > limit)) { - gfxDebug() << "Surface size too large (exceeds caller's limit)!"; + if (extentLimit && (sz.width > extentLimit || sz.height > extentLimit)) { + gfxDebug() << "Surface size too large (exceeds extent limit)!"; return false; } @@ -267,13 +310,18 @@ Factory::CheckSurfaceSize(const IntSize &sz, int32_t limit) return false; } + if (allocLimit && allocLimit < numBytes.value()) { + gfxDebug() << "Surface size too large (exceeds allocation limit)!"; + return false; + } + return true; } TemporaryRef Factory::CreateDrawTarget(BackendType aBackend, const IntSize &aSize, SurfaceFormat aFormat) { - if (!CheckSurfaceSize(aSize)) { + if (!AllowedSurfaceSize(aSize)) { gfxCriticalError(LoggerOptionsBasedOnSize(aSize)) << "Failed to allocate a surface due to invalid size " << aSize; return nullptr; } @@ -364,7 +412,7 @@ Factory::CreateDrawTargetForData(BackendType aBackend, SurfaceFormat aFormat) { MOZ_ASSERT(aData); - if (!CheckSurfaceSize(aSize)) { + if (!AllowedSurfaceSize(aSize)) { gfxCriticalError(LoggerOptionsBasedOnSize(aSize)) << "Failed to allocate a surface due to invalid size " << aSize; return nullptr; } @@ -835,7 +883,7 @@ Factory::CreateDataSourceSurface(const IntSize &aSize, SurfaceFormat aFormat, bool aZero) { - if (!CheckSurfaceSize(aSize)) { + if (!AllowedSurfaceSize(aSize)) { gfxCriticalError(LoggerOptionsBasedOnSize(aSize)) << "Failed to allocate a surface due to invalid size " << aSize; return nullptr; } @@ -881,14 +929,13 @@ Factory::SetGlobalEventRecorder(DrawEventRecorder *aRecorder) mRecorder = aRecorder; } -LogForwarder* Factory::mLogForwarder = nullptr; - // static void Factory::SetLogForwarder(LogForwarder* aLogFwd) { - mLogForwarder = aLogFwd; + sConfig->mLogForwarder = aLogFwd; } + // static void CriticalLogger::OutputMessage(const std::string &aString, diff --git a/gfx/layers/ImageDataSerializer.cpp b/gfx/layers/ImageDataSerializer.cpp index 5dd6aca..331dd04 100644 --- a/gfx/layers/ImageDataSerializer.cpp +++ b/gfx/layers/ImageDataSerializer.cpp @@ -84,21 +84,23 @@ ImageDataSerializerBase::ComputeMinBufferSize(IntSize aSize, SurfaceFormat aFormat) { MOZ_ASSERT(aSize.height >= 0 && aSize.width >= 0); - if (aSize.height <= 0 || aSize.width <= 0) { - gfxDebug() << "Non-positive image buffer size request " << aSize.width << "x" << aSize.height; + + // This takes care of checking whether there could be overflow + // with enough margin for the metadata. + if (!gfx::Factory::AllowedSurfaceSize(aSize)) { return 0; } - CheckedInt bufsize = ComputeStride(aFormat, aSize.width); - bufsize *= aSize.height; + int32_t bufsize = GetAlignedStride<16>(ComputeStride(aFormat, aSize.width) + * aSize.height) + + SurfaceBufferInfo::GetOffset(); - if (!bufsize.isValid() || bufsize.value() <= 0) { - gfxDebug() << "Buffer size overflow " << aSize.width << "x" << aSize.height; + if (bufsize < 0) { + // This should not be possible thanks to Factory::AllowedSurfaceSize return 0; } - return SurfaceBufferInfo::GetOffset() - + GetAlignedStride<16>(bufsize.value()); + return bufsize; } void @@ -114,7 +116,8 @@ ImageDataSerializerBase::Validate() } size_t requiredSize = ComputeMinBufferSize(IntSize(info->width, info->height), info->format); - mIsValid = requiredSize <= mDataSize; + + mIsValid = !!requiredSize && requiredSize <= mDataSize; } uint8_t* diff --git a/gfx/layers/YCbCrImageDataSerializer.cpp b/gfx/layers/YCbCrImageDataSerializer.cpp index c8e148d..05f5ab2 100644 --- a/gfx/layers/YCbCrImageDataSerializer.cpp +++ b/gfx/layers/YCbCrImageDataSerializer.cpp @@ -150,6 +150,13 @@ YCbCrImageDataDeserializerBase::ComputeMinBufferSize(const gfx::IntSize& aYSize, gfxDebug() << "Non-positive YCbCr buffer size request " << aYSize.height << "x" << aYSize.width << ", " << aCbCrSize.height << "x" << aCbCrSize.width; return 0; } + + if (!gfx::Factory::AllowedSurfaceSize(aYSize) || + aCbCrSize.width > aYSize.width || + aCbCrSize.height > aYSize.height) { + return 0; + } + return ComputeOffset(aYSize.height, aYStride) + 2 * ComputeOffset(aCbCrSize.height, aCbCrStride) + MOZ_ALIGN_WORD(sizeof(YCbCrBufferInfo)); diff --git a/gfx/layers/client/TextureClient.cpp b/gfx/layers/client/TextureClient.cpp index 9b45ca0..6ae7cbf 100644 --- a/gfx/layers/client/TextureClient.cpp +++ b/gfx/layers/client/TextureClient.cpp @@ -315,6 +315,10 @@ TextureClient::CreateForDrawing(ISurfaceAllocator* aAllocator, aMoz2DBackend = gfxPlatform::GetPlatform()->GetContentBackend(); } + if (!gfx::Factory::AllowedSurfaceSize(aSize)) { + return nullptr; + } + RefPtr texture; #if defined(MOZ_WIDGET_GONK) || defined(XP_WIN) @@ -415,6 +419,10 @@ TextureClient::CreateForRawBufferAccess(ISurfaceAllocator* aAllocator, TextureFlags aTextureFlags, TextureAllocationFlags aAllocFlags) { + if (!gfx::Factory::AllowedSurfaceSize(aSize)) { + return nullptr; + } + RefPtr texture = CreateBufferTextureClient(aAllocator, aFormat, aTextureFlags, aMoz2DBackend); @@ -434,6 +442,10 @@ TextureClient::CreateForYCbCr(ISurfaceAllocator* aAllocator, StereoMode aStereoMode, TextureFlags aTextureFlags) { + if (!gfx::Factory::AllowedSurfaceSize(aYSize)) { + return nullptr; + } + RefPtr texture; if (aAllocator->IsSameProcess()) { texture = new MemoryTextureClient(aAllocator, gfx::SurfaceFormat::YUV, diff --git a/gfx/thebes/gfxPlatform.cpp b/gfx/thebes/gfxPlatform.cpp index 41e4b0c..209a0a8 100644 --- a/gfx/thebes/gfxPlatform.cpp +++ b/gfx/thebes/gfxPlatform.cpp @@ -458,13 +458,18 @@ gfxPlatform::Init() } gEverInitialized = true; - CrashStatsLogForwarder* logForwarder = new CrashStatsLogForwarder("GraphicsCriticalError"); - mozilla::gfx::Factory::SetLogForwarder(logForwarder); - // Initialize the preferences by creating the singleton. gfxPrefs::GetSingleton(); - logForwarder->SetCircularBufferSize(gfxPrefs::GfxLoggingCrashLength()); + auto fwd = new CrashStatsLogForwarder("GraphicsCriticalError"); + fwd->SetCircularBufferSize(gfxPrefs::GfxLoggingCrashLength()); + + mozilla::gfx::Config cfg; + cfg.mLogForwarder = fwd; + cfg.mMaxTextureSize = gfxPrefs::MaxTextureSize(); + cfg.mMaxAllocSize = gfxPrefs::MaxAllocSize(); + + gfx::Factory::Init(cfg); gGfxPlatformPrefsLock = new Mutex("gfxPlatform::gGfxPlatformPrefsLock"); @@ -641,6 +646,8 @@ gfxPlatform::Shutdown() delete mozilla::gfx::Factory::GetLogForwarder(); mozilla::gfx::Factory::SetLogForwarder(nullptr); + gfx::Factory::ShutDown(); + delete gGfxPlatformPrefsLock; gfxPrefs::DestroySingleton(); diff --git a/gfx/thebes/gfxPrefs.h b/gfx/thebes/gfxPrefs.h index b7a5fb9..038e1ff 100644 --- a/gfx/thebes/gfxPrefs.h +++ b/gfx/thebes/gfxPrefs.h @@ -209,6 +209,9 @@ private: DECL_GFX_PREF(Live, "gfx.layerscope.port", LayerScopePort, int32_t, 23456); // Note that "gfx.logging.level" is defined in Logging.h DECL_GFX_PREF(Once, "gfx.logging.crash.length", GfxLoggingCrashLength, uint32_t, 6); + // The maximums here are quite conservative, we can tighten them if problems show up. + DECL_GFX_PREF(Once, "gfx.max-alloc-size", MaxAllocSize, int32_t, (int32_t)500000000); + DECL_GFX_PREF(Once, "gfx.max-texture-size", MaxTextureSize, int32_t, (int32_t)32767); DECL_GFX_PREF(Live, "gfx.perf-warnings.enabled", PerfWarnings, bool, false); DECL_GFX_PREF(Once, "gfx.work-around-driver-bugs", WorkAroundDriverBugs, bool, true); -- 2.6.3