QProcess/Linux: fix file descriptor leak in case of failed child start
If the child failed to start, execChild() would (unnecessarily) store -1
in childStartedPipe[1] after writing the failure message to it and
closing that pipe. This had worked for the previous 20 years of QProcess
existence, because that was run in the child process. However, with 6.5
commit e1a787a76e
(cherry-picked to 6.4)
we implemented vfork-like behavior, meaning the child would share memory
with the parent and ran before the parent, so startProcess() failed to
close it:
// parent
// close the ends we don't use and make all pipes non-blocking
qt_safe_close(childStartedPipe[1]);
Also updated the docs to account for the fact that this is a vfork()
environment and, moreover, not using the vfork() function from glibc on
Linux.
[ChangeLog][QtCore][QProcess] Fixed a file descriptor leak in QProcess
when running on Linux 5.4 or later, if the executable being run failed
to start (for example, the file for the executable didn't exist).
Pick-to: 6.4.3 6.4 6.5
Task-number: QTBUG-111243
Change-Id: I7f354474adce419ca6c2fffd17481002e4853cc3
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
This commit is contained in:
parent
96dc4acb23
commit
82b75570f0
@ -1574,7 +1574,7 @@ std::function<void(void)> QProcess::childProcessModifier() const
|
|||||||
Sets the \a modifier function for the child process, for Unix systems
|
Sets the \a modifier function for the child process, for Unix systems
|
||||||
(including \macos; for Windows, see setCreateProcessArgumentsModifier()).
|
(including \macos; for Windows, see setCreateProcessArgumentsModifier()).
|
||||||
The function contained by the \a modifier argument will be invoked in the
|
The function contained by the \a modifier argument will be invoked in the
|
||||||
child process after \c{fork()} is completed and QProcess has set up the
|
child process after \c{fork()} or \c{vfork()} is completed and QProcess has set up the
|
||||||
standard file descriptors for the child process, but before \c{execve()},
|
standard file descriptors for the child process, but before \c{execve()},
|
||||||
inside start(). The modifier is useful to change certain properties of the
|
inside start(). The modifier is useful to change certain properties of the
|
||||||
child process, such as setting up additional file descriptors or closing
|
child process, such as setting up additional file descriptors or closing
|
||||||
@ -1595,6 +1595,15 @@ std::function<void(void)> QProcess::childProcessModifier() const
|
|||||||
"async-signal-safe" is advised). Most of the Qt API is unsafe inside this
|
"async-signal-safe" is advised). Most of the Qt API is unsafe inside this
|
||||||
callback, including qDebug(), and may lead to deadlocks.
|
callback, including qDebug(), and may lead to deadlocks.
|
||||||
|
|
||||||
|
\note On some systems (notably, Linux), QProcess will use \c{vfork()}
|
||||||
|
semantics to start the child process, so this function must obey even
|
||||||
|
stricter constraints. First, because it is still sharing memory with the
|
||||||
|
parent process, it must not write to any non-local variable and must obey
|
||||||
|
proper ordering semantics when reading from them, to avoid data races.
|
||||||
|
Second, even more library functions may misbehave; therefore, this function
|
||||||
|
should only make use of low-level system calls, such as \c{read()},
|
||||||
|
\c{write()}, \c{setsid()}, \c{nice()}, and similar.
|
||||||
|
|
||||||
\sa childProcessModifier()
|
\sa childProcessModifier()
|
||||||
*/
|
*/
|
||||||
void QProcess::setChildProcessModifier(const std::function<void(void)> &modifier)
|
void QProcess::setChildProcessModifier(const std::function<void(void)> &modifier)
|
||||||
|
@ -279,9 +279,6 @@ public:
|
|||||||
bool openChannels();
|
bool openChannels();
|
||||||
bool openChannelsForDetached();
|
bool openChannelsForDetached();
|
||||||
bool openChannel(Channel &channel);
|
bool openChannel(Channel &channel);
|
||||||
#if defined(Q_OS_UNIX)
|
|
||||||
void commitChannels();
|
|
||||||
#endif
|
|
||||||
void closeChannel(Channel *channel);
|
void closeChannel(Channel *channel);
|
||||||
void closeWriteChannel();
|
void closeWriteChannel();
|
||||||
void closeChannels();
|
void closeChannels();
|
||||||
@ -308,7 +305,8 @@ public:
|
|||||||
void start(QIODevice::OpenMode mode);
|
void start(QIODevice::OpenMode mode);
|
||||||
void startProcess();
|
void startProcess();
|
||||||
#if defined(Q_OS_UNIX)
|
#if defined(Q_OS_UNIX)
|
||||||
void execChild(const char *workingDirectory, char **argv, char **envp);
|
void commitChannels() const;
|
||||||
|
void execChild(const char *workingDirectory, char **argv, char **envp) const;
|
||||||
#endif
|
#endif
|
||||||
bool processStarted(QString *errorMessage = nullptr);
|
bool processStarted(QString *errorMessage = nullptr);
|
||||||
void processFinished();
|
void processFinished();
|
||||||
|
@ -358,7 +358,7 @@ bool QProcessPrivate::openChannel(Channel &channel)
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
void QProcessPrivate::commitChannels()
|
void QProcessPrivate::commitChannels() const
|
||||||
{
|
{
|
||||||
// copy the stdin socket if asked to (without closing on exec)
|
// copy the stdin socket if asked to (without closing on exec)
|
||||||
if (stdinChannel.pipe[0] != INVALID_Q_PIPE)
|
if (stdinChannel.pipe[0] != INVALID_Q_PIPE)
|
||||||
@ -516,7 +516,12 @@ void QProcessPrivate::startProcess()
|
|||||||
::fcntl(stderrChannel.pipe[0], F_SETFL, ::fcntl(stderrChannel.pipe[0], F_GETFL) | O_NONBLOCK);
|
::fcntl(stderrChannel.pipe[0], F_SETFL, ::fcntl(stderrChannel.pipe[0], F_GETFL) | O_NONBLOCK);
|
||||||
}
|
}
|
||||||
|
|
||||||
void QProcessPrivate::execChild(const char *workingDir, char **argv, char **envp)
|
// IMPORTANT:
|
||||||
|
//
|
||||||
|
// 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
|
||||||
{
|
{
|
||||||
::signal(SIGPIPE, SIG_DFL); // reset the signal that we ignored
|
::signal(SIGPIPE, SIG_DFL); // reset the signal that we ignored
|
||||||
|
|
||||||
@ -556,7 +561,6 @@ void QProcessPrivate::execChild(const char *workingDir, char **argv, char **envp
|
|||||||
report_errno:
|
report_errno:
|
||||||
error.code = errno;
|
error.code = errno;
|
||||||
qt_safe_write(childStartedPipe[1], &error, sizeof(error));
|
qt_safe_write(childStartedPipe[1], &error, sizeof(error));
|
||||||
childStartedPipe[1] = -1;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
bool QProcessPrivate::processStarted(QString *errorMessage)
|
bool QProcessPrivate::processStarted(QString *errorMessage)
|
||||||
|
Loading…
Reference in New Issue
Block a user