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>
This commit is contained in:
Thiago Macieira 2023-03-16 18:17:22 -07:00 committed by Volker Hilsheimer
parent 6b561ccf44
commit 49eb9021e3
2 changed files with 46 additions and 19 deletions

View File

@ -303,7 +303,7 @@ public:
void startProcess(); void startProcess();
#if defined(Q_OS_UNIX) #if defined(Q_OS_UNIX)
void commitChannels() const; void commitChannels() const;
void execChild(const char *workingDirectory, char **argv, char **envp) const; void execChild(int workingDirectory, char **argv, char **envp) const;
#endif #endif
bool processStarted(QString *errorMessage = nullptr); bool processStarted(QString *errorMessage = nullptr);
void processFinished(); void processFinished();

View File

@ -41,6 +41,10 @@
#include <forkfd.h> #include <forkfd.h>
#endif #endif
#ifndef O_PATH
# define O_PATH 0
#endif
QT_BEGIN_NAMESPACE QT_BEGIN_NAMESPACE
using namespace Qt::StringLiterals; using namespace Qt::StringLiterals;
@ -72,6 +76,16 @@ QProcessEnvironment QProcessEnvironment::systemEnvironment()
#if QT_CONFIG(process) #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 { namespace {
struct AutoPipe struct AutoPipe
{ {
@ -441,6 +455,16 @@ void QProcessPrivate::startProcess()
q, SLOT(_q_startupNotification())); 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) // Start the process (platform dependent)
q->setProcessState(QProcess::Starting); q->setProcessState(QProcess::Starting);
@ -448,17 +472,9 @@ void QProcessPrivate::startProcess()
const CharPointerList argv(resolveExecutable(program), arguments); const CharPointerList argv(resolveExecutable(program), arguments);
const CharPointerList envp(environment.d.constData()); 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. // Start the child.
auto execChild1 = [this, workingDirPtr, &argv, &envp]() { auto execChild1 = [this, workingDirFd, &argv, &envp]() {
execChild(workingDirPtr, argv.pointers.get(), envp.pointers.get()); execChild(workingDirFd, argv.pointers.get(), envp.pointers.get());
}; };
auto execChild2 = [](void *lambda) { auto execChild2 = [](void *lambda) {
static_cast<decltype(execChild1) *>(lambda)->operator()(); static_cast<decltype(execChild1) *>(lambda)->operator()();
@ -477,6 +493,9 @@ void QProcessPrivate::startProcess()
forkfd = ::vforkfd(ffdflags, &pid, execChild2, &execChild1); forkfd = ::vforkfd(ffdflags, &pid, execChild2, &execChild1);
int lastForkErrno = errno; int lastForkErrno = errno;
if (workingDirFd != -1)
close(workingDirFd);
if (forkfd == -1) { if (forkfd == -1) {
// Cleanup, report error and return // Cleanup, report error and return
#if defined (QPROCESS_DEBUG) #if defined (QPROCESS_DEBUG)
@ -525,7 +544,7 @@ void QProcessPrivate::startProcess()
// This function is called in a vfork() context on some OSes (notably, Linux // 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 // with forkfd), so it MUST NOT modify any non-local variable because it's
// still sharing memory with the parent process. // 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 ::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]); qt_safe_close(childStartedPipe[0]);
// enter the working directory // enter the working directory
if (workingDir && QT_CHDIR(workingDir) == -1) { if (workingDir != -1 && fchdir(workingDir) == -1) {
// failed, stop the process // failed, stop the process
strcpy(error.function, "chdir"); strcpy(error.function, "fchdir");
goto report_errno; goto report_errno;
} }
@ -914,7 +933,6 @@ void QProcessPrivate::waitForDeadChild()
bool QProcessPrivate::startDetached(qint64 *pid) bool QProcessPrivate::startDetached(qint64 *pid)
{ {
QByteArray encodedWorkingDirectory = QFile::encodeName(workingDirectory);
#ifdef PIPE_BUF #ifdef PIPE_BUF
static_assert(PIPE_BUF >= sizeof(ChildError)); static_assert(PIPE_BUF >= sizeof(ChildError));
@ -935,6 +953,15 @@ bool QProcessPrivate::startDetached(qint64 *pid)
return false; 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 argv(resolveExecutable(program), arguments);
const CharPointerList envp(environment.d.constData()); const CharPointerList envp(environment.d.constData());
@ -953,10 +980,8 @@ bool QProcessPrivate::startDetached(qint64 *pid)
::_exit(1); ::_exit(1);
}; };
if (!encodedWorkingDirectory.isEmpty()) { if (workingDirFd != -1 && fchdir(workingDirFd) == -1)
if (QT_CHDIR(encodedWorkingDirectory.constData()) < 0) reportFailed("fchdir: ");
reportFailed("chdir: ");
}
pid_t doubleForkPid = fork(); pid_t doubleForkPid = fork();
if (doubleForkPid == 0) { if (doubleForkPid == 0) {
@ -980,6 +1005,8 @@ bool QProcessPrivate::startDetached(qint64 *pid)
int savedErrno = errno; int savedErrno = errno;
closeChannels(); closeChannels();
if (workingDirFd != -1)
close(workingDirFd);
if (childPid == -1) { if (childPid == -1) {
setErrorAndEmit(QProcess::FailedToStart, "fork: "_L1 + qt_error_string(savedErrno)); setErrorAndEmit(QProcess::FailedToStart, "fork: "_L1 + qt_error_string(savedErrno));