[Android] Gracefully handle a shared memory creation failure

ashmem_create_region() returns -error_code on failure. ScopedFD treats
only -1 as invalid handle meaning that ScopedFD will try to close
a file descriptor with a negative value other than -1. ScopedFD crashes
when close() fails.

This CL adds a check that the result of ashmem_create_region() is
non-negative before wrapping it into a ScopedFD.

This CL also adds a check that the rounding operation doesn't
overflow which was the reason of the failure in the first place.

Bug: 1022582
Change-Id: Id2dcec613e87c8eda97bf18cbd70f13b105b1e0a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1913239
Commit-Queue: Alex Ilin <alexilin@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#714884}

base/memory/platform_shared_memory_region_android.cc

@@ -163,9 +163,11 @@ PlatformSharedMemoryRegion PlatformSharedMemoryRegion::Create(Mode mode,
     return {};
   }
 
-  // Align size as required by ashmem_create_region() API documentation.
+  // Align size as required by ashmem_create_region() API documentation. This
+  // operation may overflow so check that the result doesn't decrease.
   size_t rounded_size = bits::Align(size, GetPageSize());
-  if (rounded_size > static_cast<size_t>(std::numeric_limits<int>::max())) {
+  if (rounded_size < size ||
+      rounded_size > static_cast<size_t>(std::numeric_limits<int>::max())) {
     LogCreateError(PlatformSharedMemoryRegion::CreateError::SIZE_TOO_LARGE);
     return {};
   }
@@ -175,16 +177,17 @@ PlatformSharedMemoryRegion PlatformSharedMemoryRegion::Create(Mode mode,
 
   UnguessableToken guid = UnguessableToken::Create();
 
-  ScopedFD fd(ashmem_create_region(
-      SharedMemoryTracker::GetDumpNameForTracing(guid).c_str(), rounded_size));
-  if (!fd.is_valid()) {
+  int fd = ashmem_create_region(
+      SharedMemoryTracker::GetDumpNameForTracing(guid).c_str(), rounded_size);
+  if (fd < 0) {
     LogCreateError(
         PlatformSharedMemoryRegion::CreateError::CREATE_FILE_MAPPING_FAILURE);
     DPLOG(ERROR) << "ashmem_create_region failed";
     return {};
   }
 
-  int err = ashmem_set_prot_region(fd.get(), PROT_READ | PROT_WRITE);
+  ScopedFD scoped_fd(fd);
+  int err = ashmem_set_prot_region(scoped_fd.get(), PROT_READ | PROT_WRITE);
   if (err < 0) {
     LogCreateError(
         PlatformSharedMemoryRegion::CreateError::REDUCE_PERMISSIONS_FAILURE);
@@ -193,7 +196,7 @@ PlatformSharedMemoryRegion PlatformSharedMemoryRegion::Create(Mode mode,
   }
 
   LogCreateError(PlatformSharedMemoryRegion::CreateError::SUCCESS);
-  return PlatformSharedMemoryRegion(std::move(fd), mode, size, guid);
+  return PlatformSharedMemoryRegion(std::move(scoped_fd), mode, size, guid);
 }
 
 bool PlatformSharedMemoryRegion::CheckPlatformHandlePermissionsCorrespondToMode(

base/memory/platform_shared_memory_region_fuchsia.cc

@@ -133,9 +133,12 @@ PlatformSharedMemoryRegion PlatformSharedMemoryRegion::Create(Mode mode,
   if (size == 0)
     return {};
 
+  // Aligning may overflow so check that the result doesn't decrease.
   size_t rounded_size = bits::Align(size, GetPageSize());
-  if (rounded_size > static_cast<size_t>(std::numeric_limits<int>::max()))
+  if (rounded_size < size ||
+      rounded_size > static_cast<size_t>(std::numeric_limits<int>::max())) {
     return {};
+  }
 
   CHECK_NE(mode, Mode::kReadOnly) << "Creating a region in read-only mode will "
                                      "lead to this region being non-modifiable";

base/memory/platform_shared_memory_region_unittest.cc

@@ -72,6 +72,19 @@ TEST_F(PlatformSharedMemoryRegionTest, CreateTooLargeRegionIsInvalid) {
   EXPECT_FALSE(region2.IsValid());
 }
 
+// Tests that creating a region of maximum possible value returns an invalid
+// region.
+TEST_F(PlatformSharedMemoryRegionTest, CreateMaxSizeRegionIsInvalid) {
+  size_t max_region_size = std::numeric_limits<size_t>::max();
+  PlatformSharedMemoryRegion region =
+      PlatformSharedMemoryRegion::CreateWritable(max_region_size);
+  EXPECT_FALSE(region.IsValid());
+
+  PlatformSharedMemoryRegion region2 =
+      PlatformSharedMemoryRegion::CreateUnsafe(max_region_size);
+  EXPECT_FALSE(region2.IsValid());
+}
+
 // Tests that regions consistently report their size as the size requested at
 // creation time even if their allocation size is larger due to platform
 // constraints.

base/memory/platform_shared_memory_region_win.cc

@@ -254,8 +254,10 @@ PlatformSharedMemoryRegion PlatformSharedMemoryRegion::Create(Mode mode,
     return {};
   }
 
+  // Aligning may overflow so check that the result doesn't decrease.
   size_t rounded_size = bits::Align(size, kSectionSize);
-  if (rounded_size > static_cast<size_t>(std::numeric_limits<int>::max())) {
+  if (rounded_size < size ||
+      rounded_size > static_cast<size_t>(std::numeric_limits<int>::max())) {
     LogError(CreateError::SIZE_TOO_LARGE, 0);
     return {};
   }