Title: [260913] trunk/Source
- Revision
- 260913
- Author
- [email protected]
- Date
- 2020-04-29 13:39:03 -0700 (Wed, 29 Apr 2020)
Log Message
Freezing of Gigacage and JSC Configs should be thread safe.
https://bugs.webkit.org/show_bug.cgi?id=211201
<rdar://problem/62597619>
Reviewed by Yusuke Suzuki.
Source/bmalloc:
* bmalloc/Gigacage.cpp:
(Gigacage::bmalloc::permanentlyFreezeGigacageConfig):
Source/_javascript_Core:
If a client creates multiple VM instances in different threads concurrently, the
following race can occur:
Config::permanentlyFreeze() contains the following code:
if (!g_jscConfig.isPermanentlyFrozen) // Point P1
g_jscConfig.isPermanentlyFrozen = true; // Point P2
Let's say there are 2 threads T1 and T2.
1. T1 creates a VM and gets to point P1, and sees that g_jscConfig.isPermanentlyFrozen is not set.
T1 is about to execute P2 when it gets pre-empted.
2. T2 creates a VM and gets to point P1, and sees that g_jscConfig.isPermanentlyFrozen is not set.
T2 proceeds to point P2 and sets g_jscConfig.isPermanentlyFrozen to true.
T2 goes on to freeze the Config and makes it not writable.
3. T1 gets to run again, and proceeds to point P2.
T1 tries to set g_jscConfig.isPermanentlyFrozen to true.
But because the Config has been frozen against writes, the write to
g_jscConfig.isPermanentlyFrozen results in a crash.
This is a classic TOCTOU bug. The fix is simply to ensure that only one thread
can enter Config::permanentlyFreeze() at a time.
Ditto for Gigacage::permanentlyFreezeGigacageConfig().
* runtime/JSCConfig.cpp:
(JSC::Config::permanentlyFreeze):
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (260912 => 260913)
--- trunk/Source/_javascript_Core/ChangeLog 2020-04-29 20:16:52 UTC (rev 260912)
+++ trunk/Source/_javascript_Core/ChangeLog 2020-04-29 20:39:03 UTC (rev 260913)
@@ -1,3 +1,41 @@
+2020-04-29 Mark Lam <[email protected]>
+
+ Freezing of Gigacage and JSC Configs should be thread safe.
+ https://bugs.webkit.org/show_bug.cgi?id=211201
+ <rdar://problem/62597619>
+
+ Reviewed by Yusuke Suzuki.
+
+ If a client creates multiple VM instances in different threads concurrently, the
+ following race can occur:
+
+ Config::permanentlyFreeze() contains the following code:
+
+ if (!g_jscConfig.isPermanentlyFrozen) // Point P1
+ g_jscConfig.isPermanentlyFrozen = true; // Point P2
+
+ Let's say there are 2 threads T1 and T2.
+
+ 1. T1 creates a VM and gets to point P1, and sees that g_jscConfig.isPermanentlyFrozen is not set.
+ T1 is about to execute P2 when it gets pre-empted.
+
+ 2. T2 creates a VM and gets to point P1, and sees that g_jscConfig.isPermanentlyFrozen is not set.
+ T2 proceeds to point P2 and sets g_jscConfig.isPermanentlyFrozen to true.
+ T2 goes on to freeze the Config and makes it not writable.
+
+ 3. T1 gets to run again, and proceeds to point P2.
+ T1 tries to set g_jscConfig.isPermanentlyFrozen to true.
+ But because the Config has been frozen against writes, the write to
+ g_jscConfig.isPermanentlyFrozen results in a crash.
+
+ This is a classic TOCTOU bug. The fix is simply to ensure that only one thread
+ can enter Config::permanentlyFreeze() at a time.
+
+ Ditto for Gigacage::permanentlyFreezeGigacageConfig().
+
+ * runtime/JSCConfig.cpp:
+ (JSC::Config::permanentlyFreeze):
+
2020-04-29 Yusuke Suzuki <[email protected]>
[JSC] JSStringJoiner is missing BigInt handling
Modified: trunk/Source/_javascript_Core/runtime/JSCConfig.cpp (260912 => 260913)
--- trunk/Source/_javascript_Core/runtime/JSCConfig.cpp 2020-04-29 20:16:52 UTC (rev 260912)
+++ trunk/Source/_javascript_Core/runtime/JSCConfig.cpp 2020-04-29 20:39:03 UTC (rev 260913)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2019 Apple Inc. All rights reserved.
+ * Copyright (C) 2019-2020 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -26,6 +26,7 @@
#include "config.h"
#include "JSCConfig.h"
+#include <wtf/Lock.h>
#include <wtf/ResourceUsage.h>
#include <wtf/StdLibExtras.h>
@@ -53,6 +54,9 @@
void Config::permanentlyFreeze()
{
+ static Lock configLock;
+ auto locker = holdLock(configLock);
+
RELEASE_ASSERT(roundUpToMultipleOf(pageSize(), ConfigSizeToProtect) == ConfigSizeToProtect);
if (!g_jscConfig.isPermanentlyFrozen)
Modified: trunk/Source/bmalloc/ChangeLog (260912 => 260913)
--- trunk/Source/bmalloc/ChangeLog 2020-04-29 20:16:52 UTC (rev 260912)
+++ trunk/Source/bmalloc/ChangeLog 2020-04-29 20:39:03 UTC (rev 260913)
@@ -1,3 +1,14 @@
+2020-04-29 Mark Lam <[email protected]>
+
+ Freezing of Gigacage and JSC Configs should be thread safe.
+ https://bugs.webkit.org/show_bug.cgi?id=211201
+ <rdar://problem/62597619>
+
+ Reviewed by Yusuke Suzuki.
+
+ * bmalloc/Gigacage.cpp:
+ (Gigacage::bmalloc::permanentlyFreezeGigacageConfig):
+
2020-04-25 Darin Adler <[email protected]>
[Cocoa] Deal with another round of Xcode upgrade checks
Modified: trunk/Source/bmalloc/bmalloc/Gigacage.cpp (260912 => 260913)
--- trunk/Source/bmalloc/bmalloc/Gigacage.cpp 2020-04-29 20:16:52 UTC (rev 260912)
+++ trunk/Source/bmalloc/bmalloc/Gigacage.cpp 2020-04-29 20:39:03 UTC (rev 260913)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2017-2019 Apple Inc. All rights reserved.
+ * Copyright (C) 2017-2020 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -27,6 +27,7 @@
#include "CryptoRandom.h"
#include "Environment.h"
+#include "Mutex.h"
#include "ProcessCheck.h"
#include "StaticPerProcess.h"
#include "VMAllocate.h"
@@ -33,7 +34,6 @@
#include "Vector.h"
#include "bmalloc.h"
#include <cstdio>
-#include <mutex>
#if BOS(DARWIN)
#include <mach/mach.h>
@@ -117,6 +117,9 @@
static void permanentlyFreezeGigacageConfig()
{
+ static Mutex configLock;
+ LockHolder locking(configLock);
+
if (!g_gigacageConfig.isPermanentlyFrozen) {
unfreezeGigacageConfig();
g_gigacageConfig.isPermanentlyFrozen = true;
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
