Title: [252674] trunk/Source/_javascript_Core
- Revision
- 252674
- Author
- [email protected]
- Date
- 2019-11-19 18:28:52 -0800 (Tue, 19 Nov 2019)
Log Message
[JSC] MetadataTable::sizeInBytes should not touch m_rawBuffer in UnlinkedMetadataTable unless MetadataTable is linked to that UnlinkedMetadataTable
https://bugs.webkit.org/show_bug.cgi?id=204390
Reviewed by Mark Lam.
We have a race issue here. When calling MetadataTable::sizeInBytes, we call UnlinkedMetadataTable::sizeInBytes since we change the result based on
whether this MetadataTable is linked to this UnlinkedMetadataTable or not. The problem is that we are calling `UnlinkedMetadataTable::totalSize`
unconditionally in UnlinkedMetadataTable::sizeInBytes, and this is touching m_rawBuffer unconditionally. This is not correct since it is possible
that this m_rawBuffer is realloced while we are calling MetadataTable::sizeInBytes in GC thread.
1. The GC thread is calling MetadataTable::sizeInBytes for MetadataTable "A".
2. The main thread is destroying MetadataTable "B".
3. MetadataTable "B" is linked to UnlinkedMetadataTable "C".
4. MetadataTable "A" is pointing to UnlinkedMetadataTable "C".
5. "A" is touching UnlinkedMetadataTable::m_rawBuffer in "C", called from MetadataTable::sizeInBytes.
6. (2) destroys MetadataTable "B", and realloc UnlinkedMetadataTable::m_rawBuffer in "C".
7. (5) can touch already freed buffer.
This patch fixes UnlinkedMetadataTable::sizeInBytes: not touching m_rawBuffer unless it is owned by the caller's MetadataTable. We need to call
UnlinkedMetadataTable::sizeInBytes anyway since we need to adjust the result based on whether the caller MetadataTable is linked to this UnlinkedMetadataTable.
* bytecode/UnlinkedMetadataTableInlines.h:
(JSC::UnlinkedMetadataTable::sizeInBytes):
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (252673 => 252674)
--- trunk/Source/_javascript_Core/ChangeLog 2019-11-20 02:13:24 UTC (rev 252673)
+++ trunk/Source/_javascript_Core/ChangeLog 2019-11-20 02:28:52 UTC (rev 252674)
@@ -1,3 +1,29 @@
+2019-11-19 Yusuke Suzuki <[email protected]>
+
+ [JSC] MetadataTable::sizeInBytes should not touch m_rawBuffer in UnlinkedMetadataTable unless MetadataTable is linked to that UnlinkedMetadataTable
+ https://bugs.webkit.org/show_bug.cgi?id=204390
+
+ Reviewed by Mark Lam.
+
+ We have a race issue here. When calling MetadataTable::sizeInBytes, we call UnlinkedMetadataTable::sizeInBytes since we change the result based on
+ whether this MetadataTable is linked to this UnlinkedMetadataTable or not. The problem is that we are calling `UnlinkedMetadataTable::totalSize`
+ unconditionally in UnlinkedMetadataTable::sizeInBytes, and this is touching m_rawBuffer unconditionally. This is not correct since it is possible
+ that this m_rawBuffer is realloced while we are calling MetadataTable::sizeInBytes in GC thread.
+
+ 1. The GC thread is calling MetadataTable::sizeInBytes for MetadataTable "A".
+ 2. The main thread is destroying MetadataTable "B".
+ 3. MetadataTable "B" is linked to UnlinkedMetadataTable "C".
+ 4. MetadataTable "A" is pointing to UnlinkedMetadataTable "C".
+ 5. "A" is touching UnlinkedMetadataTable::m_rawBuffer in "C", called from MetadataTable::sizeInBytes.
+ 6. (2) destroys MetadataTable "B", and realloc UnlinkedMetadataTable::m_rawBuffer in "C".
+ 7. (5) can touch already freed buffer.
+
+ This patch fixes UnlinkedMetadataTable::sizeInBytes: not touching m_rawBuffer unless it is owned by the caller's MetadataTable. We need to call
+ UnlinkedMetadataTable::sizeInBytes anyway since we need to adjust the result based on whether the caller MetadataTable is linked to this UnlinkedMetadataTable.
+
+ * bytecode/UnlinkedMetadataTableInlines.h:
+ (JSC::UnlinkedMetadataTable::sizeInBytes):
+
2019-11-19 Fujii Hironori <[email protected]>
[JSC] DisallowVMReentry and DeferGC should use WTF::ThreadSpecific instead of using WTF::threadSpecificKeyCreate directly
Modified: trunk/Source/_javascript_Core/bytecode/UnlinkedMetadataTableInlines.h (252673 => 252674)
--- trunk/Source/_javascript_Core/bytecode/UnlinkedMetadataTableInlines.h 2019-11-20 02:13:24 UTC (rev 252673)
+++ trunk/Source/_javascript_Core/bytecode/UnlinkedMetadataTableInlines.h 2019-11-20 02:28:52 UTC (rev 252674)
@@ -88,7 +88,10 @@
// In this case, we return the size of the table minus the offset table,
// which was already accounted for in the UnlinkedCodeBlock.
- size_t result = totalSize();
+
+ // Be careful not to touch m_rawBuffer if this metadataTable is not owning it.
+ // It is possible that, m_rawBuffer is realloced in the other thread while we are accessing here.
+ size_t result = metadataTable.totalSize();
if (metadataTable.buffer() == buffer()) {
ASSERT(m_isLinked);
if (m_is32Bit)
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
