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 <nicolascapens@google.com>
Tested-by: Nicolas Capens <nicolascapens@google.com>

diff --git a/src/OpenGL/common/Image.cpp b/src/OpenGL/common/Image.cpp
index 6f38322..601876e 100644 (file)
--- 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 cb77a4c..4fed893 100644 (file)
--- 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 788ca63..83412a3 100644 (file)
--- 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<size_t>::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 3132258..f263eda 100644 (file)
--- 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 567b191..075c68f 100644 (file)
--- a/tests/unittests/unittests.vcxproj.user
+++ b/tests/unittests/unittests.vcxproj.user
@@ -3,7 +3,7 @@
   <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Debug|Win32'">\r
     <LocalDebuggerEnvironment>PATH=$(SolutionDir)lib\$(Configuration)_$(Platform)</LocalDebuggerEnvironment>\r
     <DebuggerFlavor>WindowsLocalDebugger</DebuggerFlavor>\r
-    <LocalDebuggerCommandArguments>--gtest_break_on_failure</LocalDebuggerCommandArguments>\r
+    <LocalDebuggerCommandArguments>--gtest_break_on_failure --gtest_filter=*</LocalDebuggerCommandArguments>\r
   </PropertyGroup>\r
   <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Release|Win32'">\r
     <LocalDebuggerEnvironment>PATH=$(SolutionDir)lib\$(Configuration)_$(Platform)</LocalDebuggerEnvironment>\r
@@ -12,7 +12,7 @@
   <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Debug|x64'">\r
     <LocalDebuggerEnvironment>PATH=$(SolutionDir)lib\$(Configuration)_$(Platform)</LocalDebuggerEnvironment>\r
     <DebuggerFlavor>WindowsLocalDebugger</DebuggerFlavor>\r
-    <LocalDebuggerCommandArguments>--gtest_break_on_failure</LocalDebuggerCommandArguments>\r
+    <LocalDebuggerCommandArguments>--gtest_break_on_failure --gtest_filter=*</LocalDebuggerCommandArguments>\r
   </PropertyGroup>\r
   <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Release|x64'">\r
     <LocalDebuggerEnvironment>PATH=$(SolutionDir)lib\$(Configuration)_$(Platform)</LocalDebuggerEnvironment>\r