From fb6c1432c93a7e4ba683190e8ccd3ef397b511a0 Mon Sep 17 00:00:00 2001
From: "gman@chromium.org"
 <gman@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>
Date: Wed, 3 Oct 2012 17:42:10 +0000
Subject: [PATCH] Fix SetRange bounds check.

Note: The old code was tested in unit tests but still passes on a release
build. That suggests there's a differerce between optimization levels
on the chrome target vs the gpu_uinttests target

BUG=149717


Review URL: https://chromiumcodereview.appspot.com/11053012

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@159915 0039d316-1c4b-4281-b951-d872f2087c98
---
 gpu/command_buffer/service/buffer_manager.cc       | 14 ++++++++++++--
 gpu/command_buffer/service/buffer_manager.h        |  3 +++
 .../service/buffer_manager_unittest.cc             |  9 +++++++++
 3 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/gpu/command_buffer/service/buffer_manager.cc b/gpu/command_buffer/service/buffer_manager.cc
index cac64d5df09da..3dc5191a4e316 100644
--- a/gpu/command_buffer/service/buffer_manager.cc
+++ b/gpu/command_buffer/service/buffer_manager.cc
@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 #include "gpu/command_buffer/service/buffer_manager.h"
+#include <limits>
 #include "base/debug/trace_event.h"
 #include "base/logging.h"
 #include "gpu/command_buffer/common/gles2_cmd_utils.h"
@@ -105,9 +106,18 @@ void BufferManager::BufferInfo::SetInfo(
   }
 }
 
+bool BufferManager::BufferInfo::CheckRange(
+    GLintptr offset, GLsizeiptr size) const {
+  int32 end = 0;
+  return offset >= 0 && size >= 0 &&
+         offset <= std::numeric_limits<int32>::max() &&
+         size <= std::numeric_limits<int32>::max() &&
+         SafeAddInt32(offset, size, &end) && end <= size_;
+}
+
 bool BufferManager::BufferInfo::SetRange(
     GLintptr offset, GLsizeiptr size, const GLvoid * data) {
-  if (offset < 0 || offset + size < offset || offset + size > size_) {
+  if (!CheckRange(offset, size)) {
     return false;
   }
   if (shadowed_) {
@@ -122,7 +132,7 @@ const void* BufferManager::BufferInfo::GetRange(
   if (!shadowed_) {
     return NULL;
   }
-  if (offset < 0 || offset + size < offset || offset + size > size_) {
+  if (!CheckRange(offset, size)) {
     return NULL;
   }
   return shadow_.get() + offset;
diff --git a/gpu/command_buffer/service/buffer_manager.h b/gpu/command_buffer/service/buffer_manager.h
index 95ea5d31c05b9..f807349c3f97a 100644
--- a/gpu/command_buffer/service/buffer_manager.h
+++ b/gpu/command_buffer/service/buffer_manager.h
@@ -126,6 +126,9 @@ class GPU_EXPORT BufferManager {
     // Clears any cache of index ranges.
     void ClearCache();
 
+    // Check if an offset, size range is valid for the current buffer.
+    bool CheckRange(GLintptr offset, GLsizeiptr size) const;
+
     // The manager that owns this BufferInfo.
     BufferManager* manager_;
 
diff --git a/gpu/command_buffer/service/buffer_manager_unittest.cc b/gpu/command_buffer/service/buffer_manager_unittest.cc
index dcb8038979948..3d372afe8ad57 100644
--- a/gpu/command_buffer/service/buffer_manager_unittest.cc
+++ b/gpu/command_buffer/service/buffer_manager_unittest.cc
@@ -107,6 +107,11 @@ TEST_F(BufferManagerTest, SetRange) {
   EXPECT_FALSE(info->SetRange(0, sizeof(data) + 1, data));
   EXPECT_FALSE(info->SetRange(-1, sizeof(data), data));
   EXPECT_FALSE(info->SetRange(0, -1, data));
+  manager_.SetInfo(info, 1, GL_STATIC_DRAW);
+  const int size = 0x20000;
+  scoped_array<uint8> temp(new uint8[size]);
+  EXPECT_FALSE(info->SetRange(0 - size, size, temp.get()));
+  EXPECT_FALSE(info->SetRange(1, size / 2, temp.get()));
 }
 
 TEST_F(BufferManagerTest, GetRange) {
@@ -127,6 +132,10 @@ TEST_F(BufferManagerTest, GetRange) {
   EXPECT_TRUE(info->GetRange(0, sizeof(data) + 1) == NULL);
   EXPECT_TRUE(info->GetRange(-1, sizeof(data)) == NULL);
   EXPECT_TRUE(info->GetRange(-0, -1) == NULL);
+  const int size = 0x20000;
+  manager_.SetInfo(info, size / 2, GL_STATIC_DRAW);
+  EXPECT_TRUE(info->GetRange(0 - size, size) == NULL);
+  EXPECT_TRUE(info->GetRange(1, size / 2) == NULL);
 }
 
 TEST_F(BufferManagerTest, GetMaxValueForRangeUint8) {