Skip to content

Commit 0fdaf95

Browse files
brentfulghambrentfulgham
committed
Protect against invalid mach ports returned by mach_port_request_notification
https://bugs.webkit.org/show_bug.cgi?id=184106 <rdar://problem/37865316> Reviewed by Chris Dumez. Source/WebKit: * Platform/IPC/Connection.h: (IPC::Connection::Identifier::Identifier): Use default initializer syntax. * Platform/IPC/mac/ConnectionMac.mm: (IPC::Connection::open): Drive-by-fix: Include formatted mach error message in logging. (IPC::Connection::receiveSourceEventHandler): Check return value from 'mach_port_request_notification' and clean up if it experienced an error. * UIProcess/Launcher/mac/ProcessLauncherMac.mm: (WebKit::ProcessLauncher::launchProcess): Ditto. Source/WebKitLegacy/mac: * Plugins/Hosted/NetscapePluginHostProxy.mm: (WebKit::NetscapePluginHostProxy::NetscapePluginHostProxy): Check return value from 'mach_port_request_notification' and clean up if it experienced an error. (WebKit::NetscapePluginHostProxy::processRequests): Drive-by-fix: Include formatted mach error message in logging. Canonical link: https://commits.webkit.org/199676@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@230045 268f45cc-cd09-0410-ab3c-d52691b4dbfc
1 parent 6f49e34 commit 0fdaf95

6 files changed

Lines changed: 60 additions & 14 deletions

File tree

‎Source/WebKit/ChangeLog‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,20 @@
1+
2018-03-28 Brent Fulgham <bfulgham@apple.com>
2+
3+
Protect against invalid mach ports returned by mach_port_request_notification
4+
https://bugs.webkit.org/show_bug.cgi?id=184106
5+
<rdar://problem/37865316>
6+
7+
Reviewed by Chris Dumez.
8+
9+
* Platform/IPC/Connection.h:
10+
(IPC::Connection::Identifier::Identifier): Use default initializer syntax.
11+
* Platform/IPC/mac/ConnectionMac.mm:
12+
(IPC::Connection::open): Drive-by-fix: Include formatted mach error message in logging.
13+
(IPC::Connection::receiveSourceEventHandler): Check return value from 'mach_port_request_notification'
14+
and clean up if it experienced an error.
15+
* UIProcess/Launcher/mac/ProcessLauncherMac.mm:
16+
(WebKit::ProcessLauncher::launchProcess): Ditto.
17+
118
2018-03-28 Dean Jackson <dino@apple.com>
219

320
WKWebViewContentProvider shouldn't be a UIScrollViewDelegate

‎Source/WebKit/Platform/IPC/Connection.h‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (C) 2010-2016 Apple Inc. All rights reserved.
2+
* Copyright (C) 2010-2018 Apple Inc. All rights reserved.
33
* Copyright (C) 2010 Nokia Corporation and/or its subsidiary(-ies)
44
* Portions Copyright (c) 2010 Motorola Mobility, Inc. All rights reserved.
55
* Copyright (C) 2017 Sony Interactive Entertainment Inc.
@@ -116,7 +116,6 @@ class Connection : public ThreadSafeRefCounted<Connection> {
116116
#elif OS(DARWIN)
117117
struct Identifier {
118118
Identifier()
119-
: port(MACH_PORT_NULL)
120119
{
121120
}
122121

@@ -131,7 +130,7 @@ class Connection : public ThreadSafeRefCounted<Connection> {
131130
{
132131
}
133132

134-
mach_port_t port;
133+
mach_port_t port { MACH_PORT_NULL };
135134
OSObjectPtr<xpc_connection_t> xpcConnection;
136135
};
137136
static bool identifierIsNull(Identifier identifier) { return identifier.port == MACH_PORT_NULL; }

‎Source/WebKit/Platform/IPC/mac/ConnectionMac.mm‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,7 @@ void watchdogTimerFired()
186186

187187
auto kr = mach_port_allocate(mach_task_self(), MACH_PORT_RIGHT_RECEIVE, &m_receivePort);
188188
if (kr != KERN_SUCCESS) {
189-
LOG_ERROR("Could not allocate mach port, error %x", kr);
189+
LOG_ERROR("Could not allocate mach port, error %x: %s", kr, mach_error_string(kr));
190190
CRASH();
191191
}
192192
#if !PLATFORM(WATCHOS)
@@ -533,7 +533,13 @@ void watchdogTimerFired()
533533

534534
if (m_sendPort) {
535535
mach_port_t previousNotificationPort = MACH_PORT_NULL;
536-
mach_port_request_notification(mach_task_self(), m_receivePort, MACH_NOTIFY_NO_SENDERS, 0, MACH_PORT_NULL, MACH_MSG_TYPE_MOVE_SEND_ONCE, &previousNotificationPort);
536+
auto kr = mach_port_request_notification(mach_task_self(), m_receivePort, MACH_NOTIFY_NO_SENDERS, 0, MACH_PORT_NULL, MACH_MSG_TYPE_MOVE_SEND_ONCE, &previousNotificationPort);
537+
ASSERT(kr == KERN_SUCCESS);
538+
if (kr != KERN_SUCCESS) {
539+
// If mach_port_request_notification fails, 'previousNotificationPort' will be uninitialized.
540+
LOG_ERROR("mach_port_request_notification failed: (%x) %s", kr, mach_error_string(kr));
541+
previousNotificationPort = MACH_PORT_NULL;
542+
}
537543

538544
if (previousNotificationPort != MACH_PORT_NULL)
539545
mach_port_deallocate(mach_task_self(), previousNotificationPort);

‎Source/WebKit/UIProcess/Launcher/mac/ProcessLauncherMac.mm‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -153,16 +153,21 @@ static bool shouldLeakBoost(const ProcessLauncher::LaunchOptions& launchOptions)
153153
mach_port_t listeningPort = MACH_PORT_NULL;
154154
auto kr = mach_port_allocate(mach_task_self(), MACH_PORT_RIGHT_RECEIVE, &listeningPort);
155155
if (kr != KERN_SUCCESS) {
156-
LOG_ERROR("Could not allocate mach port, error %x", kr);
156+
LOG_ERROR("Could not allocate mach port, error %x: %s", kr, mach_error_string(kr));
157157
CRASH();
158158
}
159159

160160
// Insert a send right so we can send to it.
161161
mach_port_insert_right(mach_task_self(), listeningPort, listeningPort, MACH_MSG_TYPE_MAKE_SEND);
162162

163-
mach_port_t previousNotificationPort;
164-
mach_port_request_notification(mach_task_self(), listeningPort, MACH_NOTIFY_NO_SENDERS, 0, listeningPort, MACH_MSG_TYPE_MAKE_SEND_ONCE, &previousNotificationPort);
163+
mach_port_t previousNotificationPort = MACH_PORT_NULL;
164+
auto mc = mach_port_request_notification(mach_task_self(), listeningPort, MACH_NOTIFY_NO_SENDERS, 0, listeningPort, MACH_MSG_TYPE_MAKE_SEND_ONCE, &previousNotificationPort);
165165
ASSERT(!previousNotificationPort);
166+
ASSERT(mc == KERN_SUCCESS);
167+
if (mc != KERN_SUCCESS) {
168+
// If mach_port_request_notification fails, 'previousNotificationPort' will be uninitialized.
169+
LOG_ERROR("mach_port_request_notification failed: (%x) %s", mc, mach_error_string(mc));
170+
}
166171

167172
String clientIdentifier;
168173
#if PLATFORM(MAC)

‎Source/WebKitLegacy/mac/ChangeLog‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,16 @@
1+
2018-03-28 Brent Fulgham <bfulgham@apple.com>
2+
3+
Protect against invalid mach ports returned by mach_port_request_notification
4+
https://bugs.webkit.org/show_bug.cgi?id=184106
5+
<rdar://problem/37865316>
6+
7+
Reviewed by Chris Dumez.
8+
9+
* Plugins/Hosted/NetscapePluginHostProxy.mm:
10+
(WebKit::NetscapePluginHostProxy::NetscapePluginHostProxy): Check return value from 'mach_port_request_notification'
11+
and clean up if it experienced an error.
12+
(WebKit::NetscapePluginHostProxy::processRequests): Drive-by-fix: Include formatted mach error message in logging.
13+
114
2018-03-28 Brent Fulgham <bfulgham@apple.com>
215

316
Avoid uninitialized mach ports

‎Source/WebKitLegacy/mac/Plugins/Hosted/NetscapePluginHostProxy.mm‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -111,10 +111,16 @@ - (BOOL)_wantsUserAttention
111111
m_deadNameNotificationPort = adoptCF(CFMachPortCreate(0, deadNameNotificationCallback, &context, 0));
112112

113113
mach_port_t previous = MACH_PORT_NULL;
114-
mach_port_request_notification(mach_task_self(), pluginHostPort, MACH_NOTIFY_DEAD_NAME, 0,
115-
CFMachPortGetPort(m_deadNameNotificationPort.get()), MACH_MSG_TYPE_MAKE_SEND_ONCE, &previous);
114+
auto kr = mach_port_request_notification(mach_task_self(), pluginHostPort, MACH_NOTIFY_DEAD_NAME, 0,
115+
CFMachPortGetPort(m_deadNameNotificationPort.get()), MACH_MSG_TYPE_MAKE_SEND_ONCE, &previous);
116116
ASSERT(previous == MACH_PORT_NULL);
117-
117+
ASSERT(kr == KERN_SUCCESS);
118+
if (kr != KERN_SUCCESS) {
119+
// If mach_port_request_notification fails, 'previous' will be uninitialized.
120+
LOG_ERROR("mach_port_request_notification failed: (%x) %s", kr, mach_error_string(kr));
121+
previous = MACH_PORT_NULL;
122+
}
123+
118124
RetainPtr<CFRunLoopSourceRef> deathPortSource = adoptCF(CFMachPortCreateRunLoopSource(0, m_deadNameNotificationPort.get(), 0));
119125

120126
CFRunLoopAddSource(CFRunLoopGetCurrent(), deathPortSource.get(), kCFRunLoopDefaultMode);
@@ -284,7 +290,7 @@ - (BOOL)_wantsUserAttention
284290
if (!m_portSet) {
285291
auto kr = mach_port_allocate(mach_task_self(), MACH_PORT_RIGHT_PORT_SET, &m_portSet);
286292
if (kr != KERN_SUCCESS) {
287-
LOG_ERROR("Could not allocate mach port, error %x", kr);
293+
LOG_ERROR("Could not allocate mach port, error %x: %s", kr, mach_error_string(kr));
288294
CRASH();
289295
}
290296
mach_port_insert_member(mach_task_self(), m_clientPort, m_portSet);
@@ -298,7 +304,7 @@ - (BOOL)_wantsUserAttention
298304
kern_return_t kr = mach_msg(msg, MACH_RCV_MSG, 0, sizeof(buffer), m_portSet, 0, MACH_PORT_NULL);
299305

300306
if (kr != KERN_SUCCESS) {
301-
LOG_ERROR("Could not receive mach message, error %x", kr);
307+
LOG_ERROR("Could not receive mach message, error %x: %s", kr, mach_error_string(kr));
302308
s_processingRequests--;
303309
return false;
304310
}
@@ -311,7 +317,7 @@ - (BOOL)_wantsUserAttention
311317
kr = mach_msg(replyHeader, MACH_SEND_MSG, replyHeader->msgh_size, 0, MACH_PORT_NULL, 0, MACH_PORT_NULL);
312318

313319
if (kr != KERN_SUCCESS) {
314-
LOG_ERROR("Could not send mach message, error %x", kr);
320+
LOG_ERROR("Could not send mach message, error %x: %s", kr, mach_error_string(kr));
315321
s_processingRequests--;
316322
return false;
317323
}

0 commit comments

Comments
 (0)