Changeset 213930 in webkit

Timestamp:

Mar 14, 2017, 12:29:26 PM (8 years ago)

Author:

mark.lam@apple.com

Message:

Add a null check in VMTraps::willDestroyVM() to handle a race condition.
​https://bugs.webkit.org/show_bug.cgi?id=169620

Reviewed by Filip Pizlo.

There exists a race between VMTraps::willDestroyVM() (which removed SignalSenders
from its m_signalSenders list) and SignalSender::send() (which removes itself
from the list). In the event that SignalSender::send() removes itself between
the time that VMTraps::willDestroyVM() checks if m_signalSenders is empty and the
time it takes a sender from m_signalSenders, VMTraps::willDestroyVM() may end up
with a NULL sender pointer. The fix is to add the missing null check before using
the sender pointer.

• runtime/VMTraps.cpp:

(JSC::VMTraps::willDestroyVM):
(JSC::VMTraps::fireTrap):

• runtime/VMTraps.h:

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• runtime/VMTraps.cpp (modified) (3 diffs)
• runtime/VMTraps.h (modified) (1 diff)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2017-03-14  Mark Lam  <mark.lam@apple.com>
+
+        Add a null check in VMTraps::willDestroyVM() to handle a race condition.
+        https://bugs.webkit.org/show_bug.cgi?id=169620
+
+        Reviewed by Filip Pizlo.
+
+        There exists a race between VMTraps::willDestroyVM() (which removed SignalSenders
+        from its m_signalSenders list) and SignalSender::send() (which removes itself
+        from the list).  In the event that SignalSender::send() removes itself between
+        the time that VMTraps::willDestroyVM() checks if m_signalSenders is empty and the
+        time it takes a sender from m_signalSenders, VMTraps::willDestroyVM() may end up
+        with a NULL sender pointer.  The fix is to add the missing null check before using
+        the sender pointer.
+
+        * runtime/VMTraps.cpp:
+        (JSC::VMTraps::willDestroyVM):
+        (JSC::VMTraps::fireTrap):
+        * runtime/VMTraps.h:
+
 2017-03-14  Mark Lam  <mark.lam@apple.com>
 

trunk/Source/JavaScriptCore/runtime/VMTraps.cpp

@@ -404 +404 @@
 void VMTraps::willDestroyVM()
 {
+    m_isShuttingDown = true;
+    WTF::storeStoreFence();
 #if ENABLE(SIGNAL_BASED_VM_TRAPS)
     while (!m_signalSenders.isEmpty()) {
@@ -414 +416 @@
             auto locker = holdLock(m_lock);
             sender = m_signalSenders.takeAny();
+            if (!sender)
+                break;
         }
         sender->willDestroyVM();
     }
+    ASSERT(m_signalSenders.isEmpty());
 #endif
 }
@@ -477 +482 @@
     {
         auto locker = holdLock(m_lock);
+        ASSERT(!m_isShuttingDown);
         setTrapForEvent(locker, eventType);
         m_needToInvalidatedCodeBlocks = true;

trunk/Source/JavaScriptCore/runtime/VMTraps.h

@@ -168 +168 @@
     };
     bool m_needToInvalidatedCodeBlocks { false };
+    bool m_isShuttingDown { false };
 
 #if ENABLE(SIGNAL_BASED_VM_TRAPS)