[flags] Restrict the types we use for FlagValue

Add static asserts that we only use specific types for flag values.
Also, document that string values are not be frozen yet, and add TODOs
to fix that.

R=cbruni@chromium.org

Bug: v8:12887
Change-Id: I7367108810f0c6463509f744c5cefd9392c469fb
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3852487Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/main@{#82714}

src/flags/flags.cc

@@ -769,7 +769,13 @@ int FlagList::SetFlagsFromString(const char* str, size_t len) {
 
 // static
 void FlagList::FreezeFlags() {
+  // Disallow changes via the API by setting {flags_frozen}.
   flags_frozen.store(true, std::memory_order_relaxed);
+  // Also memory-protect the memory that holds the flag values. This makes it
+  // impossible for attackers to overwrite values, except if they find a way to
+  // first unprotect the memory again.
+  // Note that for string flags we only protect the pointer itself, but not the
+  // string storage. TODO(12887): Fix this.
   base::OS::SetDataReadOnly(&v8_flags, sizeof(v8_flags));
 }
 

src/flags/flags.h

@@ -21,6 +21,22 @@ namespace v8::internal {
 // The value of a single flag (this is the type of all v8_flags.* fields).
 template <typename T>
 class FlagValue {
+  // {FlagValue} types will be memory-protected in {FlagList::FreezeFlags}.
+  // We currently allow the following types to be used for flags:
+  // - Arithmetic types like bool, int, size_t, double; those will trivially be
+  //   protected.
+  // - base::Optional<bool>, which is basically a POD, and can also be
+  //   protected.
+  // - const char*, for which we currently do not protect the actual string
+  //   value. TODO(12887): Also protect the string storage.
+  //
+  // Other types can be added as needed, after checking that memory protection
+  // works for them.
+  static_assert(std::is_same_v<std::decay_t<T>, T>);
+  static_assert(std::is_arithmetic_v<T> ||
+                std::is_same_v<base::Optional<bool>, T> ||
+                std::is_same_v<const char*, T>);
+
  public:
   explicit constexpr FlagValue(T value) : value_(value) {}
 
@@ -106,7 +122,6 @@ class V8_EXPORT_PRIVATE FlagList {
   static int SetFlagsFromString(const char* str, size_t len);
 
   // Freeze the current flag values (disallow changes via the API).
-  // TODO(12887): Actually write-protect the flags.
   static void FreezeFlags();
 
   // Returns true if the flags are currently frozen.