Re: [PATCH 2/7] util: Resolve resource leak in virExec error path
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(a)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(a)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(a)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(a)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
Attachments:
- signature.asc (application/pgp-signature — 488 bytes)