QProcess/Unix: protect against stack unwinding in the child process stub

There are two types of stack unwinding that can happen on Unix systems:
C++ exceptions and PThread cancellations (on some systems, like Linux,
PThread cancellations can be caught in catch(...) statements). We call a
variety of PThread cancellation functions from inside the child stub,
like close(). To avoid problems, we disable PThread cancellations
completely before fork() or vfork().

The C++ exception case is simpler, because we can be sure of catching
them with the catch (...) statement and simply transform them into an
error message. This is also testable, which the PThread cancellation
isn't.

The error message isn't ideal because we're string-frozen. I'll improve
it for 6.6.

Pick-to: 6.5
Change-Id: Icfe44ecf285a480fafe4fffd174d97a475c93ff1
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>

3 files changed, 87 insertions, 4 deletions

diff --git a/src/corelib/io/qprocess_p.h b/src/corelib/io/qprocess_p.h
index c0273aeeae..dacf41b77a 100644
--- a/src/corelib/io/qprocess_p.h
+++ b/src/corelib/io/qprocess_p.h
@@ -303,7 +303,7 @@ public:
     void startProcess();
 #if defined(Q_OS_UNIX)
     void commitChannels() const;
-    void execChild(int workingDirectory, char **argv, char **envp) const;
+    void execChild(int workingDirectory, char **argv, char **envp) const noexcept;
 #endif
     bool processStarted(QString *errorMessage = nullptr);
     void processFinished();
diff --git a/src/corelib/io/qprocess_unix.cpp b/src/corelib/io/qprocess_unix.cpp
index 5c72a39911..d16fa7498e 100644
--- a/src/corelib/io/qprocess_unix.cpp
+++ b/src/corelib/io/qprocess_unix.cpp
@@ -47,8 +47,40 @@
 
 QT_BEGIN_NAMESPACE
 
+// we need an errno number to use to indicate the child process modifier threw,
+// something the regular operations shouldn't set.
+static constexpr int FakeErrnoForThrow =
+#ifdef ECANCELED
+        ECANCELED
+#else
+        ESHUTDOWN
+#endif
+        ;
+
 using namespace Qt::StringLiterals;
 
+namespace {
+struct PThreadCancelGuard
+{
+#if defined(PTHREAD_CANCEL_DISABLE)
+    int oldstate;
+    PThreadCancelGuard() noexcept(false)
+    {
+        pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &oldstate);
+    }
+    ~PThreadCancelGuard() noexcept(false)
+    {
+        reenable();
+    }
+    void reenable() noexcept(false)
+    {
+        // this doesn't touch errno
+        pthread_setcancelstate(oldstate, nullptr);
+    }
+#endif
+};
+}
+
 #if !defined(Q_OS_DARWIN)
 
 QT_BEGIN_INCLUDE_NAMESPACE
@@ -472,6 +504,11 @@ void QProcessPrivate::startProcess()
     const CharPointerList argv(resolveExecutable(program), arguments);
     const CharPointerList envp(environment.d.constData());
 
+    // Disable PThread cancellation from this point on: we mustn't have it
+    // enabled when the child starts running nor while our state could get
+    // corrupted if we abruptly exited this function.
+    PThreadCancelGuard cancelGuard;
+
     // Start the child.
     auto execChild1 = [this, workingDirFd, &argv, &envp]() {
         execChild(workingDirFd, argv.pointers.get(), envp.pointers.get());
@@ -539,12 +576,24 @@ void QProcessPrivate::startProcess()
         ::fcntl(stderrChannel.pipe[0], F_SETFL, ::fcntl(stderrChannel.pipe[0], F_GETFL) | O_NONBLOCK);
 }
 
+static bool callChildProcessModifier(const QProcessPrivate::UnixExtras *unixExtras) noexcept
+{
+    QT_TRY {
+        if (unixExtras->childProcessModifier)
+            unixExtras->childProcessModifier();
+    } QT_CATCH (...) {
+        errno = FakeErrnoForThrow;
+        return false;
+    }
+    return true;
+}
+
 // IMPORTANT:
 //
 // This function is called in a vfork() context on some OSes (notably, Linux
 // with forkfd), so it MUST NOT modify any non-local variable because it's
 // still sharing memory with the parent process.
-void QProcessPrivate::execChild(int workingDir, char **argv, char **envp) const
+void QProcessPrivate::execChild(int workingDir, char **argv, char **envp) const noexcept
 {
     ::signal(SIGPIPE, SIG_DFL);         // reset the signal that we ignored
 
@@ -564,8 +613,10 @@ void QProcessPrivate::execChild(int workingDir, char **argv, char **envp) const
     }
 
     if (unixExtras) {
-        if (unixExtras->childProcessModifier)
-            unixExtras->childProcessModifier();
+        if (!callChildProcessModifier(unixExtras.get())) {
+            std::strcpy(error.function, "throw");
+            goto report_errno;
+        }
     }
 
     // execute the process
@@ -965,6 +1016,9 @@ bool QProcessPrivate::startDetached(qint64 *pid)
     const CharPointerList argv(resolveExecutable(program), arguments);
     const CharPointerList envp(environment.d.constData());
 
+    // see startProcess() for more information
+    PThreadCancelGuard cancelGuard;
+
     pid_t childPid = fork();
     if (childPid == 0) {
         ::signal(SIGPIPE, SIG_DFL);     // reset the signal that we ignored
diff --git a/tests/auto/corelib/io/qprocess/tst_qprocess.cpp b/tests/auto/corelib/io/qprocess/tst_qprocess.cpp
index 4d54e8db27..d10c9ea566 100644
--- a/tests/auto/corelib/io/qprocess/tst_qprocess.cpp
+++ b/tests/auto/corelib/io/qprocess/tst_qprocess.cpp
@@ -112,6 +112,7 @@ private slots:
 #endif // Q_OS_WIN
 #if defined(Q_OS_UNIX)
     void setChildProcessModifier();
+    void throwInChildProcessModifier();
 #endif
     void exitCodeTest();
     void systemEnvironment();
@@ -1469,6 +1470,34 @@ void tst_QProcess::setChildProcessModifier()
     QCOMPARE(process.exitStatus(), QProcess::NormalExit);
     QCOMPARE(process.exitCode(), 0);
 }
+
+void tst_QProcess::throwInChildProcessModifier()
+{
+#ifndef __cpp_exceptions
+    Q_SKIP("Exceptions disabled.");
+#else
+    QProcess process;
+    process.setChildProcessModifier([]() {
+        throw 42;
+    });
+    process.setProgram("testProcessNormal/testProcessNormal");
+
+    process.start();
+    QVERIFY(!process.waitForStarted(5000));
+    QCOMPARE(process.state(), QProcess::NotRunning);
+    QCOMPARE(process.error(), QProcess::FailedToStart);
+    QVERIFY2(process.errorString().contains("throw"),
+             qPrintable(process.errorString()));
+
+    // try again, to ensure QProcess internal state wasn't corrupted
+    process.start();
+    QVERIFY(!process.waitForStarted(5000));
+    QCOMPARE(process.state(), QProcess::NotRunning);
+    QCOMPARE(process.error(), QProcess::FailedToStart);
+    QVERIFY2(process.errorString().contains("throw"),
+             qPrintable(process.errorString()));
+#endif
+}
 #endif
 
 void tst_QProcess::exitCodeTest()