[WK2][UNIX] Crash in WebKit::PluginProcessProxy::scanPlugin()
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 7 Nov 2012 14:55:47 +0000 (14:55 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 7 Nov 2012 14:55:47 +0000 (14:55 +0000)
https://bugs.webkit.org/show_bug.cgi?id=101446

Patch by Christophe Dumez <christophe.dumez@intel.com> on 2012-11-07
Reviewed by Kenneth Rohde Christiansen.

Make sure that the disposition of the SIGCHLD signal is reset to the default
before calling g_spawn_sync(). If the disposition is set to SIG_IGN, then
g_spawn_sync() will not be able to return the exit status of the child
process, our exit failure check will be useless and the following warning
will be printed:

GLib-WARNING **: In call to g_spawn_sync(), exit status of a child process
was requested but SIGCHLD action was set to SIG_IGN and ECHILD was received
by waitpid(), so exit status can't be returned. This is a bug in the
program calling g_spawn_sync(); either don't request the exit status, or
don't set the SIGCHLD action.

This patch also adds a NULL-check for stdOut to avoid crashing in such
case and makes use of String::split() to parse stdOut instead of doing it
manually.

* UIProcess/Plugins/unix/PluginProcessProxyUnix.cpp:
(WebKit::PluginProcessProxy::scanPlugin):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@133755 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebKit2/ChangeLog
Source/WebKit2/UIProcess/Plugins/unix/PluginProcessProxyUnix.cpp

index 82df890..e013c4f 100644 (file)
@@ -1,3 +1,29 @@
+2012-11-07  Christophe Dumez  <christophe.dumez@intel.com>
+
+        [WK2][UNIX] Crash in WebKit::PluginProcessProxy::scanPlugin()
+        https://bugs.webkit.org/show_bug.cgi?id=101446
+
+        Reviewed by Kenneth Rohde Christiansen.
+
+        Make sure that the disposition of the SIGCHLD signal is reset to the default
+        before calling g_spawn_sync(). If the disposition is set to SIG_IGN, then
+        g_spawn_sync() will not be able to return the exit status of the child
+        process, our exit failure check will be useless and the following warning
+        will be printed:
+
+        GLib-WARNING **: In call to g_spawn_sync(), exit status of a child process
+        was requested but SIGCHLD action was set to SIG_IGN and ECHILD was received
+        by waitpid(), so exit status can't be returned. This is a bug in the
+        program calling g_spawn_sync(); either don't request the exit status, or
+        don't set the SIGCHLD action.
+
+        This patch also adds a NULL-check for stdOut to avoid crashing in such
+        case and makes use of String::split() to parse stdOut instead of doing it
+        manually.
+
+        * UIProcess/Plugins/unix/PluginProcessProxyUnix.cpp:
+        (WebKit::PluginProcessProxy::scanPlugin):
+
 2012-11-07  Shinya Kawanaka  <shinyak@chromium.org>
 
         [Shadow] Use setPseudo() instead of setShadowPseudoId().
index 189e213..d24c391 100644 (file)
@@ -72,28 +72,34 @@ bool PluginProcessProxy::scanPlugin(const String& pluginPath, RawPluginMetaData&
     int status;
     char* stdOut = 0;
 
+    // If the disposition of SIGCLD signal is set to SIG_IGN (default)
+    // then the signal will be ignored and g_spawn_sync() will not be
+    // able to return the status.
+    // As a consequence, we make sure that the disposition is set to
+    // SIG_DFL before calling g_spawn_sync().
+    struct sigaction action;
+    sigaction(SIGCLD, 0, &action);
+    if (action.sa_handler == SIG_IGN) {
+        action.sa_handler = SIG_DFL;
+        sigaction(SIGCLD, &action, 0);
+    }
+
     if (!g_spawn_sync(0, argv, 0, G_SPAWN_STDERR_TO_DEV_NULL, 0, 0, &stdOut, 0, &status, 0))
         return false;
 
-    if (!WIFEXITED(status) || WEXITSTATUS(status) != EXIT_SUCCESS) {
+    if (!WIFEXITED(status) || WEXITSTATUS(status) != EXIT_SUCCESS || !stdOut) {
         free(stdOut);
         return false;
     }
 
-    const unsigned kNumLinesExpected = 3;
-    String lines[kNumLinesExpected];
-    unsigned lineIndex = 0;
-
-    const UChar* current = reinterpret_cast<const UChar*>(stdOut);
+    String stdOutString(reinterpret_cast<const UChar*>(stdOut));
+    free(stdOut);
 
-    while (lineIndex < kNumLinesExpected) {
-        const UChar* start = current;
-        while (*current++ != UChar('\n')) { }
-        lines[lineIndex++] = String(start, current - start - 1);
-    }
+    Vector<String> lines;
+    stdOutString.split(UChar('\n'), lines);
 
-    if (stdOut)
-        free(stdOut);
+    if (lines.size() < 3)
+        return false;
 
     result.name.swap(lines[0]);
     result.description.swap(lines[1]);