tor-browser

commit e16a6517255791b35a6077a234ae23a25cfe2dc5
parent 8da206a49a226da313113fadb1cf83339a885eb1
Author: Yannis Juglaret <yjuglaret@mozilla.com>
Date:   Mon,  3 Nov 2025 13:43:48 +0000

Bug 1997298 - Revert "Bug 1992678 - Use a more resilient initialization for sandboxed children." r=bobowen

This reverts commit bb7b5955cc0ddc9234cec40dd6b4d8932106170d.

Differential Revision: https://phabricator.services.mozilla.com/D270667

Diffstat:
D
security/sandbox/chromium-shim/patches/50_more_resilient_initialization_for_sandboxed_children.patch
 | 
45
---------------------------------------------
M
security/sandbox/chromium/sandbox/win/src/interception.cc
 | 
16
+++-------------

2 files changed, 3 insertions(+), 58 deletions(-)
diff --git a/security/sandbox/chromium-shim/patches/50_more_resilient_initialization_for_sandboxed_children.patch b/security/sandbox/chromium-shim/patches/50_more_resilient_initialization_for_sandboxed_children.patch
@@ -1,45 +0,0 @@
-Use a more resilient initialization for sandboxed children.
-
-If commiting RWX memory fails, we fall back to RW instead, which is
-enough to initialize the thunks. In that case, we make sure that the
-later transition to RX permission succeeds.
----
- sandbox/win/src/interception.cc | 16 +++++++++++++---
- 1 file changed, 13 insertions(+), 3 deletions(-)
-
-diff --git a/security/sandbox/chromium/sandbox/win/src/interception.cc b/security/sandbox/chromium/sandbox/win/src/interception.cc
-index 04f32b099646..c78d56e17abc 100644
---- a/security/sandbox/chromium/sandbox/win/src/interception.cc
-+++ b/security/sandbox/chromium/sandbox/win/src/interception.cc
-@@ -386,9 +386,16 @@ ResultCode InterceptionManager::PatchNtdll(bool hot_patch_needed) {
- 
-   // Make an aligned, padded allocation, and move the pointer to our chunk.
-   size_t thunk_bytes_padded = base::bits::AlignUp(thunk_bytes, kPageSize);
--  thunk_base = reinterpret_cast<BYTE*>(
-+  BYTE* thunk_base_rwx = reinterpret_cast<BYTE*>(
-       ::VirtualAllocEx(child, thunk_base, thunk_bytes_padded, MEM_COMMIT,
-                        PAGE_EXECUTE_READWRITE));
-+  bool rwx_commit_succeeded = static_cast<bool>(thunk_base_rwx);
-+  if (rwx_commit_succeeded) {
-+    thunk_base = thunk_base_rwx;
-+  } else {
-+    thunk_base = reinterpret_cast<BYTE*>(::VirtualAllocEx(
-+        child, thunk_base, thunk_bytes_padded, MEM_COMMIT, PAGE_READWRITE));
-+  }
-   CHECK(thunk_base);  // If this fails we'd crash anyway on an invalid access.
-   DllInterceptionData* thunks =
-       reinterpret_cast<DllInterceptionData*>(thunk_base + thunk_offset);
-@@ -418,8 +425,11 @@ ResultCode InterceptionManager::PatchNtdll(bool hot_patch_needed) {
- 
-   // Attempt to protect all the thunks, but ignore failure
-   DWORD old_protection;
--  ::VirtualProtectEx(child, thunks, thunk_bytes, PAGE_EXECUTE_READ,
--                     &old_protection);
-+  bool rx_update_succeeded = static_cast<bool>(::VirtualProtectEx(
-+      child, thunks, thunk_bytes, PAGE_EXECUTE_READ, &old_protection));
-+
-+  // We need the EXECUTE permission on the thunks, one way or the other
-+  CHECK(rwx_commit_succeeded || rx_update_succeeded);
- 
-   ResultCode ret =
-       child_->TransferVariable("g_originals", g_originals, sizeof(g_originals));
diff --git a/security/sandbox/chromium/sandbox/win/src/interception.cc b/security/sandbox/chromium/sandbox/win/src/interception.cc
@@ -386,16 +386,9 @@ ResultCode InterceptionManager::PatchNtdll(bool hot_patch_needed) {
 
   // Make an aligned, padded allocation, and move the pointer to our chunk.
   size_t thunk_bytes_padded = base::bits::AlignUp(thunk_bytes, kPageSize);
-  BYTE* thunk_base_rwx = reinterpret_cast<BYTE*>(
+  thunk_base = reinterpret_cast<BYTE*>(
       ::VirtualAllocEx(child, thunk_base, thunk_bytes_padded, MEM_COMMIT,
                        PAGE_EXECUTE_READWRITE));
-  bool rwx_commit_succeeded = static_cast<bool>(thunk_base_rwx);
-  if (rwx_commit_succeeded) {
-    thunk_base = thunk_base_rwx;
-  } else {
-    thunk_base = reinterpret_cast<BYTE*>(::VirtualAllocEx(
-        child, thunk_base, thunk_bytes_padded, MEM_COMMIT, PAGE_READWRITE));
-  }
   CHECK(thunk_base);  // If this fails we'd crash anyway on an invalid access.
   DllInterceptionData* thunks =
       reinterpret_cast<DllInterceptionData*>(thunk_base + thunk_offset);
@@ -425,11 +418,8 @@ ResultCode InterceptionManager::PatchNtdll(bool hot_patch_needed) {
 
   // Attempt to protect all the thunks, but ignore failure
   DWORD old_protection;
-  bool rx_update_succeeded = static_cast<bool>(::VirtualProtectEx(
-      child, thunks, thunk_bytes, PAGE_EXECUTE_READ, &old_protection));
-
-  // We need the EXECUTE permission on the thunks, one way or the other
-  CHECK(rwx_commit_succeeded || rx_update_succeeded);
+  ::VirtualProtectEx(child, thunks, thunk_bytes, PAGE_EXECUTE_READ,
+                     &old_protection);
 
   ResultCode ret =
       child_->TransferVariable("g_originals", g_originals, sizeof(g_originals));