On a Wednesday in 2020, Michal Privoznik wrote:
>As mentioned in one of previous commits, populating domain's
>namespace from pre-exec() hook is dangerous. This commit moves
>population of the namespace with domain chardevs into daemon's
>namespace.
>
>Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>---
> src/qemu/qemu_domain_namespace.c | 27 +++++++++++----------------
> 1 file changed, 11 insertions(+), 16 deletions(-)
>
>diff --git a/src/qemu/qemu_domain_namespace.c b/src/qemu/qemu_domain_namespace.c
>index bafb08fac8..36d22b42f2 100644
>--- a/src/qemu/qemu_domain_namespace.c
>+++ b/src/qemu/qemu_domain_namespace.c
>@@ -629,7 +629,7 @@ qemuDomainSetupChardev(virDomainDefPtr def G_GNUC_UNUSED,
> virDomainChrDefPtr dev,
> void *opaque)
> {
>- const struct qemuDomainCreateDeviceData *data = opaque;
>+ char ***paths = opaque;
> const char *path = NULL;
>
> if (!(path = virDomainChrSourceDefGetPath(dev->source)))
>@@ -640,20 +640,20 @@ qemuDomainSetupChardev(virDomainDefPtr def G_GNUC_UNUSED,
> dev->source->data.nix.listen)
> return 0;
>
>- return qemuDomainCreateDevice(path, data, true);
>+ return virStringListAdd(paths, path);
> }
>
>
> static int
> qemuDomainSetupAllChardevs(virDomainObjPtr vm,
>- const struct qemuDomainCreateDeviceData *data)
>+ char ***paths)
> {
> VIR_DEBUG("Setting up chardevs");
>
> if (virDomainChrDefForeach(vm->def,
> true,
> qemuDomainSetupChardev,
>- (void *)data) < 0)
>+ paths) < 0)
> return -1;
>
> VIR_DEBUG("Setup all chardevs");
>@@ -877,6 +877,9 @@ qemuDomainBuildNamespace(virQEMUDriverConfigPtr cfg,
> if (qemuDomainSetupAllMemories(vm, &paths) < 0)
> return -1;
>
>+ if (qemuDomainSetupAllChardevs(vm, &paths) < 0)
>+ return -1;
>+
> if (qemuDomainNamespaceMknodPaths(vm, (const char **) paths) < 0)
> return -1;
>
>@@ -928,9 +931,6 @@ qemuDomainUnshareNamespace(virQEMUDriverConfigPtr cfg,
> if (qemuDomainSetupDev(mgr, vm, devPath) < 0)
> goto cleanup;
>
>- if (qemuDomainSetupAllChardevs(vm, &data) < 0)
>- goto cleanup;
>-
> if (qemuDomainSetupAllTPMs(vm, &data) < 0)
> goto cleanup;
>
>@@ -1779,20 +1779,15 @@ int
> qemuDomainNamespaceSetupChardev(virDomainObjPtr vm,
> virDomainChrDefPtr chr)
> {
>- const char *path;
>+ VIR_AUTOSTRINGLIST paths = NULL;
>
> if (!qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT))
> return 0;
>
>- if (!(path = virDomainChrSourceDefGetPath(chr->source)))
>- return 0;
>+ if (qemuDomainSetupChardev(vm->def, chr, &paths) < 0)
>+ return -1;
>
>- /* Socket created by qemu. It doesn't exist upfront. */
>- if (chr->source->type == VIR_DOMAIN_CHR_TYPE_UNIX &&
>- chr->source->data.nix.listen)
>- return 0;
>-
Hmm, this is not necessarily true. qemuBuildChrChardevStr opens listen
type sockets if QEMU supports FD passing for them.
>- if (qemuDomainNamespaceMknodPath(vm, path) < 0)
>+ if (qemuDomainNamespaceMknodPaths(vm, (const char **) paths) < 0)
> return -1;
>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Jano