[PATCH 1/1] vircommand.c: write child pidfile before process tuning in virExec()

[Daniel Henrique Barboza]

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

Signed-off-by: Daniel Henrique Barboza <danielhb413 at gmail.com>
---
 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 76f7eb9a3d..0475e22db6 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.26.2