Skip to content

Commit

Permalink
Cherry-pick 9ff5a32. rdar://122995875
Browse files Browse the repository at this point in the history
    Add nullptr check for ProcessLauncher client
    https://bugs.webkit.org/show_bug.cgi?id=269759
    rdar://122995875

    Reviewed by Brent Fulgham.

    This patch fixes a null pointer dereference crash that was introduced in <https://commits.webkit.org/274390@main>.
    The commit 274390@main introduced a race condition by holding a reference to the Process launcher in the completion
    handler for starting WebKit extension processes. This reference was held througout the duration of the completion
    handler. This meant that on rare occasions, the Process launcher could be deleted at the end of the completion
    handler, instead of in the AuxiliaryProcessProxy destructor, where it normally is invalidated and deleted. The
    lambda to finish the launch scheduled from the completion handler on the main thread could then end up having a
    Process launcher that was invalidated but not deallocated. When the Process launcher is invalidated, the m_client
    member is set to nullptr. This member is later dereferenced in ProcessLauncher::finishLaunchingProcess, and caused
    a null pointer crash in this case. This patch is fixing the crash by reverting the change in 274390@main that
    introduced the crash as well as adding a null pointer check for m_client, to guard against this race being
    reintroduced in the future.

    * Source/WebKit/UIProcess/Launcher/cocoa/ProcessLauncherCocoa.mm:
    (WebKit::ProcessLauncher::launchProcess):

    Canonical link: https://commits.webkit.org/275047@main

Identifier: 272448.581@safari-7618.1.15.10-branch
  • Loading branch information
pvollan authored and MyahCobbs committed Feb 21, 2024
1 parent 9ce130f commit 72398f9
Showing 1 changed file with 5 additions and 5 deletions.
10 changes: 5 additions & 5 deletions Source/WebKit/UIProcess/Launcher/cocoa/ProcessLauncherCocoa.mm
Original file line number Diff line number Diff line change
Expand Up @@ -174,9 +174,8 @@ static void launchWithExtensionKit(ProcessLauncher& processLauncher, ProcessLaun
#if USE(EXTENSIONKIT)
auto handler = [](ThreadSafeWeakPtr<ProcessLauncher> weakProcessLauncher, _SEExtensionProcess* process, ASCIILiteral name, NSError* error)
{
RefPtr launcher = weakProcessLauncher.get();
if (!launcher) {
[process invalidate];
if (!weakProcessLauncher.get()) {
process.invalidate();
return;
}
if (error) {
Expand Down Expand Up @@ -211,8 +210,9 @@ static void launchWithExtensionKit(ProcessLauncher& processLauncher, ProcessLaun

callOnMainRunLoop([weakProcessLauncher, name, process = retainPtr(process), launchGrant = WTFMove(launchGrant)] () mutable {
RefPtr launcher = weakProcessLauncher.get();
if (!launcher) {
[process invalidate];
// If m_client is null, the Process launcher has been invalidated, and we should not proceed with the launch.
if (!launcher || !launcher->m_client) {
process.invalidate();
return;
}

Expand Down

0 comments on commit 72398f9

Please sign in to comment.