From ea6794e88ca66d730a6c0ba76a996ff59491176e Mon Sep 17 00:00:00 2001
From: Daniel Henrique Barboza <danielhb413@gmail.com>
Date: Thu, 1 Oct 2020 10:34:24 -0300
Subject: [PATCH 034/108] vircommand.c: write child pidfile before process
 tuning in virExec()

When VIR_EXEC_DAEMON is true and cmd->pidfile exists, the parent
will expect the pidfile to be written before exiting, sitting
tight in a saferead() call waiting.

The child then does process tuning (via virProcessSet* functions)
before writing the pidfile. Problem is that these tunings can
fail, and trigger a 'fork_error' jump, before cmd->pidfile is
written. The result is that the process was aborted in the
child, but the parent is still hang in the saferead() call.

This behavior can be reproduced by trying to create and execute
a QEMU guest in user mode (e.g. using qemu:///session as non-root).
virProcessSetMaxMemLock() will fail if the spawned libvirtd user
process does not have CAP_SYS_RESOURCE capability. setrlimit() will
fail, and a 'fork_error' jump is triggered before cmd->pidfile
is written. The parent will hung in saferead() indefinitely. From
the user perspective, 'virsh start <guest>' will hang up
indefinitely. CTRL+C can be used to retrieve the terminal, but
any subsequent 'virsh' call will also hang because the previous
libvirtd user process is still there.

We can fix this by moving all virProcessSet*() tuning functions
to be executed after cmd->pidfile is taken care of. In the case
mentioned above, this would be the result of 'virsh start'
after this patch:

error: Failed to start domain vm1
error: internal error: Process exited prior to exec: libvirt:  error :
cannot limit locked memory to 79691776: Operation not permitted

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1882093

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
(cherry picked from commit 0bb796bda31103ebf54eefc11c471586c87b95fd)
---
 src/util/vircommand.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/src/util/vircommand.c b/src/util/vircommand.c
index 86e4c5cd39..8ab83cb8f5 100644
--- a/src/util/vircommand.c
+++ b/src/util/vircommand.c
@@ -787,15 +787,6 @@ virExec(virCommandPtr cmd)
         }
     }
 
-    if (virProcessSetMaxMemLock(0, cmd->maxMemLock) < 0)
-        goto fork_error;
-    if (virProcessSetMaxProcesses(0, cmd->maxProcesses) < 0)
-        goto fork_error;
-    if (virProcessSetMaxFiles(0, cmd->maxFiles) < 0)
-        goto fork_error;
-    if (cmd->setMaxCore &&
-        virProcessSetMaxCoreSize(0, cmd->maxCore) < 0)
-        goto fork_error;
     if (cmd->pidfile) {
         int pidfilefd = -1;
         char c;
@@ -820,6 +811,16 @@ virExec(virCommandPtr cmd)
         /* pidfilefd is intentionally leaked. */
     }
 
+    if (virProcessSetMaxMemLock(0, cmd->maxMemLock) < 0)
+        goto fork_error;
+    if (virProcessSetMaxProcesses(0, cmd->maxProcesses) < 0)
+        goto fork_error;
+    if (virProcessSetMaxFiles(0, cmd->maxFiles) < 0)
+        goto fork_error;
+    if (cmd->setMaxCore &&
+        virProcessSetMaxCoreSize(0, cmd->maxCore) < 0)
+        goto fork_error;
+
     if (cmd->hook) {
         VIR_DEBUG("Run hook %p %p", cmd->hook, cmd->opaque);
         ret = cmd->hook(cmd->opaque);
-- 
2.33.0