The correct type for the size of a chunk of memory is size_t.

By using uint32, we have bugs on 64-bit platforms: callers passing in a size_t, will have their size truncated, potentially allocating a smaller
chunk than requested. There are a few places this happens, including on the
receiving ends of IPCs(!)

However, coversely, other callers of the API might directly assign the
memory chunk's length to uint32, leading to a different possible truncation
problem. This is guaraded against by limiting operations internally to
std::numeric_limits<uint32_t> in size for now.

There's some minor cascade effects that make the CL look larger than it is.

BUG=164678
Review URL: https://codereview.chromium.org/11446048

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@175987 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 35 insertions, 15 deletions

diff --git a/base/shared_memory.h b/base/shared_memory.h
index 1f8237e..4eefdbb 100644
--- a/base/shared_memory.h
+++ b/base/shared_memory.h
@@ -53,7 +53,7 @@ struct SharedMemoryCreateOptions {
 
   // Size of the shared memory object to be created.
   // When opening an existing object, this has no effect.
-  uint32 size;
+  size_t size;
 
   // If true, and the shared memory already exists, Create() will open the
   // existing shared memory and ignore the size parameter.  If false,
@@ -107,11 +107,11 @@ class BASE_EXPORT SharedMemory {
 
   // Creates and maps an anonymous shared memory segment of size size.
   // Returns true on success and false on failure.
-  bool CreateAndMapAnonymous(uint32 size);
+  bool CreateAndMapAnonymous(size_t size);
 
   // Creates an anonymous shared memory segment of size size.
   // Returns true on success and false on failure.
-  bool CreateAnonymous(uint32 size) {
+  bool CreateAnonymous(size_t size) {
     SharedMemoryCreateOptions options;
     options.size = size;
     return Create(options);
@@ -123,7 +123,7 @@ class BASE_EXPORT SharedMemory {
   // If open_existing is false, shared memory must not exist.
   // size is the size of the block to be created.
   // Returns true on success, false on failure.
-  bool CreateNamed(const std::string& name, bool open_existing, uint32 size) {
+  bool CreateNamed(const std::string& name, bool open_existing, size_t size) {
     SharedMemoryCreateOptions options;
     options.name = &name;
     options.open_existing = open_existing;
@@ -144,7 +144,7 @@ class BASE_EXPORT SharedMemory {
   // Returns true on success, false otherwise.  The memory address
   // is accessed via the memory() accessor.  The mapped address is guaranteed to
   // have an alignment of at least MAP_MINIMUM_ALIGNMENT.
-  bool Map(uint32 bytes);
+  bool Map(size_t bytes);
   enum { MAP_MINIMUM_ALIGNMENT = 32 };
 
   // Unmaps the shared memory from the caller's address space.
@@ -160,7 +160,7 @@ class BASE_EXPORT SharedMemory {
   // Deprecated method, please keep track of the size yourself if you created
   // it.
   // http://crbug.com/60821
-  uint32 created_size() const { return created_size_; }
+  size_t created_size() const { return created_size_; }
 
   // Gets a pointer to the opened memory space if it has been
   // Mapped via Map().  Returns NULL if it is not mapped.
@@ -239,12 +239,12 @@ class BASE_EXPORT SharedMemory {
   HANDLE             mapped_file_;
 #elif defined(OS_POSIX)
   int                mapped_file_;
-  uint32             mapped_size_;
+  size_t             mapped_size_;
   ino_t              inode_;
 #endif
   void*              memory_;
   bool               read_only_;
-  uint32             created_size_;
+  size_t             created_size_;
 #if !defined(OS_POSIX)
   SharedMemoryLock   lock_;
 #endif
diff --git a/base/shared_memory_android.cc b/base/shared_memory_android.cc
index a5beceb..e2c683c 100644
--- a/base/shared_memory_android.cc
+++ b/base/shared_memory_android.cc
@@ -19,6 +19,9 @@ namespace base {
 bool SharedMemory::Create(const SharedMemoryCreateOptions& options) {
   DCHECK_EQ(-1, mapped_file_ );
 
+  if (options.size > static_cast<size_t>(std::numeric_limits<int>::max()))
+    return false;
+
   // "name" is just a label in ashmem. It is visible in /proc/pid/maps.
   mapped_file_ = ashmem_create_region(
       options.name == NULL ? "" : options.name->c_str(),
diff --git a/base/shared_memory_nacl.cc b/base/shared_memory_nacl.cc
index fef2937..a86578b 100644
--- a/base/shared_memory_nacl.cc
+++ b/base/shared_memory_nacl.cc
@@ -10,6 +10,8 @@
 #include <sys/stat.h>
 #include <unistd.h>
 
+#include <limits>
+
 #include "base/logging.h"
 
 namespace base {
@@ -64,7 +66,7 @@ void SharedMemory::CloseHandle(const SharedMemoryHandle& handle) {
     DPLOG(ERROR) << "close";
 }
 
-bool SharedMemory::CreateAndMapAnonymous(uint32 size) {
+bool SharedMemory::CreateAndMapAnonymous(size_t size) {
   // Untrusted code can't create descriptors or handles.
   return false;
 }
@@ -82,10 +84,13 @@ bool SharedMemory::Open(const std::string& name, bool read_only) {
   return false;
 }
 
-bool SharedMemory::Map(uint32 bytes) {
+bool SharedMemory::Map(size_t bytes) {
   if (mapped_file_ == -1)
     return false;
 
+  if (bytes > static_cast<size_t>(std::numeric_limits<int>::max()))
+    return false;
+
   memory_ = mmap(NULL, bytes, PROT_READ | (read_only_ ? 0 : PROT_WRITE),
                  MAP_SHARED, mapped_file_, 0);
 
diff --git a/base/shared_memory_posix.cc b/base/shared_memory_posix.cc
index 7fda949..7862f42 100644
--- a/base/shared_memory_posix.cc
+++ b/base/shared_memory_posix.cc
@@ -98,7 +98,7 @@ void SharedMemory::CloseHandle(const SharedMemoryHandle& handle) {
     DPLOG(ERROR) << "close";
 }
 
-bool SharedMemory::CreateAndMapAnonymous(uint32 size) {
+bool SharedMemory::CreateAndMapAnonymous(size_t size) {
   return CreateAnonymous(size) && Map(size);
 }
 
@@ -113,6 +113,9 @@ bool SharedMemory::Create(const SharedMemoryCreateOptions& options) {
   DCHECK_EQ(-1, mapped_file_);
   if (options.size == 0) return false;
 
+  if (options.size > static_cast<size_t>(std::numeric_limits<int>::max()))
+    return false;
+
   // This function theoretically can block on the disk, but realistically
   // the temporary files we create will just go into the buffer cache
   // and be deleted before they ever make it out to disk.
@@ -153,7 +156,7 @@ bool SharedMemory::Create(const SharedMemoryCreateOptions& options) {
       file_util::CloseFile(fp);
       return false;
     }
-    const uint32 current_size = stat.st_size;
+    const size_t current_size = stat.st_size;
     if (current_size != options.size) {
       if (HANDLE_EINTR(ftruncate(fileno(fp), options.size)) != 0) {
         file_util::CloseFile(fp);
@@ -216,10 +219,13 @@ bool SharedMemory::Open(const std::string& name, bool read_only) {
 
 #endif  // !defined(OS_ANDROID)
 
-bool SharedMemory::Map(uint32 bytes) {
+bool SharedMemory::Map(size_t bytes) {
   if (mapped_file_ == -1)
     return false;
 
+  if (bytes > static_cast<size_t>(std::numeric_limits<int>::max()))
+    return false;
+
 #if defined(OS_ANDROID)
   if (bytes == 0) {
     int ashmem_bytes = ashmem_get_size_region(mapped_file_);
diff --git a/base/shared_memory_win.cc b/base/shared_memory_win.cc
index b0fdafe3..877ccd7 100644
--- a/base/shared_memory_win.cc
+++ b/base/shared_memory_win.cc
@@ -70,7 +70,7 @@ void SharedMemory::CloseHandle(const SharedMemoryHandle& handle) {
   ::CloseHandle(handle);
 }
 
-bool SharedMemory::CreateAndMapAnonymous(uint32 size) {
+bool SharedMemory::CreateAndMapAnonymous(size_t size) {
   return CreateAnonymous(size) && Map(size);
 }
 
@@ -80,6 +80,9 @@ bool SharedMemory::Create(const SharedMemoryCreateOptions& options) {
   if (options.size == 0)
     return false;
 
+  if (options.size > static_cast<size_t>(std::numeric_limits<int>::max()))
+    return false;
+
   // NaCl's memory allocator requires 0mod64K alignment and size for
   // shared memory objects.  To allow passing shared memory to NaCl,
   // therefore we round the size actually created to the nearest 64K unit.
@@ -131,10 +134,13 @@ bool SharedMemory::Open(const std::string& name, bool read_only) {
   return false;
 }
 
-bool SharedMemory::Map(uint32 bytes) {
+bool SharedMemory::Map(size_t bytes) {
   if (mapped_file_ == NULL)
     return false;
 
+  if (bytes > static_cast<size_t>(std::numeric_limits<int>::max()))
+    return false;
+
   memory_ = MapViewOfFile(mapped_file_,
       read_only_ ? FILE_MAP_READ : FILE_MAP_ALL_ACCESS, 0, 0, bytes);
   if (memory_ != NULL) {