Changeset 275605 in webkit

Timestamp:

Apr 7, 2021 6:13:34 AM (3 years ago)

Author:

commit-queue@webkit.org

Message:

[WPE][GTK] Null pointer dereference when child process exits immediately
​https://bugs.webkit.org/show_bug.cgi?id=224209

Patch by Michael Catanzaro <Michael Catanzaro> on 2021-04-07
Reviewed by Carlos Garcia Campos.

We discovered that when the child process exits immediately after it is spawned,
g_subprocess_get_identifier() will return nullptr. In this case, we should crash cleanly
with SIGABRT via g_error(), rather than crashing with a null pointer dereference inside
g_ascii_strtoll(). SIGABRT is much nicer than SIGSEGV and indicates that we really do want
to crash here, whereas SIGSEGV is just a bug.

Also, let's be careful with our terminology in the existing error message here. Although
GSubprocess currently always forks, I'm working to make it use posix_spawn() instead, so
let's not claim that fork() has failed when it soon might not be used at all.

• UIProcess/Launcher/glib/ProcessLauncherGLib.cpp:

(WebKit::ProcessLauncher::launchProcess):

Location:

trunk/Source/WebKit

Files:

• ChangeLog (modified) (1 diff)
• UIProcess/Launcher/glib/ProcessLauncherGLib.cpp (modified) (1 diff)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2021-04-07  Michael Catanzaro  <mcatanzaro@gnome.org>
+
+        [WPE][GTK] Null pointer dereference when child process exits immediately
+        https://bugs.webkit.org/show_bug.cgi?id=224209
+
+        Reviewed by Carlos Garcia Campos.
+
+        We discovered that when the child process exits immediately after it is spawned,
+        g_subprocess_get_identifier() will return nullptr. In this case, we should crash cleanly
+        with SIGABRT via g_error(), rather than crashing with a null pointer dereference inside
+        g_ascii_strtoll(). SIGABRT is much nicer than SIGSEGV and indicates that we really do want
+        to crash here, whereas SIGSEGV is just a bug.
+
+        Also, let's be careful with our terminology in the existing error message here. Although
+        GSubprocess currently always forks, I'm working to make it use posix_spawn() instead, so
+        let's not claim that fork() has failed when it soon might not be used at all.
+
+        * UIProcess/Launcher/glib/ProcessLauncherGLib.cpp:
+        (WebKit::ProcessLauncher::launchProcess):
+
 2021-04-07  Jer Noble  <jer.noble@apple.com>
 

trunk/Source/WebKit/UIProcess/Launcher/glib/ProcessLauncherGLib.cpp

@@ -185 +185 @@
 
     if (!process.get())
-        g_error("Unable to fork a new child process: %s", error->message);
+        g_error("Unable to spawn a new child process: %s", error->message);
 
     const char* processIdStr = g_subprocess_get_identifier(process.get());
+    if (!processIdStr)
+        g_error("Spawned process died immediately. This should not happen.");
+
     m_processIdentifier = g_ascii_strtoll(processIdStr, nullptr, 0);
     RELEASE_ASSERT(m_processIdentifier);