Skip to content
Permalink
Browse files
RemoteAudioSourceProviderManager is accessed in non-thread-safe manner
https://bugs.webkit.org/show_bug.cgi?id=221894

Patch by Kimmo Kinnunen <kkinnunen@apple.com> on 2021-02-16
Reviewed by Chris Dumez.

Remove call to Connection::removeWorkQueueMessageReceiver() from the destructor.
The message receive destination is still active during destructor,
so Connection might dispatch new message tasks during that time.
This causes ref of the instance already being destroyed, which
then causes the tasks to use deleted instance and then later
deleting it again.

Other subclasses call add...() correctly.
Other subclasses never call remove..(). Assert that the instances are
never destroyed, as to never leave a dangling reference in the connection.

* UIProcess/mac/SecItemShimProxy.cpp:
(WebKit::SecItemShimProxy::~SecItemShimProxy):
* UIProcess/mac/SecItemShimProxy.h:
* WebProcess/GPU/GPUProcessConnection.cpp:
(WebKit::GPUProcessConnection::~GPUProcessConnection):
* WebProcess/GPU/media/RemoteAudioSourceProviderManager.cpp:
(WebKit::RemoteAudioSourceProviderManager::~RemoteAudioSourceProviderManager):
(WebKit::RemoteAudioSourceProviderManager::stopListeningForIPC):
* WebProcess/GPU/media/RemoteAudioSourceProviderManager.h:
* WebProcess/Inspector/WebInspectorInterruptDispatcher.cpp:
(WebKit::WebInspectorInterruptDispatcher::~WebInspectorInterruptDispatcher):
* WebProcess/Plugins/PluginProcessConnectionManager.cpp:
(WebKit::PluginProcessConnectionManager::~PluginProcessConnectionManager):
* WebProcess/WebPage/EventDispatcher.cpp:
(WebKit::EventDispatcher::~EventDispatcher):
* WebProcess/WebPage/ViewUpdateDispatcher.cpp:
(WebKit::ViewUpdateDispatcher::~ViewUpdateDispatcher):

Canonical link: https://commits.webkit.org/234148@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@272915 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
kkinnunen-apple authored and webkit-commit-queue committed Feb 16, 2021
1 parent df0e19d commit 936a85d329cda4e3f3376d4f2859f7697380f658
Showing 10 changed files with 56 additions and 0 deletions.
@@ -1,3 +1,39 @@
2021-02-16 Kimmo Kinnunen <kkinnunen@apple.com>

RemoteAudioSourceProviderManager is accessed in non-thread-safe manner
https://bugs.webkit.org/show_bug.cgi?id=221894

Reviewed by Chris Dumez.

Remove call to Connection::removeWorkQueueMessageReceiver() from the destructor.
The message receive destination is still active during destructor,
so Connection might dispatch new message tasks during that time.
This causes ref of the instance already being destroyed, which
then causes the tasks to use deleted instance and then later
deleting it again.

Other subclasses call add...() correctly.
Other subclasses never call remove..(). Assert that the instances are
never destroyed, as to never leave a dangling reference in the connection.

* UIProcess/mac/SecItemShimProxy.cpp:
(WebKit::SecItemShimProxy::~SecItemShimProxy):
* UIProcess/mac/SecItemShimProxy.h:
* WebProcess/GPU/GPUProcessConnection.cpp:
(WebKit::GPUProcessConnection::~GPUProcessConnection):
* WebProcess/GPU/media/RemoteAudioSourceProviderManager.cpp:
(WebKit::RemoteAudioSourceProviderManager::~RemoteAudioSourceProviderManager):
(WebKit::RemoteAudioSourceProviderManager::stopListeningForIPC):
* WebProcess/GPU/media/RemoteAudioSourceProviderManager.h:
* WebProcess/Inspector/WebInspectorInterruptDispatcher.cpp:
(WebKit::WebInspectorInterruptDispatcher::~WebInspectorInterruptDispatcher):
* WebProcess/Plugins/PluginProcessConnectionManager.cpp:
(WebKit::PluginProcessConnectionManager::~PluginProcessConnectionManager):
* WebProcess/WebPage/EventDispatcher.cpp:
(WebKit::EventDispatcher::~EventDispatcher):
* WebProcess/WebPage/ViewUpdateDispatcher.cpp:
(WebKit::ViewUpdateDispatcher::~ViewUpdateDispatcher):

2021-02-16 Youenn Fablet <youenn@apple.com>

Set a default path for device ID hash salts
@@ -51,6 +51,11 @@ SecItemShimProxy::SecItemShimProxy()
{
}

SecItemShimProxy::~SecItemShimProxy()
{
ASSERT_NOT_REACHED();
}

void SecItemShimProxy::initializeConnection(IPC::Connection& connection)
{
connection.addWorkQueueMessageReceiver(Messages::SecItemShimProxy::messageReceiverName(), m_queue.get(), this);
@@ -43,6 +43,7 @@ WTF_MAKE_NONCOPYABLE(SecItemShimProxy);

private:
SecItemShimProxy();
~SecItemShimProxy();

// IPC::Connection::WorkQueueMessageReceiver
void didReceiveMessage(IPC::Connection&, IPC::Decoder&) override;
@@ -88,6 +88,10 @@ GPUProcessConnection::GPUProcessConnection(IPC::Connection::Identifier connectio
GPUProcessConnection::~GPUProcessConnection()
{
m_connection->invalidate();
#if PLATFORM(COCOA) && ENABLE(WEB_AUDIO)
if (m_audioSourceProviderManager)
m_audioSourceProviderManager->stopListeningForIPC();
#endif
}

void GPUProcessConnection::didClose(IPC::Connection&)
@@ -44,6 +44,11 @@ RemoteAudioSourceProviderManager::RemoteAudioSourceProviderManager()
}

RemoteAudioSourceProviderManager::~RemoteAudioSourceProviderManager()
{
ASSERT(!m_connection);
}

void RemoteAudioSourceProviderManager::stopListeningForIPC()
{
setConnection(nullptr);
}
@@ -43,6 +43,7 @@ class RemoteAudioSourceProviderManager : public IPC::Connection::WorkQueueMessag
public:
static Ref<RemoteAudioSourceProviderManager> create() { return adoptRef(*new RemoteAudioSourceProviderManager()); }
~RemoteAudioSourceProviderManager();
void stopListeningForIPC();

void addProvider(Ref<RemoteAudioSourceProvider>&&);
void removeProvider(WebCore::MediaPlayerIdentifier);
@@ -45,6 +45,7 @@ WebInspectorInterruptDispatcher::WebInspectorInterruptDispatcher()

WebInspectorInterruptDispatcher::~WebInspectorInterruptDispatcher()
{
ASSERT_NOT_REACHED();
}

void WebInspectorInterruptDispatcher::initializeConnection(IPC::Connection* connection)
@@ -54,6 +54,7 @@ PluginProcessConnectionManager::PluginProcessConnectionManager()

PluginProcessConnectionManager::~PluginProcessConnectionManager()
{
ASSERT_NOT_REACHED();
}

void PluginProcessConnectionManager::initializeConnection(IPC::Connection* connection)
@@ -66,6 +66,7 @@ EventDispatcher::EventDispatcher()

EventDispatcher::~EventDispatcher()
{
ASSERT_NOT_REACHED();
}

#if ENABLE(SCROLLING_THREAD)
@@ -48,6 +48,7 @@ ViewUpdateDispatcher::ViewUpdateDispatcher()

ViewUpdateDispatcher::~ViewUpdateDispatcher()
{
ASSERT_NOT_REACHED();
}

void ViewUpdateDispatcher::initializeConnection(IPC::Connection* connection)

0 comments on commit 936a85d

Please sign in to comment.