[base] Abort on unexpected failures in OS::SetPermissions

It is expected that changing page permissions can fail due to the system
running out of memory. However, any other failure is unexpected and
likely indicates a bug in the caller, such as changing the permissions
of an invalid memory region. To allow distinguishing between these
unexpected failures and expected OOM failures, this CL adds CHECKs into
the low-level memory management routines to abort when an unexpected
failure occurs.

Similar logic could later be added to other low-level memory management
routines that can legitimately fail due to OOM as well.

Bug: chromium:1320126
Change-Id: I3de6f4b2aed8962c91770b81382df34384584501
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3610445
Commit-Queue: Samuel Groß <saelo@chromium.org>
Reviewed-by: Victor Gomes <victorgomes@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#80245}

include/v8-platform.h

@@ -709,6 +709,10 @@ class VirtualAddressSpace {
   /**
    * Sets permissions of all allocated pages in the given range.
    *
+   * This operation can fail due to OOM, in which case false is returned. If
+   * the operation fails for a reason other than OOM, this function will
+   * terminate the process as this implies a bug in the client.
+   *
    * \param address The start address of the range. Must be aligned to
    * page_size().
    *
@@ -717,7 +721,7 @@ class VirtualAddressSpace {
    *
    * \param permissions The new permissions for the range.
    *
-   * \returns true on success, false otherwise.
+   * \returns true on success, false on OOM.
    */
   virtual V8_WARN_UNUSED_RESULT bool SetPagePermissions(
       Address address, size_t size, PagePermissions permissions) = 0;

src/base/platform/platform-fuchsia.cc

@@ -167,8 +167,16 @@ bool SetPermissionsInternal(const zx::vmar& vmar, size_t page_size,
   DCHECK_EQ(0, reinterpret_cast<uintptr_t>(address) % page_size);
   DCHECK_EQ(0, size % page_size);
   uint32_t prot = GetProtectionFromMemoryPermission(access);
-  return vmar.protect(prot, reinterpret_cast<uintptr_t>(address), size) ==
-         ZX_OK;
+  zx_status_t result =
+      vmar.protect(prot, reinterpret_cast<uintptr_t>(address), size);
+
+  // Any failure that's not OOM likely indicates a bug in the caller (e.g.
+  // using an invalid mapping) so attempt to catch that here to facilitate
+  // debugging of these failures. According to the documentation,
+  // zx_vmar_protect cannot return ZX_ERR_NO_MEMORY, so any error here is
+  // unexpected.
+  CHECK_EQ(status, ZX_OK);
+  return status == ZX_OK;
 }
 
 bool DiscardSystemPagesInternal(const zx::vmar& vmar, size_t page_size,

src/base/platform/platform-posix.cc

@@ -472,6 +472,11 @@ bool OS::SetPermissions(void* address, size_t size, MemoryPermission access) {
   int prot = GetProtectionFromMemoryPermission(access);
   int ret = mprotect(address, size, prot);
 
+  // Any failure that's not OOM likely indicates a bug in the caller (e.g.
+  // using an invalid mapping) so attempt to catch that here to facilitate
+  // debugging of these failures.
+  if (ret != 0) CHECK_EQ(ENOMEM, errno);
+
   // MacOS 11.2 on Apple Silicon refuses to switch permissions from
   // rwx to none. Just use madvise instead.
 #if defined(V8_OS_DARWIN)

src/base/platform/platform-win32.cc

@@ -997,7 +997,14 @@ bool OS::SetPermissions(void* address, size_t size, MemoryPermission access) {
     return VirtualFree(address, size, MEM_DECOMMIT) != 0;
   }
   DWORD protect = GetProtectionFromMemoryPermission(access);
-  return VirtualAllocWrapper(address, size, MEM_COMMIT, protect) != nullptr;
+  void* result = VirtualAllocWrapper(address, size, MEM_COMMIT, protect);
+
+  // Any failure that's not OOM likely indicates a bug in the caller (e.g.
+  // using an invalid mapping) so attempt to catch that here to facilitate
+  // debugging of these failures.
+  if (!result) CHECK_EQ(ERROR_NOT_ENOUGH_MEMORY, GetLastError());
+
+  return result != nullptr;
 }
 
 // static