On a Wednesday in 2020, John Ferlan wrote:
>On error, the @pidfilefd was not released
>
>Found by Coverity
>
The pidfile code was introduced by:
commit d146105f1e4a9e0ab179f0b78c070ea38b9d5334
Author: Michal Prívozník <mprivozn@redhat.com>
CommitDate: 2020-03-24 15:44:23 +0100
virCommand: Actually acquire pidfile instead of just writing it
further changed by:
commit be00118d5d9a3afb41e0edcddec823dff63a7ae1
Author: Marc-André Lureau <marcandre.lureau@redhat.com>
CommitDate: 2020-03-25 09:04:49 +0100
util: keep the pidfile locked
then some code movement added more error paths after the pidfile code:
commit 0bb796bda31103ebf54eefc11c471586c87b95fd
Author: Daniel Henrique Barboza <danielhb413@gmail.com>
CommitDate: 2020-10-02 14:32:57 -0300
vircommand.c: write child pidfile before process tuning in virExec()
IIUC the point of leaking the pidfilefd is to make sure the child holds a
lock on the pidfile - so strictly speaking we should close it in all
code paths that don't end up in a successful execv. Practically,
this already happens because the fork_error calls _exit.
Jano
>Signed-off-by: John Ferlan <jferlan@redhat.com>
>---
> src/util/vircommand.c | 2 ++
> 1 file changed, 2 insertions(+)
>
>diff --git a/src/util/vircommand.c b/src/util/vircommand.c
>index e47dd6b932..1716225aeb 100644
>--- a/src/util/vircommand.c
>+++ b/src/util/vircommand.c
>@@ -792,12 +792,14 @@ virExec(virCommandPtr cmd)
> if (virSetInherit(pidfilefd, true) < 0) {
> virReportSystemError(errno, "%s",
> _("Cannot disable close-on-exec flag"));
>+ virPidFileReleasePath(cmd->pidfile, pidfilefd);
> goto fork_error;
> }
>
> c = '1';
> if (safewrite(pipesync[1], &c, sizeof(c)) != sizeof(c)) {
> virReportSystemError(errno, "%s", _("Unable to notify child process"));
>+ virPidFileReleasePath(cmd->pidfile, pidfilefd);
> goto fork_error;
> }
> VIR_FORCE_CLOSE(pipesync[0]);
>--
>2.28.0
>