QProcess/Unix: use open() + fchdir() to change directories

This means we have more system calls (2 more in the parent), but we
can now detect non-existent or inaccessible directories before fork().

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

1 files changed, 45 insertions, 18 deletions

diff --git a/src/corelib/io/qprocess_unix.cpp b/src/corelib/io/qprocess_unix.cpp
index 8c07927ad4..5c72a39911 100644
--- a/src/corelib/io/qprocess_unix.cpp
+++ b/src/corelib/io/qprocess_unix.cpp
@@ -41,6 +41,10 @@
 #include <forkfd.h>
 #endif
 
+#ifndef O_PATH
+#  define O_PATH        0
+#endif
+
 QT_BEGIN_NAMESPACE
 
 using namespace Qt::StringLiterals;
@@ -72,6 +76,16 @@ QProcessEnvironment QProcessEnvironment::systemEnvironment()
 
 #if QT_CONFIG(process)
 
+static int opendirfd(QByteArray encodedName)
+{
+    // We append "/." to the name to ensure that the directory is actually
+    // traversable (i.e., has the +x bit set). This avoids later problems
+    // with fchdir().
+    if (encodedName != "/" && !encodedName.endsWith("/."))
+        encodedName += "/.";
+    return qt_safe_open(encodedName, QT_OPEN_RDONLY | O_DIRECTORY | O_PATH);
+}
+
 namespace {
 struct AutoPipe
 {
@@ -441,6 +455,16 @@ void QProcessPrivate::startProcess()
                          q, SLOT(_q_startupNotification()));
     }
 
+    int workingDirFd = -1;
+    if (!workingDirectory.isEmpty()) {
+        workingDirFd = opendirfd(QFile::encodeName(workingDirectory));
+        if (workingDirFd == -1) {
+            setErrorAndEmit(QProcess::FailedToStart, "chdir: "_L1 + qt_error_string());
+            cleanup();
+            return;
+        }
+    }
+
     // Start the process (platform dependent)
     q->setProcessState(QProcess::Starting);
 
@@ -448,17 +472,9 @@ void QProcessPrivate::startProcess()
     const CharPointerList argv(resolveExecutable(program), arguments);
     const CharPointerList envp(environment.d.constData());
 
-    // Encode the working directory if it's non-empty, otherwise just pass 0.
-    const char *workingDirPtr = nullptr;
-    QByteArray encodedWorkingDirectory;
-    if (!workingDirectory.isEmpty()) {
-        encodedWorkingDirectory = QFile::encodeName(workingDirectory);
-        workingDirPtr = encodedWorkingDirectory.constData();
-    }
-
     // Start the child.
-    auto execChild1 = [this, workingDirPtr, &argv, &envp]() {
-        execChild(workingDirPtr, argv.pointers.get(), envp.pointers.get());
+    auto execChild1 = [this, workingDirFd, &argv, &envp]() {
+        execChild(workingDirFd, argv.pointers.get(), envp.pointers.get());
     };
     auto execChild2 = [](void *lambda) {
         static_cast<decltype(execChild1) *>(lambda)->operator()();
@@ -477,6 +493,9 @@ void QProcessPrivate::startProcess()
     forkfd = ::vforkfd(ffdflags, &pid, execChild2, &execChild1);
     int lastForkErrno = errno;
 
+    if (workingDirFd != -1)
+        close(workingDirFd);
+
     if (forkfd == -1) {
         // Cleanup, report error and return
 #if defined (QPROCESS_DEBUG)
@@ -525,7 +544,7 @@ void QProcessPrivate::startProcess()
 // 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(const char *workingDir, char **argv, char **envp) const
+void QProcessPrivate::execChild(int workingDir, char **argv, char **envp) const
 {
     ::signal(SIGPIPE, SIG_DFL);         // reset the signal that we ignored
 
@@ -538,9 +557,9 @@ void QProcessPrivate::execChild(const char *workingDir, char **argv, char **envp
     qt_safe_close(childStartedPipe[0]);
 
     // enter the working directory
-    if (workingDir && QT_CHDIR(workingDir) == -1) {
+    if (workingDir != -1 && fchdir(workingDir) == -1) {
         // failed, stop the process
-        strcpy(error.function, "chdir");
+        strcpy(error.function, "fchdir");
         goto report_errno;
     }
 
@@ -914,7 +933,6 @@ void QProcessPrivate::waitForDeadChild()
 
 bool QProcessPrivate::startDetached(qint64 *pid)
 {
-    QByteArray encodedWorkingDirectory = QFile::encodeName(workingDirectory);
 
 #ifdef PIPE_BUF
     static_assert(PIPE_BUF >= sizeof(ChildError));
@@ -935,6 +953,15 @@ bool QProcessPrivate::startDetached(qint64 *pid)
         return false;
     }
 
+    int workingDirFd = -1;
+    if (!workingDirectory.isEmpty()) {
+        workingDirFd = opendirfd(QFile::encodeName(workingDirectory));
+        if (workingDirFd == -1) {
+            setErrorAndEmit(QProcess::FailedToStart, "chdir: "_L1 + qt_error_string(errno));
+            return false;
+        }
+    }
+
     const CharPointerList argv(resolveExecutable(program), arguments);
     const CharPointerList envp(environment.d.constData());
 
@@ -953,10 +980,8 @@ bool QProcessPrivate::startDetached(qint64 *pid)
             ::_exit(1);
         };
 
-        if (!encodedWorkingDirectory.isEmpty()) {
-            if (QT_CHDIR(encodedWorkingDirectory.constData()) < 0)
-                reportFailed("chdir: ");
-        }
+        if (workingDirFd != -1 && fchdir(workingDirFd) == -1)
+            reportFailed("fchdir: ");
 
         pid_t doubleForkPid = fork();
         if (doubleForkPid == 0) {
@@ -980,6 +1005,8 @@ bool QProcessPrivate::startDetached(qint64 *pid)
 
     int savedErrno = errno;
     closeChannels();
+    if (workingDirFd != -1)
+        close(workingDirFd);
 
     if (childPid == -1) {
         setErrorAndEmit(QProcess::FailedToStart, "fork: "_L1 + qt_error_string(savedErrno));