Supress integer-overflow in TexSubImage2D(3D)Impl

gpu/command_buffer/client/gles2_implementation.cc

Index: gpu/command_buffer/client/gles2_implementation.cc
diff --git a/gpu/command_buffer/client/gles2_implementation.cc b/gpu/command_buffer/client/gles2_implementation.cc
index aa3b18d4e783c28b8bb69de4ca9198c64856346c..4b07c69d80abba8375ce7a658e86d234e45ef72a 100644
--- a/gpu/command_buffer/client/gles2_implementation.cc
+++ b/gpu/command_buffer/client/gles2_implementation.cc
@@ -19,6 +19,7 @@
 #include <string>
 #include "base/atomic_sequence_num.h"
 #include "base/compiler_specific.h"
+#include "base/numerics/safe_math.h"
 #include "base/strings/string_split.h"
 #include "base/strings/stringprintf.h"
 #include "base/sys_info.h"
@@ -3047,10 +3048,11 @@ void GLES2Implementation::TexSubImage2DImpl(GLenum target,
   const int8_t* source = reinterpret_cast<const int8_t*>(pixels);
   // Transfer by rows.
   while (height) {
-    unsigned int desired_size =
-        buffer_padded_row_size * (height - 1) + unpadded_row_size;
+    base::CheckedNumeric<unsigned int> desired_size = buffer_padded_row_size;

[Zhenyao Mo, 2016/09/06 21:52:10]

I dig a little further into the code.  It seems to me that whoever calls this
function already did the overflow check.  For example, in TexSubImage2D(),
GLES2Util::ComputeImageDataSizesES3() should make sure no overflow is possible
in here.

Can you check further and see why that function doesn't catch the overflow?

[xidachen, 2016/09/07 02:09:42]

After a closer look, I think you are right. Most checks are not necessary in
TexSubImage2D(3D)Impl because they are already checked to be safe before it is
called. Except one place which is the
yoffset += num_rows;

There are two call sites of TexSubImage2DImpl, and none of them checks yoffset
against any "+" operations.

In PS#4, I removed all un-necessary checks. PTAL.

+    desired_size  = desired_size * (height - 1) + unpadded_row_size;
     if (!buffer->valid() || buffer->size() == 0) {
-      buffer->Reset(desired_size);
+      buffer->Reset(desired_size.ValueOrDefault(
+          std::numeric_limits<unsigned int>::max()));
       if (!buffer->valid()) {
         return;
       }
@@ -3066,8 +3068,13 @@ void GLES2Implementation::TexSubImage2DImpl(GLenum target,
         target, level, xoffset, yoffset, width, num_rows, format, type,
         buffer->shm_id(), buffer->offset(), internal);
     buffer->Release();
-    yoffset += num_rows;
-    source += num_rows * pixels_padded_row_size;
+    base::CheckedNumeric<GLint> updated_yoffset = yoffset;
+    updated_yoffset += num_rows;
+    yoffset = updated_yoffset.ValueOrDefault(std::numeric_limits<int>::max());
+    base::CheckedNumeric<uint32_t> source_offset = pixels_padded_row_size;
+    source_offset *= num_rows;
+    source += source_offset.ValueOrDefault(
+        std::numeric_limits<uint32_t>::max());
     height -= num_rows;
   }
 }
@@ -3097,31 +3104,33 @@ void GLES2Implementation::TexSubImage3DImpl(GLenum target,
   DCHECK_GE(yoffset, 0);
   DCHECK_GE(zoffset, 0);
   const int8_t* source = reinterpret_cast<const int8_t*>(pixels);
-  GLsizei total_rows = height * depth;
+  base::CheckedNumeric<GLsizei> checked_total_rows = height;
+  checked_total_rows *= depth;
+  GLsizei total_rows = checked_total_rows.ValueOrDefault(
+      std::numeric_limits<int>::max());
   GLint row_index = 0, depth_index = 0;
   while (total_rows) {
     // Each time, we either copy one or more images, or copy one or more rows
     // within a single image, depending on the buffer size limit.
     GLsizei max_rows;
-    unsigned int desired_size;
+    base::CheckedNumeric<unsigned int> desired_size = buffer_padded_row_size;
     if (row_index > 0) {
       // We are in the middle of an image. Send the remaining of the image.
       max_rows = height - row_index;
       if (total_rows <= height) {
         // Last image, so last row is unpadded.
-        desired_size = buffer_padded_row_size * (max_rows - 1) +
-            unpadded_row_size;
+        desired_size = desired_size * (max_rows - 1) + unpadded_row_size;
       } else {
-        desired_size = buffer_padded_row_size * max_rows;
+        desired_size *= max_rows;
       }
     } else {
       // Send all the remaining data if possible.
       max_rows = total_rows;
-      desired_size =
-          buffer_padded_row_size * (max_rows - 1) + unpadded_row_size;
+      desired_size = desired_size * (max_rows - 1) + unpadded_row_size;
     }
     if (!buffer->valid() || buffer->size() == 0) {
-      buffer->Reset(desired_size);
+      buffer->Reset(desired_size.ValueOrDefault(
+          std::numeric_limits<unsigned int>::max()));
       if (!buffer->valid()) {
         return;
       }
@@ -3144,8 +3153,16 @@ void GLES2Implementation::TexSubImage3DImpl(GLenum target,
       int8_t* buffer_pointer = reinterpret_cast<int8_t*>(buffer->address());
       uint32_t src_height =
           unpack_image_height_ > 0 ? unpack_image_height_ : height;
-      uint32_t image_size_dst = buffer_padded_row_size * height;
-      uint32_t image_size_src = pixels_padded_row_size * src_height;
+      base::CheckedNumeric<uint32_t> checked_image_size_dst
+          = buffer_padded_row_size;
+      checked_image_size_dst *= height;
+      uint32_t image_size_dst = checked_image_size_dst.ValueOrDefault(
+          std::numeric_limits<uint32_t>::max());
+      base::CheckedNumeric<uint32_t> checked_image_size_src
+          = pixels_padded_row_size;
+      checked_image_size_src *= src_height;
+      uint32_t image_size_src = checked_image_size_src.ValueOrDefault(
+          std::numeric_limits<uint32_t>::max());
       for (GLint ii = 0; ii < num_images; ++ii) {
         CopyRectToBuffer(
             source + ii * image_size_src, my_height, unpadded_row_size,
@@ -3178,10 +3195,15 @@ void GLES2Implementation::TexSubImage3DImpl(GLenum target,
           num_image_paddings++;
         }
       }
-      source += num_rows * pixels_padded_row_size;
+      base::CheckedNumeric<uint32_t> source_offset = pixels_padded_row_size;
+      source_offset *= num_rows;
+      source += source_offset.ValueOrDefault(
+          std::numeric_limits<uint32_t>::max());
       if (unpack_image_height_ > height && num_image_paddings > 0) {
-        source += num_image_paddings * (unpack_image_height_ - height) *
-            pixels_padded_row_size;
+        source_offset = source_offset * num_image_paddings *
+            (unpack_image_height_ - height);
+        source += source_offset.ValueOrDefault(
+            std::numeric_limits<uint32_t>::max());
       }
     }
   }