[libvirt] [PATCH] qemu: don't label anything before locking the domain

[Martin Kletzander]

If locking the domain failed, files were already labelled and thus we
restored the previous label on them.  Having disks on NFS means the
domain having the lock already gets permission denial.

This code moves the labelling part into the command hook since it's
still privileged, and also moves the clearing of
VIR_QEMU_PROCESS_STOP_NO_RELABEL from stop_flags right after the
handshare after hook.

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

Signed-off-by: Martin Kletzander <mkletzan at redhat.com>
---
 src/qemu/qemu_process.c | 69 ++++++++++++++++++++++++++++---------------------
 1 file changed, 39 insertions(+), 30 deletions(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 5b598be..bc751b9 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -2700,6 +2700,8 @@ struct qemuProcessHookData {
     virQEMUDriverPtr driver;
     virBitmapPtr nodemask;
     virQEMUDriverConfigPtr cfg;
+    const char *stdin_path;
+    int stdin_fd;
 };

 static int qemuProcessHook(void *data)
@@ -2739,6 +2741,34 @@ static int qemuProcessHook(void *data)
     if (virNumaSetupMemoryPolicy(h->vm->def->numatune, h->nodemask) < 0)
         goto cleanup;

+    /*
+     * Only after we managed to get a domain lock we can label
+     * domain-related objects.
+     */
+    VIR_DEBUG("Setting domain security labels");
+    if (virSecurityManagerSetAllLabel(h->driver->securityManager,
+                                      h->vm->def, h->stdin_path) < 0)
+        goto cleanup;
+
+    if (h->stdin_fd != -1) {
+        /* if there's an fd to migrate from, and it's a pipe, put the
+         * proper security label on it
+         */
+        struct stat stdin_sb;
+
+        VIR_DEBUG("setting security label on pipe used for migration");
+
+        if (fstat(h->stdin_fd, &stdin_sb) < 0) {
+            virReportSystemError(errno,
+                                 _("cannot stat fd %d"), h->stdin_fd);
+            goto cleanup;
+        }
+        if (S_ISFIFO(stdin_sb.st_mode) &&
+            virSecurityManagerSetImageFDLabel(h->driver->securityManager,
+                                              h->vm->def, h->stdin_fd) < 0)
+            goto cleanup;
+    }
+
     ret = 0;

  cleanup:
@@ -3702,6 +3732,8 @@ int qemuProcessStart(virConnectPtr conn,
     hookData.driver = driver;
     /* We don't increase cfg's reference counter here. */
     hookData.cfg = cfg;
+    hookData.stdin_path = stdin_path;
+    hookData.stdin_fd = stdin_fd;

     VIR_DEBUG("Beginning VM startup process");

@@ -4082,6 +4114,12 @@ int qemuProcessStart(virConnectPtr conn,
         goto cleanup;
     }

+    /* Security manager labeled all devices, therefore
+     * if any operation from now on fails and we goto cleanup,
+     * where virSecurityManagerRestoreAllLabel() is called
+     * (hidden under qemuProcessStop) we need to restore labels. */
+    stop_flags &= ~VIR_QEMU_PROCESS_STOP_NO_RELABEL;
+
     VIR_DEBUG("Setting up domain cgroup (if required)");
     if (qemuSetupCgroup(driver, vm, nodemask) < 0)
         goto cleanup;
@@ -4092,36 +4130,7 @@ int qemuProcessStart(virConnectPtr conn,
         qemuProcessInitCpuAffinity(driver, vm, nodemask) < 0)
         goto cleanup;

-    VIR_DEBUG("Setting domain security labels");
-    if (virSecurityManagerSetAllLabel(driver->securityManager,
-                                      vm->def, stdin_path) < 0)
-        goto cleanup;
-
-    /* Security manager labeled all devices, therefore
-     * if any operation from now on fails and we goto cleanup,
-     * where virSecurityManagerRestoreAllLabel() is called
-     * (hidden under qemuProcessStop) we need to restore labels. */
-    stop_flags &= ~VIR_QEMU_PROCESS_STOP_NO_RELABEL;
-
-    if (stdin_fd != -1) {
-        /* if there's an fd to migrate from, and it's a pipe, put the
-         * proper security label on it
-         */
-        struct stat stdin_sb;
-
-        VIR_DEBUG("setting security label on pipe used for migration");
-
-        if (fstat(stdin_fd, &stdin_sb) < 0) {
-            virReportSystemError(errno,
-                                 _("cannot stat fd %d"), stdin_fd);
-            goto cleanup;
-        }
-        if (S_ISFIFO(stdin_sb.st_mode) &&
-            virSecurityManagerSetImageFDLabel(driver->securityManager, vm->def, stdin_fd) < 0)
-            goto cleanup;
-    }
-
-    VIR_DEBUG("Labelling done, completing handshake to child");
+    VIR_DEBUG("Affinity/cgroups set, completing handshake to child");
     if (virCommandHandshakeNotify(cmd) < 0) {
         goto cleanup;
     }
-- 
2.0.0