Further tighten the Partition Alloc RNG API.

Bug: 984742
Change-Id: I35a17fc8b923c949125ff63511c424143f3f80e7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1761402
Commit-Queue: Chris Palmer <palmer@chromium.org>
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#688711}

base/allocator/partition_allocator/address_space_randomization.cc

@@ -19,11 +19,11 @@
 namespace base {
 
 void* GetRandomPageBase() {
-  uintptr_t random = static_cast<uintptr_t>(RandomValue(GetRandomContext()));
+  uintptr_t random = static_cast<uintptr_t>(RandomValue());
 
 #if defined(ARCH_CPU_64_BITS)
   random <<= 32ULL;
-  random |= static_cast<uintptr_t>(RandomValue(GetRandomContext()));
+  random |= static_cast<uintptr_t>(RandomValue());
 
 // The kASLRMask and kASLROffset constants will be suitable for the
 // OS and build configuration.

base/allocator/partition_allocator/address_space_randomization_unittest.cc

@@ -101,8 +101,8 @@ TEST(AddressSpaceRandomizationTest, Predictable) {
   if (!mask)
     return;
 
-  const uintptr_t kInitialSeed = 0xfeed5eedULL;
-  base::SetRandomPageBaseSeed(kInitialSeed);
+  const uint64_t kInitialSeed = 0xfeed5eedULL;
+  base::SetMmapSeedForTesting(kInitialSeed);
 
   std::vector<uintptr_t> sequence;
   for (size_t i = 0; i < kSamples; ++i) {
@@ -110,7 +110,7 @@ TEST(AddressSpaceRandomizationTest, Predictable) {
     sequence.push_back(address);
   }
 
-  base::SetRandomPageBaseSeed(kInitialSeed);
+  base::SetMmapSeedForTesting(kInitialSeed);
 
   for (size_t i = 0; i < kSamples; ++i) {
     uintptr_t address = reinterpret_cast<uintptr_t>(base::GetRandomPageBase());

base/allocator/partition_allocator/random.cc

@@ -5,13 +5,14 @@
 #include "base/allocator/partition_allocator/random.h"
 
 #include "base/allocator/partition_allocator/spin_lock.h"
+#include "base/logging.h"
 #include "base/no_destructor.h"
 #include "base/rand_util.h"
 
 namespace base {
 
 // This is the same PRNG as used by tcmalloc for mapping address randomness;
-// see http://burtleburtle.net/bob/rand/smallprng.html
+// see http://burtleburtle.net/bob/rand/smallprng.html.
 struct RandomContext {
   subtle::SpinLock lock;
   bool initialized;
@@ -21,12 +22,11 @@ struct RandomContext {
   uint32_t d;
 };
 
-RandomContext* GetRandomContext() {
-  static NoDestructor<RandomContext> s_RandomContext;
-  return s_RandomContext.get();
-}
+namespace {
 
-uint32_t RandomValue(RandomContext* x) {
+RandomContext* GetRandomContext() {
+  static NoDestructor<RandomContext> g_random_context;
+  RandomContext* x = g_random_context.get();
   subtle::SpinLock::Guard guard(x->lock);
   if (UNLIKELY(!x->initialized)) {
     const uint64_t r1 = RandUint64();
@@ -37,7 +37,14 @@ uint32_t RandomValue(RandomContext* x) {
     x->d = static_cast<uint32_t>(r2 >> 32);
     x->initialized = true;
   }
+  return x;
+}
 
+}  // namespace
+
+uint32_t RandomValue() {
+  RandomContext* x = GetRandomContext();
+  subtle::SpinLock::Guard guard(x->lock);
 #define rot(x, k) (((x) << (k)) | ((x) >> (32 - (k))))
   uint32_t e = x->a - rot(x->b, 27);
   x->a = x->b ^ rot(x->c, 17);
@@ -48,7 +55,7 @@ uint32_t RandomValue(RandomContext* x) {
 #undef rot
 }
 
-void SetRandomPageBaseSeed(int64_t seed) {
+void SetMmapSeedForTesting(uint64_t seed) {
   RandomContext* x = GetRandomContext();
   subtle::SpinLock::Guard guard(x->lock);
   x->a = x->b = static_cast<uint32_t>(seed);

base/allocator/partition_allocator/random.h

@@ -11,23 +11,16 @@
 
 namespace base {
 
-struct RandomContext;
+// Returns a random value. The generator's internal state is initialized with
+// `base::RandUint64` which is very unpredictable, but which is expensive due to
+// the need to call into the kernel. Therefore this generator uses a fast,
+// entirely user-space function after initialization.
+uint32_t RandomValue();
 
-// TODO(crbug.com/984742): This doesn't need to be part of the interface, and
-// can be an implementation detail of `RandomValue`.
-//
-// Returns a pointer to the global `RandomContext`. If the context has not been
-// initialized yet, initializes it with a random starting state.
-RandomContext* GetRandomContext();
-
-uint32_t RandomValue(RandomContext* x);
-
-// TODO(crbug.com/984742): Rename this to `SetRandomSeedForTesting`.
-//
 // Sets the seed for the random number generator to a known value, to cause the
 // RNG to generate a predictable sequence of outputs. May be called multiple
 // times.
-BASE_EXPORT void SetRandomPageBaseSeed(int64_t seed);
+BASE_EXPORT void SetMmapSeedForTesting(uint64_t seed);
 
 }  // namespace base
 

gin/v8_platform.cc

@@ -207,7 +207,7 @@ class PageAllocator : public v8::PageAllocator {
   size_t CommitPageSize() override { return base::kSystemPageSize; }
 
   void SetRandomMmapSeed(int64_t seed) override {
-    base::SetRandomPageBaseSeed(seed);
+    base::SetMmapSeedForTesting(seed);
   }
 
   void* GetRandomMmapAddr() override { return base::GetRandomPageBase(); }