Bug 1835876 part 2 - Disable code write protection in content processes. r=nbp

We've worked with the security and performance teams to re-evaluate the W^X policy
we have in place to mark JIT code memory pages as either writable or executable
(but not both).

Recommendation from the security team is to disable this mitigation in the content
process, because it's not worth the performance cost since there are known and reusable
techniques to bypass it. The V8 team has recently reached a similar conclusion.

We still leave write protection enabled in the parent process and other non-content
processes, because performance is less of a concern there and it's unclear if the techniques
to bypass this mitigation apply to these processes as well.

This patch adds a `javascript.options.content_process_write_protect_code` pref for this
and sets it to false. The JS shell has a new `--write-protect-code=off/on` flag. This
defaults to `on` for now to help catch W^X violations early on.

This is expected to improve performance on Speedometer 3 by about 3-4%. We've seen similar
numbers for other benchmarks including page load measurements.

Depends on D179468

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

js/src/jit/JitOptions.cpp

@@ -280,6 +280,10 @@ DefaultJitOptions::DefaultJitOptions() {
   SET_DEFAULT(spectreJitToCxxCalls, true);
 #endif
 
+  // Whether the W^X policy is enforced to mark JIT code pages as either
+  // writable or executable but never both at the same time.
+  SET_DEFAULT(writeProtectCode, true);
+
   // This is set to its actual value in InitializeJit.
   SET_DEFAULT(supportsUnalignedAccesses, false);
 
@@ -410,5 +414,15 @@ void DefaultJitOptions::resetNormalIonWarmUpThreshold() {
   setNormalIonWarmUpThreshold(defaultValues.normalIonWarmUpThreshold);
 }
 
+void DefaultJitOptions::maybeSetWriteProtectCode(bool val) {
+  // For now don't change the default (true) on Apple ARM64 because we can't use
+  // RWX pages there without MAP_JIT. See bug 1837194.
+#if defined(XP_MACOSX) && defined(__aarch64__)
+  MOZ_ASSERT(writeProtectCode);
+#else
+  writeProtectCode = val;
+#endif
+}
+
 }  // namespace jit
 }  // namespace js

js/src/jit/JitOptions.h

@@ -122,6 +122,8 @@ struct DefaultJitOptions {
   bool spectreValueMasking;
   bool spectreJitToCxxCalls;
 
+  bool writeProtectCode;
+
   bool supportsUnalignedAccesses;
   BaseRegForAddress baseRegForLocals;
 
@@ -146,6 +148,8 @@ struct DefaultJitOptions {
   void enableGvn(bool val);
   void setFastWarmUp();
 
+  void maybeSetWriteProtectCode(bool val);
+
   bool eagerIonCompilation() const { return normalIonWarmUpThreshold == 0; }
 };
 

js/src/jit/ProcessExecutableMemory.cpp

@@ -384,6 +384,9 @@ static void DeallocateProcessExecutableMemory(void* addr, size_t bytes) {
 }
 
 static DWORD ProtectionSettingToFlags(ProtectionSetting protection) {
+  if (!JitOptions.writeProtectCode) {
+    return PAGE_EXECUTE_READWRITE;
+  }
   switch (protection) {
     case ProtectionSetting::Writable:
       return PAGE_READWRITE;
@@ -496,6 +499,9 @@ static void DeallocateProcessExecutableMemory(void* addr, size_t bytes) {
 }
 
 static unsigned ProtectionSettingToFlags(ProtectionSetting protection) {
+  if (!JitOptions.writeProtectCode) {
+    return PROT_READ | PROT_WRITE | PROT_EXEC;
+  }
 #  ifdef MOZ_VALGRIND
   // If we're configured for Valgrind and running on it, use a slacker
   // scheme that doesn't change execute permissions, since doing so causes
@@ -890,6 +896,10 @@ bool js::jit::ReprotectRegion(void* start, size_t size,
 #else
   std::atomic_thread_fence(std::memory_order_seq_cst);
 
+  if (!JitOptions.writeProtectCode) {
+    return true;
+  }
+
 #  ifdef XP_WIN
   DWORD flags = ProtectionSettingToFlags(protection);
   // This is a essentially a VirtualProtect, but with lighter impact on

js/src/jsapi.cpp

@@ -4225,6 +4225,9 @@ JS_PUBLIC_API void JS_SetGlobalJitCompilerOption(JSContext* cx,
     case JSJITCOMPILER_SPECTRE_JIT_TO_CXX_CALLS:
       jit::JitOptions.spectreJitToCxxCalls = !!value;
       break;
+    case JSJITCOMPILER_WRITE_PROTECT_CODE:
+      jit::JitOptions.maybeSetWriteProtectCode(!!value);
+      break;
     case JSJITCOMPILER_WATCHTOWER_MEGAMORPHIC:
       jit::JitOptions.enableWatchtowerMegamorphic = !!value;
       break;
@@ -4314,6 +4317,9 @@ JS_PUBLIC_API bool JS_GetGlobalJitCompilerOption(JSContext* cx,
     case JSJITCOMPILER_SPECTRE_JIT_TO_CXX_CALLS:
       *valueOut = jit::JitOptions.spectreJitToCxxCalls ? 1 : 0;
       break;
+    case JSJITCOMPILER_WRITE_PROTECT_CODE:
+      *valueOut = jit::JitOptions.writeProtectCode ? 1 : 0;
+      break;
     case JSJITCOMPILER_WATCHTOWER_MEGAMORPHIC:
       *valueOut = jit::JitOptions.enableWatchtowerMegamorphic ? 1 : 0;
       break;

js/src/jsapi.h

@@ -836,6 +836,7 @@ extern JS_PUBLIC_API void JS_SetOffthreadIonCompilationEnabled(JSContext* cx,
   Register(SPECTRE_STRING_MITIGATIONS, "spectre.string-mitigations") \
   Register(SPECTRE_VALUE_MASKING, "spectre.value-masking") \
   Register(SPECTRE_JIT_TO_CXX_CALLS, "spectre.jit-to-cxx-calls") \
+  Register(WRITE_PROTECT_CODE, "write-protect-code") \
   Register(WATCHTOWER_MEGAMORPHIC, "watchtower.megamorphic") \
   Register(WASM_FOLD_OFFSETS, "wasm.fold-offsets") \
   Register(WASM_DELAY_TIER2, "wasm.delay-tier2") \

js/src/shell/fuzz-flags.txt

@@ -50,6 +50,8 @@
 --nursery-bigints=on
 --spectre-mitigations=off
 --spectre-mitigations=on
+--write-protect-code=off
+--write-protect-code=on
 --more-compartments
 --fast-warmup
 --no-jit-backend

js/src/shell/js.cpp

@@ -11437,6 +11437,11 @@ bool InitOptionParser(OptionParser& op) {
       !op.addStringOption('\0', "spectre-mitigations", "on/off",
                           "Whether Spectre mitigations are enabled (default: "
                           "off, on to enable)") ||
+      !op.addStringOption('\0', "write-protect-code", "on/off",
+                          "Whether the W^X policy is enforced to mark JIT code "
+                          "pages as either writable or executable but never "
+                          "both at the same time (default: on, off to "
+                          "disable)") ||
       !op.addStringOption('\0', "cache-ir-stubs", "on/off/call",
                           "Use CacheIR stubs (default: on, off to disable, "
                           "call to enable work-in-progress call ICs)") ||
@@ -12156,6 +12161,16 @@ bool SetContextJITOptions(JSContext* cx, const OptionParser& op) {
     }
   }
 
+  if (const char* str = op.getStringOption("write-protect-code")) {
+    if (strcmp(str, "on") == 0) {
+      jit::JitOptions.maybeSetWriteProtectCode(true);
+    } else if (strcmp(str, "off") == 0) {
+      jit::JitOptions.maybeSetWriteProtectCode(false);
+    } else {
+      return OptionFailure("write-protect-code", str);
+    }
+  }
+
   if (const char* str = op.getStringOption("ion-scalar-replacement")) {
     if (strcmp(str, "on") == 0) {
       jit::JitOptions.disableScalarReplacement = false;

js/src/tests/lib/tests.py

@@ -25,7 +25,7 @@ JITFLAGS = {
             "--no-sse3",
             "--no-threads",
         ],
-        ["--baseline-eager"],
+        ["--baseline-eager", "--write-protect-code=off"],
         ["--no-blinterp", "--no-baseline", "--no-ion", "--more-compartments"],
         ["--blinterp-eager"],
     ],
@@ -38,12 +38,12 @@ JITFLAGS = {
             "--ion-offthread-compile=off",  # implies --baseline-eager
             "--more-compartments",
         ],
-        ["--baseline-eager"],
+        ["--baseline-eager", "--write-protect-code=off"],
         ["--no-blinterp", "--no-baseline", "--no-ion", "--more-compartments"],
     ],
     # used by jit_test.py
     "ion": [
-        ["--baseline-eager"],
+        ["--baseline-eager", "--write-protect-code=off"],
         ["--ion-eager", "--ion-offthread-compile=off", "--more-compartments"],
     ],
     # Run reduced variants on debug builds, since they take longer time.
@@ -54,7 +54,7 @@ JITFLAGS = {
             "--ion-offthread-compile=off",  # implies --baseline-eager
             "--more-compartments",
         ],
-        ["--baseline-eager"],
+        ["--baseline-eager", "--write-protect-code=off"],
     ],
     # Cover cases useful for tsan. Note that we test --ion-eager without
     # --ion-offthread-compile=off here, because it helps catch races.

js/xpconnect/src/XPCJSContext.cpp

@@ -969,6 +969,14 @@ static void LoadStartupJSPrefs(XPCJSContext* xpccx) {
           javascript_options_spectre_jit_to_cxx_calls_DoNotUseDirectly());
 #endif
 
+  bool writeProtectCode = true;
+  if (XRE_IsContentProcess()) {
+    writeProtectCode =
+        StaticPrefs::javascript_options_content_process_write_protect_code();
+  }
+  JS_SetGlobalJitCompilerOption(cx, JSJITCOMPILER_WRITE_PROTECT_CODE,
+                                writeProtectCode);
+
   JS_SetGlobalJitCompilerOption(
       cx, JSJITCOMPILER_WATCHTOWER_MEGAMORPHIC,
       StaticPrefs::

modules/libpref/init/StaticPrefList.yaml

@@ -7829,6 +7829,13 @@
   value: false
   mirror: always
 
+# Whether the W^X policy is enforced to mark JIT code pages as either writable
+# or executable but never both at the same time.
+- name: javascript.options.content_process_write_protect_code
+  type: bool
+  value: false
+  mirror: always
+
 # Whether to use the XPCOM thread pool for JS helper tasks.
 - name: javascript.options.external_thread_pool
   type: bool