virtlockd: make re-exec more robust

- Use $XDG_RUNTIME_DIR for re-exec state file when running unprivileged.

- argv[0] may not contain a full path to the binary, however it should
  contain something that can be looked up in the PATH. Use execvp() to
  do path lookup on re-exec.

- As per list discussion [1], ignore --daemon on re-exec.

[1] https://www.redhat.com/archives/libvir-list/2013-December/msg00514.htmlSigned-off-by: NMichael Chapman <mike@very.puzzling.org>

src/locking/lock_daemon.c

@@ -923,7 +923,42 @@ error:
 }
 
 
-#define VIR_LOCK_DAEMON_RESTART_EXEC_FILE LOCALSTATEDIR "/run/virtlockd-restart-exec.json"
+static int
+virLockDaemonExecRestartStatePath(bool privileged,
+                                  char **state_file)
+{
+    if (privileged) {
+        if (VIR_STRDUP(*state_file, LOCALSTATEDIR "/run/virtlockd-restart-exec.json") < 0)
+            goto error;
+    } else {
+        char *rundir = NULL;
+        mode_t old_umask;
+
+        if (!(rundir = virGetUserRuntimeDirectory()))
+            goto error;
+
+        old_umask = umask(077);
+        if (virFileMakePath(rundir) < 0) {
+            umask(old_umask);
+            VIR_FREE(rundir);
+            goto error;
+        }
+        umask(old_umask);
+
+        if (virAsprintf(state_file, "%s/virtlockd-restart-exec.json", rundir) < 0) {
+            VIR_FREE(rundir);
+            goto error;
+        }
+
+        VIR_FREE(rundir);
+    }
+
+    return 0;
+
+error:
+    return -1;
+}
+
 
 static char *
 virLockDaemonGetExecRestartMagic(void)
@@ -936,7 +971,10 @@ virLockDaemonGetExecRestartMagic(void)
 
 
 static int
-virLockDaemonPostExecRestart(bool privileged)
+virLockDaemonPostExecRestart(const char *state_file,
+                             const char *pid_file,
+                             int *pid_file_fd,
+                             bool privileged)
 {
     const char *gotmagic;
     char *wantmagic = NULL;
@@ -946,14 +984,14 @@ virLockDaemonPostExecRestart(bool privileged)
 
     VIR_DEBUG("Running post-restart exec");
 
-    if (!virFileExists(VIR_LOCK_DAEMON_RESTART_EXEC_FILE)) {
-        VIR_DEBUG("No restart file %s present",
-                  VIR_LOCK_DAEMON_RESTART_EXEC_FILE);
+    if (!virFileExists(state_file)) {
+        VIR_DEBUG("No restart state file %s present",
+                  state_file);
         ret = 0;
         goto cleanup;
     }
 
-    if (virFileReadAll(VIR_LOCK_DAEMON_RESTART_EXEC_FILE,
+    if (virFileReadAll(state_file,
                        1024 * 1024 * 10, /* 10 MB */
                        &state) < 0)
         goto cleanup;
@@ -980,13 +1018,18 @@ virLockDaemonPostExecRestart(bool privileged)
         goto cleanup;
     }
 
+    /* Re-claim PID file now as we will not be daemonizing */
+    if (pid_file &&
+        (*pid_file_fd = virPidFileAcquirePath(pid_file, getpid())) < 0)
+        goto cleanup;
+
     if (!(lockDaemon = virLockDaemonNewPostExecRestart(object, privileged)))
         goto cleanup;
 
     ret = 1;
 
 cleanup:
-    unlink(VIR_LOCK_DAEMON_RESTART_EXEC_FILE);
+    unlink(state_file);
     VIR_FREE(wantmagic);
     VIR_FREE(state);
     virJSONValueFree(object);
@@ -995,7 +1038,8 @@ cleanup:
 
 
 static int
-virLockDaemonPreExecRestart(virNetServerPtr srv,
+virLockDaemonPreExecRestart(const char *state_file,
+                            virNetServerPtr srv,
                             char **argv)
 {
     virJSONValuePtr child;
@@ -1063,15 +1107,15 @@ virLockDaemonPreExecRestart(virNetServerPtr srv,
 
     VIR_DEBUG("Saving state %s", state);
 
-    if (virFileWriteStr(VIR_LOCK_DAEMON_RESTART_EXEC_FILE,
+    if (virFileWriteStr(state_file,
                         state, 0700) < 0) {
         virReportSystemError(errno,
                              _("Unable to save state file %s"),
-                             VIR_LOCK_DAEMON_RESTART_EXEC_FILE);
+                             state_file);
         goto cleanup;
     }
 
-    if (execv(argv[0], argv) < 0) {
+    if (execvp(argv[0], argv) < 0) {
         virReportSystemError(errno, "%s",
                              _("Unable to restart self"));
         goto cleanup;
@@ -1151,6 +1195,7 @@ int main(int argc, char **argv) {
     char *pid_file = NULL;
     int pid_file_fd = -1;
     char *sock_file = NULL;
+    char *state_file = NULL;
     bool implicit_conf = false;
     mode_t old_umask;
     bool privileged = false;
@@ -1274,21 +1319,13 @@ int main(int argc, char **argv) {
     VIR_DEBUG("Decided on socket paths '%s'",
               sock_file);
 
-    if (godaemon) {
-        char ebuf[1024];
-
-        if (chdir("/") < 0) {
-            VIR_ERROR(_("cannot change to root directory: %s"),
-                      virStrerror(errno, ebuf, sizeof(ebuf)));
-            goto cleanup;
-        }
-
-        if ((statuswrite = virLockDaemonForkIntoBackground(argv[0])) < 0) {
-            VIR_ERROR(_("Failed to fork as daemon: %s"),
-                      virStrerror(errno, ebuf, sizeof(ebuf)));
-            goto cleanup;
-        }
+    if (virLockDaemonExecRestartStatePath(privileged,
+                                          &state_file) < 0) {
+        VIR_ERROR(_("Can't determine restart state file path"));
+        exit(EXIT_FAILURE);
     }
+    VIR_DEBUG("Decided on restart state file path '%s'",
+              state_file);
 
     /* Ensure the rundir exists (on tmpfs on some systems) */
     if (privileged) {
@@ -1315,20 +1352,41 @@ int main(int argc, char **argv) {
     }
     umask(old_umask);
 
-    /* If we have a pidfile set, claim it now, exiting if already taken */
-    if ((pid_file_fd = virPidFileAcquirePath(pid_file, getpid())) < 0) {
-        ret = VIR_LOCK_DAEMON_ERR_PIDFILE;
-        goto cleanup;
-    }
-
-    if ((rv = virLockDaemonPostExecRestart(privileged)) < 0) {
+    if ((rv = virLockDaemonPostExecRestart(state_file,
+                                           pid_file,
+                                           &pid_file_fd,
+                                           privileged)) < 0) {
         ret = VIR_LOCK_DAEMON_ERR_INIT;
         goto cleanup;
     }
 
     /* rv == 1, means we setup everything from saved state,
-     * so we only setup stuff from scratch if rv == 0 */
+     * so only (possibly) daemonize and setup stuff from
+     * scratch if rv == 0
+     */
     if (rv == 0) {
+        if (godaemon) {
+            char ebuf[1024];
+
+            if (chdir("/") < 0) {
+                VIR_ERROR(_("cannot change to root directory: %s"),
+                          virStrerror(errno, ebuf, sizeof(ebuf)));
+                goto cleanup;
+            }
+
+            if ((statuswrite = virLockDaemonForkIntoBackground(argv[0])) < 0) {
+                VIR_ERROR(_("Failed to fork as daemon: %s"),
+                          virStrerror(errno, ebuf, sizeof(ebuf)));
+                goto cleanup;
+            }
+        }
+
+        /* If we have a pidfile set, claim it now, exiting if already taken */
+        if ((pid_file_fd = virPidFileAcquirePath(pid_file, getpid())) < 0) {
+            ret = VIR_LOCK_DAEMON_ERR_PIDFILE;
+            goto cleanup;
+        }
+
         if (!(lockDaemon = virLockDaemonNew(config, privileged))) {
             ret = VIR_LOCK_DAEMON_ERR_INIT;
             goto cleanup;
@@ -1386,7 +1444,9 @@ int main(int argc, char **argv) {
     virNetServerRun(lockDaemon->srv);
 
     if (execRestart &&
-        virLockDaemonPreExecRestart(lockDaemon->srv, argv) < 0)
+        virLockDaemonPreExecRestart(state_file,
+                                    lockDaemon->srv,
+                                    argv) < 0)
         ret = VIR_LOCK_DAEMON_ERR_REEXEC;
     else
         ret = 0;
@@ -1408,6 +1468,7 @@ cleanup:
         virPidFileReleasePath(pid_file, pid_file_fd);
     VIR_FREE(pid_file);
     VIR_FREE(sock_file);
+    VIR_FREE(state_file);
     VIR_FREE(run_dir);
     return ret;