[PATCH v1 15/34] qemu_domain_namespace: Repurpose qemuDomainBuildNamespace()
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(a)redhat.com>
---
src/qemu/qemu_domain_namespace.c | 23 ++++++++++++++++++++---
src/qemu/qemu_domain_namespace.h | 8 +++++---
src/qemu/qemu_process.c | 6 +++++-
3 files changed, 30 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_domain_namespace.c b/src/qemu/qemu_domain_namespace.c
index 18ec86816c..38abed56c8 100644
--- a/src/qemu/qemu_domain_namespace.c
+++ b/src/qemu/qemu_domain_namespace.c
@@ -856,10 +856,27 @@ qemuDomainSetupLaunchSecurity(virDomainObjPtr vm,
}
+static int
+qemuDomainNamespaceMknodPaths(virDomainObjPtr vm,
+ const char **paths);
+
+
int
-qemuDomainBuildNamespace(virQEMUDriverConfigPtr cfg,
- virSecurityManagerPtr mgr,
- virDomainObjPtr vm)
+qemuDomainBuildNamespace(virDomainObjPtr vm)
+{
+ VIR_AUTOSTRINGLIST paths = NULL;
+
+ if (qemuDomainNamespaceMknodPaths(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_domain_namespace.h b/src/qemu/qemu_domain_namespace.h
index 0182ce50a2..70eebf4dc4 100644
--- a/src/qemu/qemu_domain_namespace.h
+++ b/src/qemu/qemu_domain_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 c076dcac3a..bee0fd031b 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;
--
2.26.2