[PATCH 2/7] util: Resolve resource leak in virExec error path

[Ján Tomko]

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 at 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 at 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 at 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 at 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
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20201202/ff96f381/attachment-0001.sig>