commit ece7c5a848df6b64e361184bbf3a9739b209e40c
author Corentin Wallez <cwallez@chromium.org> Wed Sep 21 15:28:23 2016 -0400
committer Commit Bot <commit-bot@chromium.org> Tue Sep 27 15:58:00 2016 +0000
tree 7cbe5217698549d7d20517a0e29bdf4f7b59da7b
parent 2cacb7783116bdd94b2dd70315418a62e035bd1d

Add validation for the pack buffer in ReadPixels

BUG=angleproject:1512

Change-Id: Ia6bac628c35f04bc5d3adfde1569902475519698
Reviewed-on: https://chromium-review.googlesource.com/387668
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Geoff Lang <geofflang@chromium.org>
Reviewed-by: Jamie Madill <jmadill@chromium.org>

src/libANGLE/validationES.cpp

diff --git a/src/libANGLE/validationES.cpp b/src/libANGLE/validationES.cpp
index 3ab4f6b..2d3474a 100644
--- a/src/libANGLE/validationES.cpp
+++ b/src/libANGLE/validationES.cpp
@@ -1168,6 +1168,44 @@
         return false;
     }
 
+    // Check for pixel pack buffer related API errors
+    gl::Buffer *pixelPackBuffer = context->getGLState().getTargetBuffer(GL_PIXEL_PACK_BUFFER);
+    if (pixelPackBuffer != nullptr)
+    {
+        // ..  the data would be packed to the buffer object such that the memory writes required
+        // would exceed the data store size.
+        GLenum sizedInternalFormat       = GetSizedInternalFormat(format, type);
+        const InternalFormat &formatInfo = GetInternalFormatInfo(sizedInternalFormat);
+        const gl::Extents size(width, height, 1);
+        const auto &pack = context->getGLState().getPackState();
+
+        auto endByteOrErr = formatInfo.computePackEndByte(type, size, pack);
+        if (endByteOrErr.isError())
+        {
+            context->handleError(endByteOrErr.getError());
+            return false;
+        }
+
+        CheckedNumeric<size_t> checkedEndByte(endByteOrErr.getResult());
+        CheckedNumeric<size_t> checkedOffset(reinterpret_cast<size_t>(pixels));
+        checkedEndByte += checkedOffset;
+
+        if (checkedEndByte.ValueOrDie() > static_cast<size_t>(pixelPackBuffer->getSize()))
+        {
+            // Overflow past the end of the buffer
+            context->handleError(
+                Error(GL_INVALID_OPERATION, "Writes would overflow the pixel pack buffer."));
+            return false;
+        }
+
+        // ...the buffer object's data store is currently mapped.
+        if (pixelPackBuffer->isMapped())
+        {
+            context->handleError(Error(GL_INVALID_OPERATION, "Pixel pack buffer is mapped."));
+            return false;
+        }
+    }
+
     return true;
 }
 
@@ -1188,30 +1226,20 @@
     }
 
     GLenum sizedInternalFormat = GetSizedInternalFormat(format, type);
-    const InternalFormat &sizedFormatInfo = GetInternalFormatInfo(sizedInternalFormat);
+    const InternalFormat &formatInfo = GetInternalFormatInfo(sizedInternalFormat);
+    const gl::Extents size(width, height, 1);
+    const auto &pack = context->getGLState().getPackState();
 
-    auto outputPitchOrErr =
-        sizedFormatInfo.computeRowPitch(type, width, context->getGLState().getPackAlignment(),
-                                        context->getGLState().getPackRowLength());
-
-    if (outputPitchOrErr.isError())
+    auto endByteOrErr = formatInfo.computePackEndByte(type, size, pack);
+    if (endByteOrErr.isError())
     {
-        context->handleError(outputPitchOrErr.getError());
+        context->handleError(endByteOrErr.getError());
         return false;
     }
 
-    CheckedNumeric<GLuint> checkedOutputPitch(outputPitchOrErr.getResult());
-    auto checkedRequiredSize = checkedOutputPitch * height;
-    if (!checkedRequiredSize.IsValid())
+    if (endByteOrErr.getResult() > static_cast<GLuint>(bufSize))
     {
-        context->handleError(Error(GL_INVALID_OPERATION, "Unsigned multiplication overflow."));
-        return false;
-    }
-
-    // sized query sanity check
-    if (checkedRequiredSize.ValueOrDie() > static_cast<GLuint>(bufSize))
-    {
-        context->handleError(Error(GL_INVALID_OPERATION));
+        context->handleError(Error(GL_INVALID_OPERATION, "Writes would overflow past bufSize."));
         return false;
     }