From ffe8884ac3a3b494786055aa51fd48e3d6edac16 Mon Sep 17 00:00:00 2001
From: Thiago Macieira <thiago.macieira@intel.com>
Date: Mon, 26 Jun 2017 16:02:05 -0700
Subject: QProcess/Unix: move strerror() up from child to parent

In theory, there's nothing wrong with having it in the child process. In
practice, we've found that strerror/malloc can hang: if an application-
wide lock was held by another thread before fork(), the child process
could wait forever for an unlocking that will not happen (no threads
running). See https://sourceware.org/bugzilla/show_bug.cgi?id=19431

As an added bonus, we now use qt_error_string(), which may produce
slightly different text from strerror.

[ChangeLog][QtCore][QProcess] Added a workaround for a rare race-
condition bug in some C libraries that caused the child process started
by QProcess to hang after trying to launch a non-existent executable or
change to a non-existent directory.

Task-number: QTBUG-61634
Change-Id: I1eba2b016de74620bfc8fffd14cbce4b9f9af69b
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@qt.io>
---
 src/corelib/io/qprocess_unix.cpp | 46 +++++++++++++++++-----------------------
 1 file changed, 19 insertions(+), 27 deletions(-)

(limited to 'src/corelib/io/qprocess_unix.cpp')

diff --git a/src/corelib/io/qprocess_unix.cpp b/src/corelib/io/qprocess_unix.cpp
index 98d196ff7b..6b7f187fee 100644
--- a/src/corelib/io/qprocess_unix.cpp
+++ b/src/corelib/io/qprocess_unix.cpp
@@ -146,10 +146,6 @@ QProcessEnvironment QProcessEnvironment::systemEnvironment()
 
 #if QT_CONFIG(process)
 
-// POSIX requires PIPE_BUF to be 512 or larger
-// so we will use 512
-static const int errorBufferMax = 512;
-
 namespace {
 struct QProcessPoller
 {
@@ -548,11 +544,18 @@ void QProcessPrivate::startProcess()
     }
 }
 
+struct ChildError
+{
+    int code;
+    char function[8];
+};
+
 void QProcessPrivate::execChild(const char *workingDir, char **path, char **argv, char **envp)
 {
     ::signal(SIGPIPE, SIG_DFL);         // reset the signal that we ignored
 
     Q_Q(QProcess);
+    ChildError error = { 0, {} };       // force zeroing of function[8]
 
     // copy the stdin socket if asked to (without closing on exec)
     if (inputChannelMode != QProcess::ForwardedInputChannel)
@@ -575,9 +578,9 @@ void QProcessPrivate::execChild(const char *workingDir, char **path, char **argv
     qt_safe_close(childStartedPipe[0]);
 
     // enter the working directory
-    const char *callthatfailed = "chdir: ";
     if (workingDir && QT_CHDIR(workingDir) == -1) {
         // failed, stop the process
+        strcpy(error.function, "chdir");
         goto report_errno;
     }
 
@@ -587,7 +590,7 @@ void QProcessPrivate::execChild(const char *workingDir, char **path, char **argv
     // execute the process
     if (!envp) {
         qt_safe_execvp(argv[0], argv);
-        callthatfailed = "execvp: ";
+        strcpy(error.function, "execvp");
     } else {
         if (path) {
             char **arg = path;
@@ -605,33 +608,22 @@ void QProcessPrivate::execChild(const char *workingDir, char **path, char **argv
 #endif
             qt_safe_execve(argv[0], argv, envp);
         }
-        callthatfailed = "execve: ";
+        strcpy(error.function, "execve");
     }
 
     // notify failure
-    // we're running in the child process, so we don't need to be thread-safe;
-    // we can use strerror
+    // don't use strerror or any other routines that may allocate memory, since
+    // some buggy libc versions can deadlock on locked mutexes.
 report_errno:
-    const char *msg = strerror(errno);
-#if defined (QPROCESS_DEBUG)
-    fprintf(stderr, "QProcessPrivate::execChild() failed (%s), notifying parent process\n", msg);
-#endif
-    qt_safe_write(childStartedPipe[1], callthatfailed, strlen(callthatfailed));
-    qt_safe_write(childStartedPipe[1], msg, strlen(msg));
-    qt_safe_close(childStartedPipe[1]);
+    error.code = errno;
+    qt_safe_write(childStartedPipe[1], &error, sizeof(error));
     childStartedPipe[1] = -1;
 }
 
 bool QProcessPrivate::processStarted(QString *errorMessage)
 {
-    char buf[errorBufferMax];
-    int i = 0;
-    int ret;
-    do {
-        ret = qt_safe_read(childStartedPipe[0], buf + i, sizeof buf - i);
-        if (ret > 0)
-            i += ret;
-    } while (ret > 0 && i < int(sizeof buf));
+    ChildError buf;
+    int ret = qt_safe_read(childStartedPipe[0], &buf, sizeof(buf));
 
     if (startupSocketNotifier) {
         startupSocketNotifier->setEnabled(false);
@@ -646,10 +638,10 @@ bool QProcessPrivate::processStarted(QString *errorMessage)
 #endif
 
     // did we read an error message?
-    if ((i > 0) && errorMessage)
-        *errorMessage = QString::fromLocal8Bit(buf, i);
+    if (ret > 0 && errorMessage)
+        *errorMessage = QLatin1String(buf.function) + QLatin1String(": ") + qt_error_string(buf.code);
 
-    return i <= 0;
+    return ret <= 0;
 }
 
 qint64 QProcessPrivate::bytesAvailableInChannel(const Channel *channel) const
-- 
cgit v1.2.3