From 8da362fe62766b4eee209cd3ce591ceb62299d13 Mon Sep 17 00:00:00 2001
From: Michal Privoznik <mprivozn@redhat.com>
Date: Tue, 21 Jul 2020 18:12:26 +0200
Subject: [PATCH] qemu_domain_namespace: Repurpose qemuDomainBuildNamespace()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Okay, here is the deal. Currently, the way we build namespace is
very fragile. It is done from pre-exec hook when starting a
domain, after we mass closed all FDs and before we drop
privileges and exec() QEMU. This fact poses some limitations onto
the namespace build code, e.g. it has to make sure not to keep
any FD opened (not even through a library call), because it would
be leaked to QEMU. Also, it has to call only async signal safe
functions. These requirements are hard to meet - in fact as of my
commit v6.2.0-rc1~235 we are leaking a FD into QEMU by calling
libdevmapper functions.

To solve this issue and avoid similar problems in the future, we
should change our paradigm. We already have functions which can
populate domain's namespace with nodes from the daemon context.
If we use them to populate the namespace and keep only the bare
minimum in the pre-exec hook, we've mitigated the risk.

Therefore, the old qemuDomainBuildNamespace() is renamed to
qemuDomainUnshareNamespace() and new qemuDomainBuildNamespace()
function is introduced. So far, the new function is basically a
NOP and domain's namespace is still populated from the pre-exec
hook - next patches will fix it.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
---
 src/qemu/qemu_namespace.c | 23 ++++++++++++++++++++---
 src/qemu/qemu_namespace.h |  8 +++++---
 src/qemu/qemu_process.c   |  6 +++++-
 3 files changed, 30 insertions(+), 7 deletions(-)

diff --git a/src/qemu/qemu_namespace.c b/src/qemu/qemu_namespace.c
index a57c8d219d..4194c60ded 100644
--- a/src/qemu/qemu_namespace.c
+++ b/src/qemu/qemu_namespace.c
@@ -856,10 +856,27 @@ qemuDomainSetupLaunchSecurity(virDomainObjPtr vm,
 }
 
 
+static int
+qemuNamespaceMknodPaths(virDomainObjPtr vm,
+                        const char **paths);
+
+
 int
-qemuDomainBuildNamespace(virQEMUDriverConfigPtr cfg,
-                         virSecurityManagerPtr mgr,
-                         virDomainObjPtr vm)
+qemuDomainBuildNamespace(virDomainObjPtr vm)
+{
+    VIR_AUTOSTRINGLIST paths = NULL;
+
+    if (qemuNamespaceMknodPaths(vm, (const char **) paths) < 0)
+        return -1;
+
+    return 0;
+}
+
+
+int
+qemuDomainUnshareNamespace(virQEMUDriverConfigPtr cfg,
+                           virSecurityManagerPtr mgr,
+                           virDomainObjPtr vm)
 {
     struct qemuDomainCreateDeviceData data;
     const char *devPath = NULL;
diff --git a/src/qemu/qemu_namespace.h b/src/qemu/qemu_namespace.h
index a123f1734b..017e94ade6 100644
--- a/src/qemu/qemu_namespace.h
+++ b/src/qemu/qemu_namespace.h
@@ -37,9 +37,11 @@ int qemuDomainEnableNamespace(virDomainObjPtr vm,
 bool qemuDomainNamespaceEnabled(virDomainObjPtr vm,
                                 qemuDomainNamespace ns);
 
-int qemuDomainBuildNamespace(virQEMUDriverConfigPtr cfg,
-                             virSecurityManagerPtr mgr,
-                             virDomainObjPtr vm);
+int qemuDomainUnshareNamespace(virQEMUDriverConfigPtr cfg,
+                               virSecurityManagerPtr mgr,
+                               virDomainObjPtr vm);
+
+int qemuDomainBuildNamespace(virDomainObjPtr vm);
 
 void qemuDomainDestroyNamespace(virQEMUDriverPtr driver,
                                 virDomainObjPtr vm);
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 629406fd2e..e3060cd054 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3164,7 +3164,7 @@ static int qemuProcessHook(void *data)
     if (qemuSecurityClearSocketLabel(h->driver->securityManager, h->vm->def) < 0)
         goto cleanup;
 
-    if (qemuDomainBuildNamespace(h->cfg, h->driver->securityManager, h->vm) < 0)
+    if (qemuDomainUnshareNamespace(h->cfg, h->driver->securityManager, h->vm) < 0)
         goto cleanup;
 
     if (virDomainNumatuneGetMode(h->vm->def->numa, -1, &mode) == 0) {
@@ -6831,6 +6831,10 @@ qemuProcessLaunch(virConnectPtr conn,
         goto cleanup;
     }
 
+    VIR_DEBUG("Building domain mount namespace (if required)");
+    if (qemuDomainBuildNamespace(vm) < 0)
+        goto cleanup;
+
     VIR_DEBUG("Setting up domain cgroup (if required)");
     if (qemuSetupCgroup(vm, nnicindexes, nicindexes) < 0)
         goto cleanup;