Re: [libvirt] [PATCH v1 09/21] qemu: Spawn qemu under mount namespace

Prime time. When it comes to spawning qemu process and relabelling all the devices it's going to touch, there's inherent race with other applications in the system (e.g. udev). Instead of trying convincing udev to not touch libvirt managed devices, we can create a separate mount namespace for the qemu, and mount our own /dev there. Of course this puts more work onto us as we have to maintain /dev files on each domain start and device hot(un-)plug. On the other hand, this enhances security also. >From technical POV, on domain startup process the parent (libvirtd) creates: /var/lib/libvirt/qemu/$domain.dev /var/lib/libvirt/qemu/$domain.devpts The child (which is going to be qemu eventually) calls unshare() to create new mount namespace. From now on anything that child does is invisible to the parent. Child then mounts tmpfs on $domain.dev (so that it still sees original /dev from the host) and creates some devices (as explained in one of the previous patches). The devices have to be created exactly as they are in the host (including perms, seclabels, ACLs, ...). After that it moves $domain.dev mount to /dev. What's the $domain.devpts mount there for then you ask? QEMU can create PTYs for some chardevs. And historically we exposed the host ends in our domain XML allowing users to connect to them. Therefore we must preserve devpts mount to be shared with the host's one. To make this patch as small as possible, creating of devices configured for domain in question is implemented in next patches.

Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/qemu/qemu_domain.c | 289 ++++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_domain.h | 10 ++ src/qemu/qemu_process.c | 13 +++ 3 files changed, 312 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 137d4d5..d6a1c29 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -55,6 +55,9 @@ #include <sys/time.h> #include <fcntl.h> +#if defined(HAVE_SYS_MOUNT_H) +# include <sys/mount.h> +#endif #include <libxml/xpathInternals.h> @@ -1627,6 +1630,9 @@ qemuDomainObjPrivateXMLFormat(virBufferPtr buf, virDomainChrTypeToString(priv->monConfig->type)); } + if (priv->containerized) + virBufferAddLit(buf, "<containerized/>\n");

+ qemuDomainObjPrivateXMLFormatVcpus(buf, vm->def); if (priv->qemuCaps) { @@ -1809,6 +1815,8 @@ qemuDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt, goto error; } + priv->containerized = virXPathBoolean("count(./containerized) > 0", ctxt) > 0; + if ((n = virXPathNodeSet("./vcpus/vcpu", ctxt, &nodes)) < 0) goto error; @@ -6653,3 +6661,284 @@ qemuDomainSupportsVideoVga(virDomainVideoDefPtr video, return true; } + + +static int +qemuDomainCreateDevice(const char *device, + const char *path, + bool allow_noent) +{ + char *devicePath = NULL; + struct stat sb; + int ret = -1; + + if (!STRPREFIX(device, "/dev")) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("invalid device: %s"), + device); + goto cleanup; + } + + if (virAsprintf(&devicePath, "%s/%s", + path, device + 4) < 0) + goto cleanup; + + if (stat(device, &sb) < 0) { + if (errno == ENOENT && allow_noent) { + /* Ignore non-existent device. */ + ret = 0; + goto cleanup; + } + + virReportSystemError(errno, _("Unable to stat %s"), device); + goto cleanup; + } + + if (virFileMakeParentPath(devicePath) < 0) { + virReportSystemError(errno, + _("Unable to create %s"), + devicePath); + goto cleanup; + } + + if (mknod(devicePath, sb.st_mode, sb.st_rdev) < 0) { + virReportSystemError(errno, + _("Failed to make device %s"), + devicePath); + goto cleanup; + } + + if (chown(devicePath, sb.st_uid, sb.st_gid) < 0) { + virReportSystemError(errno, + _("Failed to chown device %s"), + devicePath); + goto cleanup; + } + + if (virFileCopyACLs(device, devicePath) < 0 && + errno != ENOTSUP) { + virReportSystemError(errno, + _("Failed to copy ACLs on device %s"), + devicePath); + goto cleanup; + }

+ + ret = 0; + cleanup: + VIR_FREE(devicePath); + return ret; +} + + + +static int +qemuDomainPopulateDevices(virQEMUDriverPtr driver, + virDomainObjPtr vm ATTRIBUTE_UNUSED, + const char *path) +{ + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + const char *const *devices = (const char *const *) cfg->cgroupDeviceACL; + size_t i; + int ret = -1; + + if (!devices) + devices = defaultDeviceACL; + + for (i = 0; devices[i]; i++) { + if (qemuDomainCreateDevice(devices[i], path, true) < 0) + goto cleanup; + } + + ret = 0; + cleanup: + virObjectUnref(cfg); + return ret; +} + + +static int +qemuDomainSetupDev(virQEMUDriverPtr driver, + virDomainObjPtr vm, + const char *path) +{ + char *mount_options = NULL; + char *opts = NULL; + int ret = -1; + + VIR_DEBUG("Setting up /dev/ for domain %s", vm->def->name); + + mount_options = virSecurityManagerGetMountOptions(driver->securityManager, + vm->def); + + if (!mount_options && + VIR_STRDUP(mount_options, "") < 0) + goto cleanup; + + /* + * tmpfs is limited to 64kb, since we only have device nodes in there + * and don't want to DOS the entire OS RAM usage + */ + if (virAsprintf(&opts, + "mode=755,size=65536%s", mount_options) < 0) + goto cleanup; + + if (virFileSetupDev(path, opts) < 0) + goto cleanup; + + if (qemuDomainPopulateDevices(driver, vm, path) < 0) + goto cleanup; + + ret = 0; + cleanup: + VIR_FREE(opts); + VIR_FREE(mount_options); + return ret; +} + + +int +qemuDomainBuildNamespace(virQEMUDriverPtr driver, + virDomainObjPtr vm) +{ + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + qemuDomainObjPrivatePtr priv = vm->privateData; + const unsigned long mount_flags = MS_MOVE; + char *devPath = NULL; + char *devptsPath = NULL; + int ret = -1; + + if (!priv->containerized) { + ret = 0; + goto cleanup; + } + + if (virAsprintf(&devPath, "%s/%s.dev", + cfg->stateDir, vm->def->name) < 0 || + virAsprintf(&devptsPath, "%s/%s.devpts", + cfg->stateDir, vm->def->name) < 0) + goto cleanup; + + if (qemuDomainSetupDev(driver, vm, devPath) < 0) + goto cleanup; + + /* Save /dev/pts mount point because /dev/pts/NNN is exposed in our live + * XML and other applications are supposed to be able to use it. */ + if (mount("/dev/pts", devptsPath, NULL, mount_flags, NULL) < 0) { + virReportSystemError(errno, "%s", + _("Unable to move /dev/pts mount")); + goto cleanup; + } + + if (mount(devPath, "/dev", NULL, mount_flags, NULL) < 0) { + virReportSystemError(errno, + _("Failed to mount %s on /dev"), + devPath); + goto cleanup; + } + + if (virFileMakePath("/dev/pts") < 0) { + virReportSystemError(errno, "%s", + _("Cannot create /dev/pts")); + goto cleanup; + } + + if (mount(devptsPath, "/dev/pts", NULL, mount_flags, NULL) < 0) { + virReportSystemError(errno, + _("Failed to mount %s on /dev/pts"), + devptsPath); + goto cleanup; + } + + if (virFileBindMountDevice("/dev/pts/ptmx", "/dev/ptmx") < 0) + goto cleanup; + + VIR_DEBUG("blaaah: %d", system("find /dev/ -ls > /tmp/blaaah")); + VIR_DEBUG("blaaah: %d", system("echo >> /tmp/blaaah")); + VIR_DEBUG("blaaah: %d", system("mount >> /tmp/blaaah"));

+ ret = 0; + cleanup: + virObjectUnref(cfg); + VIR_FREE(devPath); + return ret; +} + + +int +qemuDomainCreateNamespace(virQEMUDriverPtr driver, + virDomainObjPtr vm) +{ + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + qemuDomainObjPrivatePtr priv = vm->privateData; + int ret = -1; + char *path = NULL; + +#if !defined(__linux__) + /* Namespaces are Linux specific. On other platforms just + * carry on with the old behaviour. */ + return 0; +#endif

+ + if (!virQEMUDriverIsPrivileged(driver)) { + ret = 0; + goto cleanup; + } + + if (virAsprintf(&path, "%s/%s.dev", + cfg->stateDir, vm->def->name) < 0) + goto cleanup; + + if (virFileMakePath(path) < 0) { + virReportSystemError(errno, + _("Failed to create %s"), + path); + goto cleanup; + } + + VIR_FREE(path); + if (virAsprintf(&path, "%s/%s.devpts", + cfg->stateDir, vm->def->name) < 0) + goto cleanup; + + if (virFileMakePath(path) < 0) { + virReportSystemError(errno, + _("Failed to create %s"), + path); + goto cleanup; + } + + priv->containerized = true; + ret = 0; + cleanup: + VIR_FREE(path); + virObjectUnref(cfg); + return ret; +} + + +void +qemuDomainDeleteNamespace(virQEMUDriverPtr driver, + virDomainObjPtr vm) +{ + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + qemuDomainObjPrivatePtr priv = vm->privateData; + char *path; + + if (!priv->containerized) + return; + + if (virAsprintf(&path, "%s/%s.dev", + cfg->stateDir, vm->def->name) < 0) + goto cleanup; + + virFileDeleteTree(path); + + VIR_FREE(path); + if (virAsprintf(&path, "%s/%s.devpts", + cfg->stateDir, vm->def->name) < 0) + goto cleanup; + + virFileDeleteTree(path); + cleanup: + virObjectUnref(cfg); + VIR_FREE(path); +}

diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 7650ff3..9c34476 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -170,6 +170,8 @@ typedef qemuDomainObjPrivate *qemuDomainObjPrivatePtr; struct _qemuDomainObjPrivate { struct qemuDomainJobObj job; + bool containerized; + qemuMonitorPtr mon; virDomainChrSourceDefPtr monConfig; bool monJSON; @@ -785,4 +787,12 @@ int qemuDomainCheckMonitor(virQEMUDriverPtr driver, bool qemuDomainSupportsVideoVga(virDomainVideoDefPtr video, virQEMUCapsPtr qemuCaps); +int qemuDomainBuildNamespace(virQEMUDriverPtr driver, + virDomainObjPtr vm); + +int qemuDomainCreateNamespace(virQEMUDriverPtr driver, + virDomainObjPtr vm); + +void qemuDomainDeleteNamespace(virQEMUDriverPtr driver, + virDomainObjPtr vm); #endif /* __QEMU_DOMAIN_H__ */ diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 93a38e1..4f64dd1 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2661,6 +2661,12 @@ static int qemuProcessHook(void *data) if (virSecurityManagerClearSocketLabel(h->driver->securityManager, h->vm->def) < 0) goto cleanup; + if (virProcessSetupPrivateMountNS() < 0) + goto cleanup; + + if (qemuDomainBuildNamespace(h->driver, h->vm) < 0) + goto cleanup; + if (virDomainNumatuneGetMode(h->vm->def->numa, -1, &mode) == 0) { if (mode == VIR_DOMAIN_NUMATUNE_MEM_STRICT && h->cfg->cgroupControllers & (1 << VIR_CGROUP_CONTROLLER_CPUSET) && @@ -5434,6 +5440,11 @@ qemuProcessLaunch(virConnectPtr conn, qemuDomainLogContextMarkPosition(logCtxt); + VIR_DEBUG("Building mount namespace"); + + if (qemuDomainCreateNamespace(driver, vm) < 0) + goto cleanup; + VIR_DEBUG("Clear emulator capabilities: %d", cfg->clearEmulatorCapabilities); if (cfg->clearEmulatorCapabilities) @@ -6210,6 +6221,8 @@ void qemuProcessStop(virQEMUDriverPtr driver, } } + qemuDomainDeleteNamespace(driver, vm); + vm->taint = 0; vm->pid = -1; virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, reason);