From 6763b9f7ec2a7f8dd0bfc011ccd102316bb95e7a Mon Sep 17 00:00:00 2001
From: Thiago Macieira <thiago.macieira@intel.com>
Date: Wed, 28 Aug 2024 11:01:32 -0700
Subject: [PATCH] QProcess/Unix: clean up QChildProcess before calling
 setErrorAndEmit()

QChildProcess' destructor may unwind the stack, due to the call to
pthread_setcancelstate(), which is why it's marked noexcept(false)
explicitly. To avoid a situation where it causes a stack unwinding while
a stack unwinding is already in progress, this commit makes QProcess
re-enable the thread cancellation state (which immediately processes a
pending cancellation) before calling anything else that may allocate
memory (thus throw). It also guards against calling
pthread_setcancelstate() twice.

The successful execution paths remain under disabled cancellation so C
library calls like close(), fcntl(), read(), and waitpid() will not
cause premature unwinding.

Pick-to: 6.8
Change-Id: I009cf64012e167321aa6fffd609c603cf18b50e4
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
---
 src/corelib/io/qprocess_unix.cpp | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/src/corelib/io/qprocess_unix.cpp b/src/corelib/io/qprocess_unix.cpp
index e304be3f14d..d9b672df11d 100644
--- a/src/corelib/io/qprocess_unix.cpp
+++ b/src/corelib/io/qprocess_unix.cpp
@@ -284,9 +284,15 @@ struct QChildProcess
         disableThreadCancellations();
     }
     ~QChildProcess() noexcept(false)
+    {
+        cleanup();
+    }
+
+    void cleanup() noexcept(false)
     {
         if (workingDirectory >= 0)
             close(workingDirectory);
+        workingDirectory = -1;
 
         restoreSignalMask();
         restoreThreadCancellations();
@@ -306,6 +312,7 @@ struct QChildProcess
 
     void restoreSignalMask() const noexcept
     {
+        // this function may be called more than once
         if (isUsingVfork)
             pthread_sigmask(SIG_SETMASK, &oldsigset, nullptr);
     }
@@ -348,9 +355,11 @@ private:
     }
     void restoreThreadCancellations() noexcept(false)
     {
-        if (oldstate != PTHREAD_CANCEL_DISABLE) {
+        // ensure we don't call pthread_setcancelstate() again
+        int oldoldstate = std::exchange(oldstate, PTHREAD_CANCEL_DISABLE);
+        if (oldoldstate != PTHREAD_CANCEL_DISABLE) {
             // this doesn't touch errno
-            pthread_setcancelstate(oldstate, nullptr);
+            pthread_setcancelstate(oldoldstate, nullptr);
         }
     }
 #else
@@ -727,6 +736,7 @@ void QProcessPrivate::startProcess()
 
     if (forkfd == -1) {
         // Cleanup, report error and return
+        childProcess.cleanup();
 #if defined (QPROCESS_DEBUG)
         qDebug("fork failed: %ls", qUtf16Printable(qt_error_string(lastForkErrno)));
 #endif
@@ -1337,6 +1347,7 @@ bool QProcessPrivate::startDetached(qint64 *pid)
     closeChannels();
 
     if (childPid == -1) {
+        childProcess.cleanup();
         setErrorAndEmit(QProcess::FailedToStart, "fork: "_L1 + qt_error_string(savedErrno));
         return false;
     }
@@ -1367,6 +1378,7 @@ bool QProcessPrivate::startDetached(qint64 *pid)
     } else if (!success) {
         if (pid)
             *pid = -1;
+        childProcess.cleanup();
         setErrorAndEmit(QProcess::FailedToStart,
                         startFailureErrorMessage(childStatus, startResult));
     }