Gigacage disabling checks should handle the GIGACAGE_ALLOCATION_CAN_FAIL case properly.

https://bugs.webkit.org/show_bug.cgi?id=193292
<rdar://problem/46485450>

Reviewed by Yusuke Suzuki.

Source/bmalloc:

Previously, when GIGACAGE_ALLOCATION_CAN_FAIL is true, we allow the Gigacage to
be disabled if we fail to allocate memory for it.  However, Gigacage::primitiveGigacageDisabled()
still always assumes that the Gigacage is always enabled after ensureGigacage() is
called.

This patch updates Gigacage::primitiveGigacageDisabled() to allow the Gigacage to
already be disabled if GIGACAGE_ALLOCATION_CAN_FAIL is true and wasEnabled() is
false.

In this patch, we also put the wasEnabled flag in the 0th slot of the
g_gigacageBasePtrs buffer to ensure that it is also protected against writes just
like the Gigacage base pointers.

To achieve this, we do the following:
1. Added a reservedForFlags field in struct BasePtrs.
2. Added a ReservedForFlagsAndNotABasePtr Gigacage::Kind.
3. Added assertions to ensure that the BasePtrs::primitive is at the offset
   matching the offset computed from Gigacage::Primitive.  Ditto for
   BasePtrs::jsValue and Gigacage::JSValue.
4. Added assertions to ensure that Gigacage::ReservedForFlagsAndNotABasePtr is not
   used for fetching a Gigacage base pointer.
5. Added RELEASE_BASSERT_NOT_REACHED() to implement such assertions in bmalloc.

No test added because this issue requires Gigacage allocation to fail in order to
manifest.  I've tested it manually by modifying the code locally to force an
allocation failure.

* bmalloc/BAssert.h:
* bmalloc/Gigacage.cpp:
(Gigacage::ensureGigacage):
(Gigacage::primitiveGigacageDisabled):
* bmalloc/Gigacage.h:
(Gigacage::wasEnabled):
(Gigacage::setWasEnabled):
(Gigacage::name):
(Gigacage::basePtr):
(Gigacage::size):
* bmalloc/HeapKind.h:
(bmalloc::heapKind):

Source/JavaScriptCore:

* runtime/VM.h:
(JSC::VM::gigacageAuxiliarySpace):

Source/WTF:

Update the USE_SYSTEM_MALLOC version of Gigacage.h to match the bmalloc version.

* wtf/Gigacage.h:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@239787 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index 263008d..c3e7958 100644 (file)
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,14 @@
+2019-01-09  Mark Lam  <mark.lam@apple.com>
+
+        Gigacage disabling checks should handle the GIGACAGE_ALLOCATION_CAN_FAIL case properly.
+        https://bugs.webkit.org/show_bug.cgi?id=193292
+        <rdar://problem/46485450>
+
+        Reviewed by Yusuke Suzuki.
+
+        * runtime/VM.h:
+        (JSC::VM::gigacageAuxiliarySpace):
+
 2019-01-08  Keith Miller  <keith_miller@apple.com>
 
         builtins should be able to use async/await

diff --git a/Source/JavaScriptCore/runtime/VM.h b/Source/JavaScriptCore/runtime/VM.h
index f42c74d..ffd9fda 100644 (file)
--- a/Source/JavaScriptCore/runtime/VM.h
+++ b/Source/JavaScriptCore/runtime/VM.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2008-2018 Apple Inc. All rights reserved.
+ * Copyright (C) 2008-2019 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -346,6 +346,8 @@ public:
     ALWAYS_INLINE CompleteSubspace& gigacageAuxiliarySpace(Gigacage::Kind kind)
     {
         switch (kind) {
+        case Gigacage::ReservedForFlagsAndNotABasePtr:
+            RELEASE_ASSERT_NOT_REACHED();
         case Gigacage::Primitive:
             return primitiveGigacageAuxiliarySpace;
         case Gigacage::JSValue:

diff --git a/Source/WTF/ChangeLog b/Source/WTF/ChangeLog
index f8a87b0..66998e9 100644 (file)
--- a/Source/WTF/ChangeLog
+++ b/Source/WTF/ChangeLog
@@ -1,3 +1,15 @@
+2019-01-09  Mark Lam  <mark.lam@apple.com>
+
+        Gigacage disabling checks should handle the GIGACAGE_ALLOCATION_CAN_FAIL case properly.
+        https://bugs.webkit.org/show_bug.cgi?id=193292
+        <rdar://problem/46485450>
+
+        Reviewed by Yusuke Suzuki.
+
+        Update the USE_SYSTEM_MALLOC version of Gigacage.h to match the bmalloc version.
+
+        * wtf/Gigacage.h:
+
 2019-01-07  David Kilzer  <ddkilzer@apple.com>
 
         Prefer RetainPtr<NSObject> to RetainPtr<NSObject *>

diff --git a/Source/WTF/wtf/Gigacage.h b/Source/WTF/wtf/Gigacage.h
index b8b8f12..1b04831 100644 (file)
--- a/Source/WTF/wtf/Gigacage.h
+++ b/Source/WTF/wtf/Gigacage.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2017-2018 Apple Inc. All rights reserved.
+ * Copyright (C) 2017-2019 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -40,15 +40,20 @@ alignas(void*) extern WTF_EXPORT_PRIVATE char g_gigacageBasePtrs[GIGACAGE_BASE_P
 namespace Gigacage {
 
 struct BasePtrs {
+    uintptr_t reservedForFlags;
     void* primitive;
     void* jsValue;
 };
 
 enum Kind {
+    ReservedForFlagsAndNotABasePtr = 0,
     Primitive,
     JSValue,
 };
 
+static_assert(offsetof(BasePtrs, primitive) == Kind::Primitive * sizeof(void*), "");
+static_assert(offsetof(BasePtrs, jsValue) == Kind::JSValue * sizeof(void*), "");
+
 inline void ensureGigacage() { }
 inline void disablePrimitiveGigacage() { }
 inline bool shouldBeEnabled() { return false; }
@@ -65,6 +70,8 @@ inline bool canPrimitiveGigacageBeDisabled() { return true; }
 ALWAYS_INLINE const char* name(Kind kind)
 {
     switch (kind) {
+    case ReservedForFlagsAndNotABasePtr:
+        RELEASE_ASSERT_NOT_REACHED();
     case Primitive:
         return "Primitive";
     case JSValue:
@@ -77,6 +84,8 @@ ALWAYS_INLINE const char* name(Kind kind)
 ALWAYS_INLINE void*& basePtr(BasePtrs& basePtrs, Kind kind)
 {
     switch (kind) {
+    case ReservedForFlagsAndNotABasePtr:
+        RELEASE_ASSERT_NOT_REACHED();
     case Primitive:
         return basePtrs.primitive;
     case JSValue:

diff --git a/Source/bmalloc/ChangeLog b/Source/bmalloc/ChangeLog
index ae0fde2..a2c7064 100644 (file)
--- a/Source/bmalloc/ChangeLog
+++ b/Source/bmalloc/ChangeLog
@@ -1,3 +1,51 @@
+2019-01-09  Mark Lam  <mark.lam@apple.com>
+
+        Gigacage disabling checks should handle the GIGACAGE_ALLOCATION_CAN_FAIL case properly.
+        https://bugs.webkit.org/show_bug.cgi?id=193292
+        <rdar://problem/46485450>
+
+        Reviewed by Yusuke Suzuki.
+
+        Previously, when GIGACAGE_ALLOCATION_CAN_FAIL is true, we allow the Gigacage to
+        be disabled if we fail to allocate memory for it.  However, Gigacage::primitiveGigacageDisabled()
+        still always assumes that the Gigacage is always enabled after ensureGigacage() is
+        called.
+
+        This patch updates Gigacage::primitiveGigacageDisabled() to allow the Gigacage to
+        already be disabled if GIGACAGE_ALLOCATION_CAN_FAIL is true and wasEnabled() is
+        false.
+
+        In this patch, we also put the wasEnabled flag in the 0th slot of the
+        g_gigacageBasePtrs buffer to ensure that it is also protected against writes just
+        like the Gigacage base pointers.
+
+        To achieve this, we do the following:
+        1. Added a reservedForFlags field in struct BasePtrs.
+        2. Added a ReservedForFlagsAndNotABasePtr Gigacage::Kind.
+        3. Added assertions to ensure that the BasePtrs::primitive is at the offset
+           matching the offset computed from Gigacage::Primitive.  Ditto for
+           BasePtrs::jsValue and Gigacage::JSValue.
+        4. Added assertions to ensure that Gigacage::ReservedForFlagsAndNotABasePtr is not
+           used for fetching a Gigacage base pointer.
+        5. Added RELEASE_BASSERT_NOT_REACHED() to implement such assertions in bmalloc.
+
+        No test added because this issue requires Gigacage allocation to fail in order to
+        manifest.  I've tested it manually by modifying the code locally to force an
+        allocation failure.
+
+        * bmalloc/BAssert.h:
+        * bmalloc/Gigacage.cpp:
+        (Gigacage::ensureGigacage):
+        (Gigacage::primitiveGigacageDisabled):
+        * bmalloc/Gigacage.h:
+        (Gigacage::wasEnabled):
+        (Gigacage::setWasEnabled):
+        (Gigacage::name):
+        (Gigacage::basePtr):
+        (Gigacage::size):
+        * bmalloc/HeapKind.h:
+        (bmalloc::heapKind):
+
 2018-12-15  Yusuke Suzuki  <yusukesuzuki@slowstart.org>
 
         Unreviewed, suppress warnings in Linux

diff --git a/Source/bmalloc/bmalloc/BAssert.h b/Source/bmalloc/bmalloc/BAssert.h
index 572d4b5..10e4518 100644 (file)
--- a/Source/bmalloc/bmalloc/BAssert.h
+++ b/Source/bmalloc/bmalloc/BAssert.h
@@ -81,6 +81,7 @@
 } while (0)
 
 #define RELEASE_BASSERT(x) BASSERT_IMPL(x)
+#define RELEASE_BASSERT_NOT_REACHED() BCRASH()
 
 #if BUSE(OS_LOG)
 #define BMALLOC_LOGGING_PREFIX "bmalloc: "

diff --git a/Source/bmalloc/bmalloc/Gigacage.cpp b/Source/bmalloc/bmalloc/Gigacage.cpp
index 434c519..5b925c4 100644 (file)
--- a/Source/bmalloc/bmalloc/Gigacage.cpp
+++ b/Source/bmalloc/bmalloc/Gigacage.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2017-2018 Apple Inc. All rights reserved.
+ * Copyright (C) 2017-2019 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -42,14 +42,16 @@
 // If this were less than 32GB, those OOB accesses could reach outside of the cage.
 #define GIGACAGE_RUNWAY (32llu * 1024 * 1024 * 1024)
 
+// Note: g_gigacageBasePtrs[0] is reserved for storing the wasEnabled flag.
+// The first gigacageBasePtr will start at g_gigacageBasePtrs[sizeof(void*)].
+// This is done so that the wasEnabled flag will also be protected along with the
+// gigacageBasePtrs.
 alignas(GIGACAGE_BASE_PTRS_SIZE) char g_gigacageBasePtrs[GIGACAGE_BASE_PTRS_SIZE];
 
 using namespace bmalloc;
 
 namespace Gigacage {
 
-bool g_wasEnabled;
-
 namespace {
 
 bool s_isDisablingPrimitiveGigacageDisabled;
@@ -103,6 +105,8 @@ struct PrimitiveDisableCallbacks {
 size_t runwaySize(Kind kind)
 {
     switch (kind) {
+    case Kind::ReservedForFlagsAndNotABasePtr:
+        RELEASE_BASSERT_NOT_REACHED();
     case Kind::Primitive:
         return static_cast<size_t>(GIGACAGE_RUNWAY);
     case Kind::JSValue:
@@ -126,7 +130,7 @@ void ensureGigacage()
             
             Kind shuffledKinds[numKinds];
             for (unsigned i = 0; i < numKinds; ++i)
-                shuffledKinds[i] = static_cast<Kind>(i);
+                shuffledKinds[i] = static_cast<Kind>(i + 1); // + 1 to skip Kind::ReservedForFlagsAndNotABasePtr.
             
             // We just go ahead and assume that 64 bits is enough randomness. That's trivially true right
             // now, but would stop being true if we went crazy with gigacages. Based on my math, 21 is the
@@ -182,8 +186,8 @@ void ensureGigacage()
             }
             
             vmDeallocatePhysicalPages(base, totalSize);
+            setWasEnabled();
             protectGigacageBasePtrs();
-            g_wasEnabled = true;
         });
 #endif // GIGACAGE_ENABLED
 }
@@ -236,6 +240,9 @@ void removePrimitiveDisableCallback(void (*function)(void*), void* argument)
 
 static void primitiveGigacageDisabled(void*)
 {
+    if (GIGACAGE_ALLOCATION_CAN_FAIL && !wasEnabled())
+        return;
+
     static bool s_false;
     fprintf(stderr, "FATAL: Primitive gigacage disabled, but we don't want that in this process.\n");
     if (!s_false)

diff --git a/Source/bmalloc/bmalloc/Gigacage.h b/Source/bmalloc/bmalloc/Gigacage.h
index e7789a4..eab80fd 100644 (file)
--- a/Source/bmalloc/bmalloc/Gigacage.h
+++ b/Source/bmalloc/bmalloc/Gigacage.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2017 Apple Inc. All rights reserved.
+ * Copyright (C) 2017-2019 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -75,19 +75,24 @@ extern "C" alignas(GIGACAGE_BASE_PTRS_SIZE) BEXPORT char g_gigacageBasePtrs[GIGA
 
 namespace Gigacage {
 
-extern BEXPORT bool g_wasEnabled;
-BINLINE bool wasEnabled() { return g_wasEnabled; }
+BINLINE bool wasEnabled() { return g_gigacageBasePtrs[0]; }
+BINLINE void setWasEnabled() { g_gigacageBasePtrs[0] = true; }
 
 struct BasePtrs {
+    uintptr_t reservedForFlags;
     void* primitive;
     void* jsValue;
 };
 
 enum Kind {
+    ReservedForFlagsAndNotABasePtr = 0,
     Primitive,
     JSValue,
 };
 
+static_assert(offsetof(BasePtrs, primitive) == Kind::Primitive * sizeof(void*), "");
+static_assert(offsetof(BasePtrs, jsValue) == Kind::JSValue * sizeof(void*), "");
+
 static constexpr unsigned numKinds = 2;
 
 BEXPORT void ensureGigacage();
@@ -107,6 +112,8 @@ inline bool canPrimitiveGigacageBeDisabled() { return !isDisablingPrimitiveGigac
 BINLINE const char* name(Kind kind)
 {
     switch (kind) {
+    case ReservedForFlagsAndNotABasePtr:
+        RELEASE_BASSERT_NOT_REACHED();
     case Primitive:
         return "Primitive";
     case JSValue:
@@ -119,6 +126,8 @@ BINLINE const char* name(Kind kind)
 BINLINE void*& basePtr(BasePtrs& basePtrs, Kind kind)
 {
     switch (kind) {
+    case ReservedForFlagsAndNotABasePtr:
+        RELEASE_BASSERT_NOT_REACHED();
     case Primitive:
         return basePtrs.primitive;
     case JSValue:
@@ -146,6 +155,8 @@ BINLINE bool isEnabled(Kind kind)
 BINLINE size_t size(Kind kind)
 {
     switch (kind) {
+    case ReservedForFlagsAndNotABasePtr:
+        RELEASE_BASSERT_NOT_REACHED();
     case Primitive:
         return static_cast<size_t>(PRIMITIVE_GIGACAGE_SIZE);
     case JSValue:

diff --git a/Source/bmalloc/bmalloc/HeapKind.h b/Source/bmalloc/bmalloc/HeapKind.h
index 9e1af6c..7d7c9ba 100644 (file)
--- a/Source/bmalloc/bmalloc/HeapKind.h
+++ b/Source/bmalloc/bmalloc/HeapKind.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2017 Apple Inc. All rights reserved.
+ * Copyright (C) 2017-2019 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -70,6 +70,8 @@ BINLINE Gigacage::Kind gigacageKind(HeapKind kind)
 BINLINE HeapKind heapKind(Gigacage::Kind kind)
 {
     switch (kind) {
+    case Gigacage::ReservedForFlagsAndNotABasePtr:
+        RELEASE_BASSERT_NOT_REACHED();
     case Gigacage::Primitive:
         return HeapKind::PrimitiveGigacage;
     case Gigacage::JSValue: