Changeset 211124 in webkit

Timestamp:

Jan 24, 2017, 5:04:26 PM (9 years ago)

Author:

msaboff@apple.com

Message:

InferredTypeTable entry manipulation is not TOCTOU race safe
​https://bugs.webkit.org/show_bug.cgi?id=167344

Reviewed by Filip Pizlo.

Made the accesses to table values safe from Time of Check,
Time of Use races with local temporary values.

Fixed point that we set an entry in the table to access the
current table entry instead of using the local entry. In that case,
we reload the now changed entry.

• runtime/InferredTypeTable.cpp:

(JSC::InferredTypeTable::visitChildren):
(JSC::InferredTypeTable::get):
(JSC::InferredTypeTable::willStoreValue):
(JSC::InferredTypeTable::makeTop):

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• runtime/InferredTypeTable.cpp (modified) (5 diffs)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2017-01-24  Michael Saboff  <msaboff@apple.com>
+
+        InferredTypeTable entry manipulation is not TOCTOU race safe
+        https://bugs.webkit.org/show_bug.cgi?id=167344
+
+        Reviewed by Filip Pizlo.
+
+        Made the accesses to table values safe from Time of Check,
+        Time of Use races with local temporary values.
+
+        Fixed point that we set an entry in the table to access the
+        current table entry instead of using the local entry.  In that case,
+        we reload the now changed entry.
+
+        * runtime/InferredTypeTable.cpp:
+        (JSC::InferredTypeTable::visitChildren):
+        (JSC::InferredTypeTable::get):
+        (JSC::InferredTypeTable::willStoreValue):
+        (JSC::InferredTypeTable::makeTop):
+
 2017-01-24  Filip Pizlo  <fpizlo@apple.com>
 

trunk/Source/JavaScriptCore/runtime/InferredTypeTable.cpp

@@ -58 +58 @@
     
     for (auto& entry : inferredTypeTable->m_table) {
-        if (!entry.value)
+        auto entryValue = entry.value;
+
+        if (!entryValue)
             continue;
-        if (entry.value->isRelevant())
-            visitor.append(entry.value);
+        if (entryValue->isRelevant())
+            visitor.append(entryValue);
         else
             entry.value.clear();
@@ -70 +72 @@
 {
     auto iter = m_table.find(uid);
-    if (iter == m_table.end() || !iter->value)
+    if (iter == m_table.end())
+        return nullptr;
+
+    InferredType* entryValue = iter->value.get();
+    if (!entryValue)
         return nullptr;
 
     // Take this opportunity to prune invalidated types.
-    if (!iter->value->isRelevant()) {
+    if (!entryValue->isRelevant()) {
         iter->value.clear();
         return nullptr;
     }
 
-    return iter->value.get();
+    return entryValue;
 }
 
@@ -100 +106 @@
     if (age == OldProperty) {
         TableType::iterator iter = m_table.find(propertyName.uid());
-        if (iter == m_table.end() || !iter->value)
+        if (iter == m_table.end())
             return false; // Absence on replace => top.
+
+        InferredType* entryValue = iter->value.get();
+        if (!entryValue)
+            return false;
         
-        if (iter->value->willStoreValue(vm, propertyName, value))
+        if (entryValue->willStoreValue(vm, propertyName, value))
             return true;
         
@@ -115 +125 @@
         result = m_table.add(propertyName.uid(), WriteBarrier<InferredType>());
     }
+    InferredType* entryValue = result.iterator->value.get();
+
     if (result.isNewEntry) {
         InferredType* inferredType = InferredType::create(vm);
         WTF::storeStoreFence();
         result.iterator->value.set(vm, this, inferredType);
-    } else if (!result.iterator->value)
+        entryValue = inferredType;
+    } else if (!entryValue)
         return false;
     
-    if (result.iterator->value->willStoreValue(vm, propertyName, value))
+    if (entryValue->willStoreValue(vm, propertyName, value))
         return true;
     
@@ -134 +147 @@
     if (age == OldProperty) {
         TableType::iterator iter = m_table.find(propertyName.uid());
-        if (iter == m_table.end() || !iter->value)
+        if (iter == m_table.end())
             return; // Absence on replace => top.
 
-        iter->value->makeTop(vm, propertyName);
+        InferredType* entryValue = iter->value.get();
+
+        if (!entryValue)
+            return;
+
+        entryValue->makeTop(vm, propertyName);
         iter->value.clear();
         return;