Re: [PATCH v1 20/34] qemuDomainBuildNamespace: Populate chardevs from daemon's namespace

[Ján Tomko]

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 <mpriv...@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 <jto...@redhat.com>

Jano

signature.asc
Description: PGP signature