Title: [216953] trunk/Source/WebCore
- Revision
- 216953
- Author
- mark....@apple.com
- Date
- 2017-05-16 16:06:53 -0700 (Tue, 16 May 2017)
Log Message
WorkerRunLoop::Task::performTask() needs to null check context->script() before use.
https://bugs.webkit.org/show_bug.cgi?id=172193
<rdar://problem/32225346>
Reviewed by Filip Pizlo.
According to https://build-safari.apple.com/results/Trunk%20Fuji%20GuardMalloc%20Production%20WK2%20Tests/r216929_459760e0918316187c8e52c6585a3a9ba9181204%20(12066)/results.html,
we see a crash with this crash trace:
Thread 13 Crashed:: WebCore: Worker
0 com.apple.WebCore 0x00000001099607b2 WebCore::WorkerScriptController::isTerminatingExecution() const + 18
1 com.apple.WebCore 0x000000010995ebbf WebCore::WorkerRunLoop::runCleanupTasks(WebCore::WorkerGlobalScope*) + 143
2 com.apple.WebCore 0x000000010995e80f WebCore::WorkerRunLoop::run(WebCore::WorkerGlobalScope*) + 111
3 com.apple.WebCore 0x00000001099621b6 WebCore::WorkerThread::workerThread() + 742
4 com.apple._javascript_Core 0x000000010a964b92 WTF::threadEntryPoint(void*) + 178
5 com.apple._javascript_Core 0x000000010a964a69 WTF::wtfThreadEntryPoint(void*) + 121
6 libsystem_pthread.dylib 0x00007fffbdb5caab _pthread_body + 180
7 libsystem_pthread.dylib 0x00007fffbdb5c9f7 _pthread_start + 286
8 libsystem_pthread.dylib 0x00007fffbdb5c1fd thread_start + 13
... and the crashing address is:
Exception Codes: KERN_INVALID_ADDRESS at 0x0000000000000022
0x0000000000000022 is the offset of m_scheduledTerminationMutex in the
WorkerScriptController. This means that WorkerScriptController::isTerminatingExecution()
is passed a NULL this pointer. This means that it's possible to have a race
where a WorkerRunLoop::Task gets enqueued beyond the Cleanup task that deletes the
context->script(). As a result, WorkerRunLoop::Task::performTask() (called by
runCleanupTasks()) may see a null context->script().
Hence, WorkerRunLoop::Task::performTask() should null check context->script()
before invoking the isTerminatingExecution() query on it.
No new tests because this is already covered by existing tests.
* workers/WorkerRunLoop.cpp:
(WebCore::WorkerRunLoop::Task::performTask):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (216952 => 216953)
--- trunk/Source/WebCore/ChangeLog 2017-05-16 22:49:59 UTC (rev 216952)
+++ trunk/Source/WebCore/ChangeLog 2017-05-16 23:06:53 UTC (rev 216953)
@@ -1,3 +1,44 @@
+2017-05-16 Mark Lam <mark....@apple.com>
+
+ WorkerRunLoop::Task::performTask() needs to null check context->script() before use.
+ https://bugs.webkit.org/show_bug.cgi?id=172193
+ <rdar://problem/32225346>
+
+ Reviewed by Filip Pizlo.
+
+ According to https://build-safari.apple.com/results/Trunk%20Fuji%20GuardMalloc%20Production%20WK2%20Tests/r216929_459760e0918316187c8e52c6585a3a9ba9181204%20(12066)/results.html,
+ we see a crash with this crash trace:
+
+ Thread 13 Crashed:: WebCore: Worker
+ 0 com.apple.WebCore 0x00000001099607b2 WebCore::WorkerScriptController::isTerminatingExecution() const + 18
+ 1 com.apple.WebCore 0x000000010995ebbf WebCore::WorkerRunLoop::runCleanupTasks(WebCore::WorkerGlobalScope*) + 143
+ 2 com.apple.WebCore 0x000000010995e80f WebCore::WorkerRunLoop::run(WebCore::WorkerGlobalScope*) + 111
+ 3 com.apple.WebCore 0x00000001099621b6 WebCore::WorkerThread::workerThread() + 742
+ 4 com.apple._javascript_Core 0x000000010a964b92 WTF::threadEntryPoint(void*) + 178
+ 5 com.apple._javascript_Core 0x000000010a964a69 WTF::wtfThreadEntryPoint(void*) + 121
+ 6 libsystem_pthread.dylib 0x00007fffbdb5caab _pthread_body + 180
+ 7 libsystem_pthread.dylib 0x00007fffbdb5c9f7 _pthread_start + 286
+ 8 libsystem_pthread.dylib 0x00007fffbdb5c1fd thread_start + 13
+
+ ... and the crashing address is:
+
+ Exception Codes: KERN_INVALID_ADDRESS at 0x0000000000000022
+
+ 0x0000000000000022 is the offset of m_scheduledTerminationMutex in the
+ WorkerScriptController. This means that WorkerScriptController::isTerminatingExecution()
+ is passed a NULL this pointer. This means that it's possible to have a race
+ where a WorkerRunLoop::Task gets enqueued beyond the Cleanup task that deletes the
+ context->script(). As a result, WorkerRunLoop::Task::performTask() (called by
+ runCleanupTasks()) may see a null context->script().
+
+ Hence, WorkerRunLoop::Task::performTask() should null check context->script()
+ before invoking the isTerminatingExecution() query on it.
+
+ No new tests because this is already covered by existing tests.
+
+ * workers/WorkerRunLoop.cpp:
+ (WebCore::WorkerRunLoop::Task::performTask):
+
2017-05-16 Youenn Fablet <you...@apple.com>
Modernize WebKit2 getUserMedia passing of parameters
Modified: trunk/Source/WebCore/workers/WorkerRunLoop.cpp (216952 => 216953)
--- trunk/Source/WebCore/workers/WorkerRunLoop.cpp 2017-05-16 22:49:59 UTC (rev 216952)
+++ trunk/Source/WebCore/workers/WorkerRunLoop.cpp 2017-05-16 23:06:53 UTC (rev 216953)
@@ -254,8 +254,7 @@
void WorkerRunLoop::Task::performTask(WorkerGlobalScope* context)
{
- ASSERT(context->script());
- if ((!context->isClosing() && !context->script()->isTerminatingExecution()) || m_task.isCleanupTask())
+ if ((!context->isClosing() && context->script() && !context->script()->isTerminatingExecution()) || m_task.isCleanupTask())
m_task.performTask(*context);
}
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
