WebKit Bugzilla
Attachment 358741 Details for
Bug 193292
: Gigacage disabling checks should handle the GIGACAGE_ALLOCATION_CAN_FAIL case properly.
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
proposed patch.
bug-193292.patch (text/plain), 12.08 KB, created by
Mark Lam
on 2019-01-09 14:39:21 PST
(
hide
)
Description:
proposed patch.
Filename:
MIME Type:
Creator:
Mark Lam
Created:
2019-01-09 14:39:21 PST
Size:
12.08 KB
patch
obsolete
>Index: Source/JavaScriptCore/ChangeLog >=================================================================== >--- Source/JavaScriptCore/ChangeLog (revision 239781) >+++ Source/JavaScriptCore/ChangeLog (working copy) >@@ -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 NOBODY (OOPS!). >+ >+ * runtime/VM.h: >+ (JSC::VM::gigacageAuxiliarySpace): >+ > 2019-01-08 Keith Miller <keith_miller@apple.com> > > builtins should be able to use async/await >Index: Source/JavaScriptCore/runtime/VM.h >=================================================================== >--- Source/JavaScriptCore/runtime/VM.h (revision 239781) >+++ Source/JavaScriptCore/runtime/VM.h (working copy) >@@ -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: >Index: Source/WTF/ChangeLog >=================================================================== >--- Source/WTF/ChangeLog (revision 239781) >+++ Source/WTF/ChangeLog (working copy) >@@ -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 NOBODY (OOPS!). >+ >+ 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 *> >Index: Source/WTF/wtf/Gigacage.h >=================================================================== >--- Source/WTF/wtf/Gigacage.h (revision 239781) >+++ Source/WTF/wtf/Gigacage.h (working copy) >@@ -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 > 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 canPrimitiveGigacageBeDisabl > 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: >Index: Source/bmalloc/ChangeLog >=================================================================== >--- Source/bmalloc/ChangeLog (revision 239786) >+++ Source/bmalloc/ChangeLog (working copy) >@@ -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 NOBODY (OOPS!). >+ >+ 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 >Index: Source/bmalloc/bmalloc/BAssert.h >=================================================================== >--- Source/bmalloc/bmalloc/BAssert.h (revision 239781) >+++ Source/bmalloc/bmalloc/BAssert.h (working copy) >@@ -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: " >Index: Source/bmalloc/bmalloc/Gigacage.cpp >=================================================================== >--- Source/bmalloc/bmalloc/Gigacage.cpp (revision 239781) >+++ Source/bmalloc/bmalloc/Gigacage.cpp (working copy) >@@ -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 > > 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) >Index: Source/bmalloc/bmalloc/Gigacage.h >=================================================================== >--- Source/bmalloc/bmalloc/Gigacage.h (revision 239781) >+++ Source/bmalloc/bmalloc/Gigacage.h (working copy) >@@ -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_SI > > 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 canPrimitiveGigacageBeDisabl > 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: >Index: Source/bmalloc/bmalloc/HeapKind.h >=================================================================== >--- Source/bmalloc/bmalloc/HeapKind.h (revision 239781) >+++ Source/bmalloc/bmalloc/HeapKind.h (working copy) >@@ -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(Heap > BINLINE HeapKind heapKind(Gigacage::Kind kind) > { > switch (kind) { >+ case Gigacage::ReservedForFlagsAndNotABasePtr: >+ RELEASE_BASSERT_NOT_REACHED(); > case Gigacage::Primitive: > return HeapKind::PrimitiveGigacage; > case Gigacage::JSValue:
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Flags:
ysuzuki
:
review+
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 193292
:
358729
| 358741