From 87fbcf438cbc7140e2167a42be0554a6af077b08 Mon Sep 17 00:00:00 2001
From: Thiago Macieira <thiago.macieira@intel.com>
Date: Mon, 26 Aug 2024 18:37:16 -0700
Subject: [PATCH] QProcess/Unix: open the working directory before changing the
 thread state

This operation can fail and, in handling the failure, we call functions
that may allocate memory and thus throw. If that happens, our destructor
won't be called, so this commit avoids leaving the thread in an
incorrect state by not changing it in the first place.

In case there was a failure but no exception happened (a much more
likely scenario), the destructor will be called by startProcess() or
startDetached(). Therefore, we need to ensure it won't cause problems.

Change-Id: I091193bb945a2637f463fffdd93694e555401452
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
(cherry picked from commit 34edfe134403e331f0fa4550a074507a393bdc52)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
---
 src/corelib/io/qprocess_unix.cpp | 31 +++++++++++++++++++------------
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/src/corelib/io/qprocess_unix.cpp b/src/corelib/io/qprocess_unix.cpp
index d434c8eb11e..e304be3f14d 100644
--- a/src/corelib/io/qprocess_unix.cpp
+++ b/src/corelib/io/qprocess_unix.cpp
@@ -257,6 +257,20 @@ struct QChildProcess
         : d(d), argv(resolveExecutable(d->program), d->arguments),
           envp(d->environmentPrivate())
     {
+        // Open the working directory first, because this operation can fail.
+        // That way, if it does, we don't have anything to clean up.
+        if (!d->workingDirectory.isEmpty()) {
+            workingDirectory = opendirfd(QFile::encodeName(d->workingDirectory));
+            if (workingDirectory < 0) {
+                d->setErrorAndEmit(QProcess::FailedToStart, "chdir: "_L1 + qt_error_string());
+                d->cleanup();
+
+                // make sure our destructor does nothing
+                isUsingVfork = false;
+                return;
+            }
+        }
+
         // Block Unix signals, to ensure the user's handlers aren't run in the
         // child side and do something weird, especially if the handler and the
         // user of QProcess are completely different codebases.
@@ -268,15 +282,6 @@ struct QChildProcess
         // would be bad enough with regular fork(), but it's likely fatal with
         // vfork().
         disableThreadCancellations();
-
-        if (!d->workingDirectory.isEmpty()) {
-            workingDirectory = opendirfd(QFile::encodeName(d->workingDirectory));
-            if (workingDirectory < 0) {
-                d->setErrorAndEmit(QProcess::FailedToStart, "chdir: "_L1 + qt_error_string());
-                d->cleanup();
-            }
-        }
-
     }
     ~QChildProcess() noexcept(false)
     {
@@ -335,7 +340,7 @@ private:
     }
 
 #if defined(PTHREAD_CANCEL_DISABLE)
-    int oldstate;
+    int oldstate = PTHREAD_CANCEL_DISABLE;
     void disableThreadCancellations() noexcept
     {
         // the following is *not* noexcept, but it won't throw while disabling
@@ -343,8 +348,10 @@ private:
     }
     void restoreThreadCancellations() noexcept(false)
     {
-        // this doesn't touch errno
-        pthread_setcancelstate(oldstate, nullptr);
+        if (oldstate != PTHREAD_CANCEL_DISABLE) {
+            // this doesn't touch errno
+            pthread_setcancelstate(oldstate, nullptr);
+        }
     }
 #else
     void disableThreadCancellations() noexcept {}