From 607771b444748a7c1e5a6e4f8220053559870728 Mon Sep 17 00:00:00 2001 From: Nicolas Capens Date: Tue, 8 May 2018 17:20:50 -0400 Subject: [PATCH] Prevent 32-bit numeric overflow on image allocation. We assume the pixels of an image can be addressed with a signed 32-bit offset, including any padding. For a 3D image it's possible to exceed this without exceeding the per-dimension limits. Lowering the per- dimension limit so the allocation is always less than 2 GiB makes them unreasonably small, so instead we must check the total size. Use 1 GiB as the soft limit in OpenGL. Bug chromium:835299 Change-Id: I9c5184002c1710e3923b549f8c21e7f6a516e1c7 Reviewed-on: https://swiftshader-review.googlesource.com/18869 Reviewed-by: Nicolas Capens Tested-by: Nicolas Capens --- src/OpenGL/common/Image.cpp | 29 +++++++++++++++++++++++++++++ src/OpenGL/common/Image.hpp | 2 ++ src/Renderer/Surface.cpp | 17 +++++++++++++---- tests/unittests/unittests.cpp | 27 ++++++++++++++++++++++++++- tests/unittests/unittests.vcxproj.user | 4 ++-- 5 files changed, 72 insertions(+), 7 deletions(-) diff --git a/src/OpenGL/common/Image.cpp b/src/OpenGL/common/Image.cpp index 6f3832222..601876ef5 100644 --- a/src/OpenGL/common/Image.cpp +++ b/src/OpenGL/common/Image.cpp @@ -773,6 +773,10 @@ namespace gl namespace egl { + // We assume the data can be indexed with a signed 32-bit offset, including any padding, + // so we must keep the image size reasonable. 1 GiB ought to be enough for anybody. + enum { IMPLEMENTATION_MAX_IMAGE_SIZE_BYTES = 0x40000000 }; + enum TransferType { Bytes, @@ -1192,24 +1196,49 @@ namespace egl Image *Image::create(Texture *parentTexture, GLsizei width, GLsizei height, GLint internalformat) { + if(size(width, height, 1, 0, 1, internalformat) > IMPLEMENTATION_MAX_IMAGE_SIZE_BYTES) + { + return nullptr; + } + return new ImageImplementation(parentTexture, width, height, internalformat); } Image *Image::create(Texture *parentTexture, GLsizei width, GLsizei height, GLsizei depth, int border, GLint internalformat) { + if(size(width, height, depth, border, 1, internalformat) > IMPLEMENTATION_MAX_IMAGE_SIZE_BYTES) + { + return nullptr; + } + return new ImageImplementation(parentTexture, width, height, depth, border, internalformat); } Image *Image::create(GLsizei width, GLsizei height, GLint internalformat, int pitchP) { + if(size(pitchP, height, 1, 0, 1, internalformat) > IMPLEMENTATION_MAX_IMAGE_SIZE_BYTES) + { + return nullptr; + } + return new ImageImplementation(width, height, internalformat, pitchP); } Image *Image::create(GLsizei width, GLsizei height, GLint internalformat, int multiSampleDepth, bool lockable) { + if(size(width, height, 1, 0, multiSampleDepth, internalformat) > IMPLEMENTATION_MAX_IMAGE_SIZE_BYTES) + { + return nullptr; + } + return new ImageImplementation(width, height, internalformat, multiSampleDepth, lockable); } + size_t Image::size(int width, int height, int depth, int border, int samples, GLint internalformat) + { + return sw::Surface::size(width, height, depth, border, samples, gl::SelectInternalFormat(internalformat)); + } + int ClientBuffer::getWidth() const { return width; diff --git a/src/OpenGL/common/Image.hpp b/src/OpenGL/common/Image.hpp index cb77a4cfc..4fed89396 100644 --- a/src/OpenGL/common/Image.hpp +++ b/src/OpenGL/common/Image.hpp @@ -145,6 +145,8 @@ public: // Back buffer from client buffer static Image *create(const egl::ClientBuffer& clientBuffer); + static size_t size(int width, int height, int depth, int border, int samples, GLint internalformat); + GLsizei getWidth() const { return width; diff --git a/src/Renderer/Surface.cpp b/src/Renderer/Surface.cpp index 788ca638d..83412a341 100644 --- a/src/Renderer/Surface.cpp +++ b/src/Renderer/Surface.cpp @@ -2648,14 +2648,23 @@ namespace sw size_t Surface::size(int width, int height, int depth, int border, int samples, Format format) { + samples = max(1, samples); + switch(format) { default: - // FIXME: Unpacking byte4 to short4 in the sampler currently involves reading 8 bytes, - // and stencil operations also read 8 bytes per four 8-bit stencil values, - // so we have to allocate 4 extra bytes to avoid buffer overruns. - return (size_t)sliceB(width, height, border, format, true) * depth * samples + 4; + { + uint64_t size = (uint64_t)sliceB(width, height, border, format, true) * depth * samples; + // FIXME: Unpacking byte4 to short4 in the sampler currently involves reading 8 bytes, + // and stencil operations also read 8 bytes per four 8-bit stencil values, + // so we have to allocate 4 extra bytes to avoid buffer overruns. + size += 4; + + // We can only sample buffers smaller than 2 GiB. + // Force an out-of-memory if larger, or let the caller report an error. + return size < 0x80000000u ? (size_t)size : std::numeric_limits::max(); + } case FORMAT_YV12_BT601: case FORMAT_YV12_BT709: case FORMAT_YV12_JFIF: diff --git a/tests/unittests/unittests.cpp b/tests/unittests/unittests.cpp index 3132258c5..f263eda80 100644 --- a/tests/unittests/unittests.cpp +++ b/tests/unittests/unittests.cpp @@ -204,7 +204,8 @@ protected: EXPECT_EQ((EGLBoolean)EGL_TRUE, success); } - struct ProgramHandles { + struct ProgramHandles + { GLuint program; GLuint vsShader; GLuint fsShader; @@ -287,6 +288,7 @@ protected: EGLConfig getConfig() const { return config; } EGLSurface getSurface() const { return surface; } EGLContext getContext() const { return context; } + private: EGLDisplay display; EGLConfig config; @@ -425,6 +427,29 @@ TEST_F(SwiftShaderTest, AtanCornerCases) Uninitialize(); } +// Test conditions that should result in a GL_OUT_OF_MEMORY and not crash +TEST_F(SwiftShaderTest, OutOfMemory) +{ + // Image sizes are assumed to fit in a 32-bit signed integer by the renderer, + // so test that we can't create a 2+ GiB image. + { + Initialize(3, false); + + GLuint tex = 1; + glBindTexture(GL_TEXTURE_3D, tex); + + const int width = 0xC2; + const int height = 0x541; + const int depth = 0x404; + glTexImage3D(GL_TEXTURE_3D, 0, GL_RGBA32F, width, height, depth, 0, GL_RGBA, GL_FLOAT, nullptr); + EXPECT_GLENUM_EQ(GL_OUT_OF_MEMORY, glGetError()); + + // The spec states that the GL is in an undefined state when GL_OUT_OF_MEMORY + // is returned, and the context must be recreated before attempting more rendering. + Uninitialize(); + } +} + // Note: GL_ARB_texture_rectangle is part of gl2extchromium.h in the Chromium repo // GL_ARB_texture_rectangle #ifndef GL_ARB_texture_rectangle diff --git a/tests/unittests/unittests.vcxproj.user b/tests/unittests/unittests.vcxproj.user index 567b19157..075c68fc1 100644 --- a/tests/unittests/unittests.vcxproj.user +++ b/tests/unittests/unittests.vcxproj.user @@ -3,7 +3,7 @@ PATH=$(SolutionDir)lib\$(Configuration)_$(Platform) WindowsLocalDebugger - --gtest_break_on_failure + --gtest_break_on_failure --gtest_filter=* PATH=$(SolutionDir)lib\$(Configuration)_$(Platform) @@ -12,7 +12,7 @@ PATH=$(SolutionDir)lib\$(Configuration)_$(Platform) WindowsLocalDebugger - --gtest_break_on_failure + --gtest_break_on_failure --gtest_filter=* PATH=$(SolutionDir)lib\$(Configuration)_$(Platform) -- 2.11.0