Changeset 180716 in webkit


Ignore:
Timestamp:
Feb 26, 2015 5:25:21 PM (9 years ago)
Author:
mark.lam@apple.com
Message:

MachineThreads::Thread clean up has a use after free race condition.
<https://webkit.org/b/141990>

Reviewed by Filip Pizlo.

MachineThreads::Thread clean up relies on the clean up mechanism
implemented in _pthread_tsd_cleanup_key(), which looks like this:

void _pthread_tsd_cleanup_key(pthread_t self, pthread_key_t key)
{

void (*destructor)(void *);
if (_pthread_key_get_destructor(key, &destructor)) {

void ptr = &self->tsd[key];
void *value = *ptr;

=== Start of window for the bug to manifest =================

At this point, this thread has cached "destructor" and "value"
(which is a MachineThreads*). If the VM gets destructed (along
with its MachineThreads registry) by another thread, then this
thread will have no way of knowing that the MachineThreads* is
now pointing to freed memory. Calling the destructor below will
therefore result in a use after free scenario when it tries to
access the MachineThreads' data members.

if (value) {

*ptr = NULL;
if (destructor) {

=== End of window for the bug to manifest ==================

destructor(value);

}

}

}

}

The fix is to add each active MachineThreads to an ActiveMachineThreadsManager,
and always check if the manager still contains that MachineThreads object
before we call removeCurrentThread() on it. When MachineThreads is destructed,
it will remove itself from the manager. The add, remove, and checking
operations are all synchronized on the manager's lock, thereby ensuring that
the MachineThreads object, if found in the manager, will remain alive for the
duration of time we call removeCurrentThread() on it.

There's also possible for the MachineThreads object to already be destructed
and another one happened to have been instantiated at the same address.
Hence, we should only remove the exiting thread if it is found in the
MachineThreads object.

There is no test for this issue because this bug requires a race condition
between 2 threads where:

  1. Thread B, which had previously used the VM, exiting and getting to the bug window shown in _pthread_tsd_cleanup_key() above.
  2. Thread A destructing the VM (and its MachineThreads object) within that window of time before Thread B calls the destructor.

It is not possible to get a reliable test case without invasively
instrumenting _pthread_tsd_cleanup_key() or MachineThreads::removeCurrentThread()
to significantly increase that window of opportunity.

  • heap/MachineStackMarker.cpp:

(JSC::ActiveMachineThreadsManager::Locker::Locker):
(JSC::ActiveMachineThreadsManager::add):
(JSC::ActiveMachineThreadsManager::remove):
(JSC::ActiveMachineThreadsManager::contains):
(JSC::ActiveMachineThreadsManager::ActiveMachineThreadsManager):
(JSC::activeMachineThreadsManager):
(JSC::MachineThreads::MachineThreads):
(JSC::MachineThreads::~MachineThreads):
(JSC::MachineThreads::removeThread):
(JSC::MachineThreads::removeThreadIfFound):
(JSC::MachineThreads::removeCurrentThread): Deleted.

  • heap/MachineStackMarker.h:
Location:
trunk/Source/JavaScriptCore
Files:
3 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/JavaScriptCore/ChangeLog

    r180715 r180716  
     12015-02-26  Mark Lam  <mark.lam@apple.com>
     2
     3        MachineThreads::Thread clean up has a use after free race condition.
     4        <https://webkit.org/b/141990>
     5
     6        Reviewed by Filip Pizlo.
     7
     8        MachineThreads::Thread clean up relies on the clean up mechanism
     9        implemented in _pthread_tsd_cleanup_key(), which looks like this:
     10
     11        void _pthread_tsd_cleanup_key(pthread_t self, pthread_key_t key)
     12        {
     13            void (*destructor)(void *);
     14            if (_pthread_key_get_destructor(key, &destructor)) {
     15                void **ptr = &self->tsd[key];
     16                void *value = *ptr;
     17
     18            // === Start of window for the bug to manifest =================
     19
     20                // At this point, this thread has cached "destructor" and "value"
     21                // (which is a MachineThreads*).  If the VM gets destructed (along
     22                // with its MachineThreads registry) by another thread, then this
     23                // thread will have no way of knowing that the MachineThreads* is
     24                // now pointing to freed memory.  Calling the destructor below will
     25                // therefore result in a use after free scenario when it tries to
     26                // access the MachineThreads' data members.
     27
     28                if (value) {
     29                    *ptr = NULL;
     30                    if (destructor) {
     31
     32            // === End of window for the bug to manifest ==================
     33
     34                        destructor(value);
     35                    }
     36                }
     37            }
     38        }
     39
     40        The fix is to add each active MachineThreads to an ActiveMachineThreadsManager,
     41        and always check if the manager still contains that MachineThreads object
     42        before we call removeCurrentThread() on it.  When MachineThreads is destructed,
     43        it will remove itself from the manager.  The add, remove, and checking
     44        operations are all synchronized on the manager's lock, thereby ensuring that
     45        the MachineThreads object, if found in the manager, will remain alive for the
     46        duration of time we call removeCurrentThread() on it.
     47
     48        There's also possible for the MachineThreads object to already be destructed
     49        and another one happened to have been instantiated at the same address.
     50        Hence, we should only remove the exiting thread if it is found in the
     51        MachineThreads object.
     52
     53        There is no test for this issue because this bug requires a race condition
     54        between 2 threads where:
     55        1. Thread B, which had previously used the VM, exiting and
     56           getting to the bug window shown in _pthread_tsd_cleanup_key() above.
     57        2. Thread A destructing the VM (and its MachineThreads object)
     58           within that window of time before Thread B calls the destructor.
     59
     60        It is not possible to get a reliable test case without invasively
     61        instrumenting _pthread_tsd_cleanup_key() or MachineThreads::removeCurrentThread()
     62        to significantly increase that window of opportunity.
     63
     64        * heap/MachineStackMarker.cpp:
     65        (JSC::ActiveMachineThreadsManager::Locker::Locker):
     66        (JSC::ActiveMachineThreadsManager::add):
     67        (JSC::ActiveMachineThreadsManager::remove):
     68        (JSC::ActiveMachineThreadsManager::contains):
     69        (JSC::ActiveMachineThreadsManager::ActiveMachineThreadsManager):
     70        (JSC::activeMachineThreadsManager):
     71        (JSC::MachineThreads::MachineThreads):
     72        (JSC::MachineThreads::~MachineThreads):
     73        (JSC::MachineThreads::removeThread):
     74        (JSC::MachineThreads::removeThreadIfFound):
     75        (JSC::MachineThreads::removeCurrentThread): Deleted.
     76        * heap/MachineStackMarker.h:
     77
    1782015-02-26  Joseph Pecoraro  <pecoraro@apple.com>
    279
  • trunk/Source/JavaScriptCore/heap/MachineStackMarker.cpp

    r180690 r180716  
    11/*
    2  *  Copyright (C) 2003, 2004, 2005, 2006, 2007, 2008, 2009 Apple Inc. All rights reserved.
     2 *  Copyright (C) 2003-2009, 2015 Apple Inc. All rights reserved.
    33 *  Copyright (C) 2007 Eric Seidel <eric@webkit.org>
    44 *  Copyright (C) 2009 Acision BV. All rights reserved.
     
    8989#endif
    9090
     91class ActiveMachineThreadsManager;
     92static ActiveMachineThreadsManager& activeMachineThreadsManager();
     93
     94class ActiveMachineThreadsManager {
     95    WTF_MAKE_NONCOPYABLE(ActiveMachineThreadsManager);
     96public:
     97
     98    class Locker {
     99    public:
     100        Locker(ActiveMachineThreadsManager& manager)
     101            : m_locker(manager.m_lock)
     102        {
     103        }
     104
     105    private:
     106        MutexLocker m_locker;
     107    };
     108
     109    void add(MachineThreads* machineThreads)
     110    {
     111        MutexLocker managerLock(m_lock);
     112        m_set.add(machineThreads);
     113    }
     114
     115    void remove(MachineThreads* machineThreads)
     116    {
     117        MutexLocker managerLock(m_lock);
     118        auto recordedMachineThreads = m_set.take(machineThreads);
     119        RELEASE_ASSERT(recordedMachineThreads = machineThreads);
     120    }
     121
     122    bool contains(MachineThreads* machineThreads)
     123    {
     124        return m_set.contains(machineThreads);
     125    }
     126
     127private:
     128    typedef HashSet<MachineThreads*> MachineThreadsSet;
     129
     130    ActiveMachineThreadsManager() { }
     131   
     132    Mutex m_lock;
     133    MachineThreadsSet m_set;
     134
     135    friend ActiveMachineThreadsManager& activeMachineThreadsManager();
     136};
     137
     138static ActiveMachineThreadsManager& activeMachineThreadsManager()
     139{
     140    static std::once_flag initializeManagerOnceFlag;
     141    static ActiveMachineThreadsManager* manager = nullptr;
     142
     143    std::call_once(initializeManagerOnceFlag, [] {
     144        manager = new ActiveMachineThreadsManager();
     145    });
     146    return *manager;
     147}
     148   
     149
    91150class MachineThreads::Thread {
    92151    WTF_MAKE_FAST_ALLOCATED;
     
    125184    UNUSED_PARAM(heap);
    126185    threadSpecificKeyCreate(&m_threadSpecific, removeThread);
     186    activeMachineThreadsManager().add(this);
    127187}
    128188
    129189MachineThreads::~MachineThreads()
    130190{
     191    activeMachineThreadsManager().remove(this);
    131192    threadSpecificKeyDelete(m_threadSpecific);
    132193
     
    181242void MachineThreads::removeThread(void* p)
    182243{
    183     static_cast<MachineThreads*>(p)->removeCurrentThread();
    184 }
    185 
    186 void MachineThreads::removeCurrentThread()
    187 {
    188     PlatformThread currentPlatformThread = getCurrentPlatformThread();
    189 
     244    auto& manager = activeMachineThreadsManager();
     245    ActiveMachineThreadsManager::Locker lock(manager);
     246    auto machineThreads = static_cast<MachineThreads*>(p);
     247    if (manager.contains(machineThreads)) {
     248        // There's a chance that the MachineThreads registry that this thread
     249        // was registered with was already destructed, and another one happened
     250        // to be instantiated at the same address. Hence, this thread may or
     251        // may not be found in this MachineThreads registry. We only need to
     252        // do a removal if this thread is found in it.
     253        machineThreads->removeThreadIfFound(getCurrentPlatformThread());
     254    }
     255}
     256
     257template<typename PlatformThread>
     258void MachineThreads::removeThreadIfFound(PlatformThread platformThread)
     259{
    190260    MutexLocker lock(m_registeredThreadsMutex);
    191     if (equalThread(currentPlatformThread, m_registeredThreads->platformThread)) {
     261    if (equalThread(platformThread, m_registeredThreads->platformThread)) {
    192262        Thread* t = m_registeredThreads;
    193263        m_registeredThreads = m_registeredThreads->next;
     
    197267        Thread* t;
    198268        for (t = m_registeredThreads->next; t; t = t->next) {
    199             if (equalThread(t->platformThread, currentPlatformThread)) {
     269            if (equalThread(t->platformThread, platformThread)) {
    200270                last->next = t->next;
    201271                break;
     
    203273            last = t;
    204274        }
    205         ASSERT(t); // If t is NULL, we never found ourselves in the list.
    206275        delete t;
    207276    }
  • trunk/Source/JavaScriptCore/heap/MachineStackMarker.h

    r180690 r180716  
    5656
    5757        static void removeThread(void*);
    58         void removeCurrentThread();
     58
     59        template<typename PlatformThread>
     60        void removeThreadIfFound(PlatformThread);
    5961
    6062        Mutex m_registeredThreadsMutex;
Note: See TracChangeset for help on using the changeset viewer.