From ba5e2ce49a43c7d68e2ffa57b9e7f8d5d7fafe2f Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Fri, 21 Feb 2020 20:40:35 -0800 Subject: forkfd: fix forkfd_wait when FFD_USE_FORK was active If we detected that the OS supports a version of system forkfd (Linux pidfd, FreeBSD procdesc), the forkfd_wait() function was using only the system waiting implementation, which of course can't work for file descriptors created with FFD_USE_FORK. So just detect EBADF and attempt again. If the file descriptor is neither one of our pipes nor a system forkfd, bad things will happen... Fixes: QTBUG-82351 Change-Id: I4e559af2a9a1455ab770fffd15f59fb3160b22eb Reviewed-by: Edward Welbourne Reviewed-by: Oswald Buddenhagen --- src/3rdparty/forkfd/forkfd.c | 28 +++++++++++-- tests/auto/corelib/io/qprocess/tst_qprocess.cpp | 56 ++++++++++++++++++++++++- 2 files changed, 79 insertions(+), 5 deletions(-) diff --git a/src/3rdparty/forkfd/forkfd.c b/src/3rdparty/forkfd/forkfd.c index 795aa9dd68..80f0e448e6 100644 --- a/src/3rdparty/forkfd/forkfd.c +++ b/src/3rdparty/forkfd/forkfd.c @@ -29,6 +29,11 @@ #include "forkfd.h" +/* Macros fine-tuning the build: */ +//#define FORKFD_NO_FORKFD 1 /* disable the forkfd() function */ +//#define FORKFD_NO_SPAWNFD 1 /* disable the spawnfd() function */ +//#define FORKFD_DISABLE_FORK_FALLBACK 1 /* disable falling back to fork() from system_forkfd() */ + #include #if defined(__OpenBSD__) || defined(__NetBSD__) # include @@ -96,6 +101,16 @@ static int system_has_forkfd(void); static int system_forkfd(int flags, pid_t *ppid, int *system); static int system_forkfd_wait(int ffd, struct forkfd_info *info, int ffdwoptions, struct rusage *rusage); +static int disable_fork_fallback(void) +{ +#ifdef FORKFD_DISABLE_FORK_FALLBACK + /* if there's no system forkfd, we have to use the fallback */ + return system_has_forkfd(); +#else + return false; +#endif +} + #define CHILDREN_IN_SMALL_ARRAY 16 #define CHILDREN_IN_BIG_ARRAY 256 #define sizeofarray(array) (sizeof(array)/sizeof(array[0])) @@ -629,9 +644,12 @@ int forkfd(int flags, pid_t *ppid) int efd; #endif + if (disable_fork_fallback()) + flags &= ~FFD_USE_FORK; + if ((flags & FFD_USE_FORK) == 0) { fd = system_forkfd(flags, ppid, &ret); - if (ret) + if (ret || disable_fork_fallback()) return fd; } @@ -815,8 +833,12 @@ int forkfd_wait4(int ffd, struct forkfd_info *info, int options, struct rusage * struct pipe_payload payload; int ret; - if (system_has_forkfd()) - return system_forkfd_wait(ffd, info, options, rusage); + if (system_has_forkfd()) { + /* if this is one of our pipes, not a procdesc/pidfd, we'll get an EBADF */ + ret = system_forkfd_wait(ffd, info, options, rusage); + if (disable_fork_fallback() || ret != -1 || errno != EBADF) + return ret; + } ret = read(ffd, &payload, sizeof(payload)); if (ret == -1) diff --git a/tests/auto/corelib/io/qprocess/tst_qprocess.cpp b/tests/auto/corelib/io/qprocess/tst_qprocess.cpp index 3de1bef789..9495631c23 100644 --- a/tests/auto/corelib/io/qprocess/tst_qprocess.cpp +++ b/tests/auto/corelib/io/qprocess/tst_qprocess.cpp @@ -1,7 +1,7 @@ /**************************************************************************** ** -** Copyright (C) 2016 The Qt Company Ltd. -** Copyright (C) 2016 Intel Corporation. +** Copyright (C) 2020 The Qt Company Ltd. +** Copyright (C) 2020 Intel Corporation. ** Contact: https://www.qt.io/licensing/ ** ** This file is part of the test suite of the Qt Toolkit. @@ -40,6 +40,12 @@ #include #include #include + +#include +#ifdef Q_OS_UNIX +# include +#endif + #include typedef void (QProcess::*QProcessFinishedSignal1)(int); @@ -59,6 +65,7 @@ private slots: void getSetCheck(); void constructing(); void simpleStart(); + void setupChildProcess(); void startWithOpen(); void startWithOldOpen(); void execute(); @@ -277,6 +284,51 @@ void tst_QProcess::simpleStart() QCOMPARE(qvariant_cast(spy.at(2).at(0)), QProcess::NotRunning); } +void tst_QProcess::setupChildProcess() +{ + /* This test exists because in Qt 5.15, the Unix version of QProcess has + * some code that depends on whether it's an actual QProcess or a + * derived class */ + static const char setupChildMessage[] = "Called from setupChildProcess()"; + class DerivedProcessClass : public QProcess { + public: + int fd; + DerivedProcessClass(int fd) : fd(fd) + { + } + + protected: + void setupChildProcess() override + { + QT_WRITE(fd, setupChildMessage, sizeof(setupChildMessage) - 1); + QT_CLOSE(fd); + } + }; + + int pipes[2] = { -1 , -1 }; +#ifdef Q_OS_UNIX + QVERIFY(qt_safe_pipe(pipes) == 0); +#endif + + DerivedProcessClass process(pipes[1]); + process.start("testProcessNormal/testProcessNormal"); + if (process.state() != QProcess::Starting) + QCOMPARE(process.state(), QProcess::Running); + QVERIFY2(process.waitForStarted(5000), qPrintable(process.errorString())); + +#ifdef Q_OS_UNIX + char buf[sizeof setupChildMessage] = {}; + qt_safe_close(pipes[1]); + QCOMPARE(qt_safe_read(pipes[0], buf, sizeof(buf)), qint64(sizeof(setupChildMessage) - 1)); + QCOMPARE(buf, setupChildMessage); + qt_safe_close(pipes[0]); +#endif + + QVERIFY2(process.waitForFinished(5000), qPrintable(process.errorString())); + QCOMPARE(process.exitStatus(), QProcess::NormalExit); + QCOMPARE(process.exitCode(), 0); +} + void tst_QProcess::startWithOpen() { QProcess p; -- cgit v1.2.3