On Fri, 2017-04-28 at 13:22 +0200, Michal Privoznik wrote:
> Even though there are several checks before calling this function
> and for some scenarios we don't call it at all (e.g. on disk hot
> unplug), it may be possible to sneak in some weird files (e.g. if
> domain would have RNG with /dev/shm/some_file as its backend). No
> matter how improbable, we shouldn't unlink it as we would be
> unlinking a file from the host which we haven't created in the
> first place.
>
> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
> ---
> src/qemu/qemu_domain.c | 86 ++++++++++++++++++++++++++++++++++++++++++++------
> 1 file changed, 76 insertions(+), 10 deletions(-)
>
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 60f8f01..c393d5e 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -8395,14 +8395,32 @@ qemuDomainDetachDeviceUnlinkHelper(pid_t pid ATTRIBUTE_UNUSED,
> static int
> qemuDomainDetachDeviceUnlink(virQEMUDriverPtr driver ATTRIBUTE_UNUSED,
> virDomainObjPtr vm,
> - const char *file)
> + const char *file,
> + char * const *devMountsPath,
> + size_t ndevMountsPath)
> {
> - if (virProcessRunInMountNamespace(vm->pid,
> - qemuDomainDetachDeviceUnlinkHelper,
> - (void *)file) < 0)
> - return -1;
> + int ret = -1;
> + size_t i;
>
> - return 0;
> + if (STRPREFIX(file, DEVPREFIX)) {
> + for (i = 0; i < ndevMountsPath; i++) {
> + if (STREQ(devMountsPath[i], "/dev"))
> + continue;
> + if (STRPREFIX(file, devMountsPath[i]))
> + break;
> + }
> +
> + if (i == ndevMountsPath) {
> + if (virProcessRunInMountNamespace(vm->pid,
> + qemuDomainDetachDeviceUnlinkHelper,
> + (void *)file) < 0)
> + goto cleanup;
> + }
> + }
> +
> + ret = 0;
> + cleanup:
> + return ret;
> }
>
>
> @@ -8521,6 +8539,9 @@ qemuDomainNamespaceTeardownHostdev(virQEMUDriverPtr driver,
> virDomainObjPtr vm,
> virDomainHostdevDefPtr hostdev)
> {
> + virQEMUDriverConfigPtr cfg = NULL;
> + char **devMountsPath = NULL;
> + size_t ndevMountsPath = 0;
> int ret = -1;
> char **path = NULL;
> size_t i, npaths = 0;
> @@ -8532,8 +8553,15 @@ qemuDomainNamespaceTeardownHostdev(virQEMUDriverPtr driver,
> &npaths, &path, NULL) < 0)
> goto cleanup;
>
> + cfg = virQEMUDriverGetConfig(driver);
> + if (qemuDomainGetPreservedMounts(cfg, vm,
> + &devMountsPath, NULL,
> + &ndevMountsPath) < 0)
> + goto cleanup;
> +
> for (i = 0; i < npaths; i++) {
> - if (qemuDomainDetachDeviceUnlink(driver, vm, path[i]) < 0)
> + if (qemuDomainDetachDeviceUnlink(driver, vm, path[i],
> + devMountsPath, ndevMountsPath) < 0)
> goto cleanup;
> }
>
> @@ -8542,6 +8570,8 @@ qemuDomainNamespaceTeardownHostdev(virQEMUDriverPtr driver,
> for (i = 0; i < npaths; i++)
> VIR_FREE(path[i]);
> VIR_FREE(path);
> + virStringListFreeCount(devMountsPath, ndevMountsPath);
> + virObjectUnref(cfg);
> return ret;
> }
>
> @@ -8584,6 +8614,9 @@ qemuDomainNamespaceTeardownMemory(virQEMUDriverPtr driver,
> virDomainObjPtr vm,
> virDomainMemoryDefPtr mem)
> {
> + virQEMUDriverConfigPtr cfg = NULL;
> + char **devMountsPath = NULL;
> + size_t ndevMountsPath = 0;
> int ret = -1;
>
> if (mem->model != VIR_DOMAIN_MEMORY_MODEL_NVDIMM)
> @@ -8592,10 +8625,19 @@ qemuDomainNamespaceTeardownMemory(virQEMUDriverPtr driver,
> if (!qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT))
> return 0;
>
> - if (qemuDomainDetachDeviceUnlink(driver, vm, mem->nvdimmPath) < 0)
> + cfg = virQEMUDriverGetConfig(driver);
> + if (qemuDomainGetPreservedMounts(cfg, vm,
> + &devMountsPath, NULL,
> + &ndevMountsPath) < 0)
> + goto cleanup;
> +
> + if (qemuDomainDetachDeviceUnlink(driver, vm, mem->nvdimmPath,
> + devMountsPath, ndevMountsPath) < 0)
> goto cleanup;
> ret = 0;
> cleanup:
> + virStringListFreeCount(devMountsPath, ndevMountsPath);
> + virObjectUnref(cfg);
> return ret;
> }
>
> @@ -8643,6 +8685,9 @@ qemuDomainNamespaceTeardownChardev(virQEMUDriverPtr driver,
> virDomainObjPtr vm,
> virDomainChrDefPtr chr)
> {
> + virQEMUDriverConfigPtr cfg = NULL;
> + char **devMountsPath = NULL;
> + size_t ndevMountsPath = 0;
> int ret = -1;
> const char *path = NULL;
>
> @@ -8654,11 +8699,20 @@ qemuDomainNamespaceTeardownChardev(virQEMUDriverPtr driver,
>
> path = chr->source->data.file.path;
>
> - if (qemuDomainDetachDeviceUnlink(driver, vm, path) < 0)
> + cfg = virQEMUDriverGetConfig(driver);
> + if (qemuDomainGetPreservedMounts(cfg, vm,
> + &devMountsPath, NULL,
> + &ndevMountsPath) < 0)
> + goto cleanup;
> +
> + if (qemuDomainDetachDeviceUnlink(driver, vm, path,
> + devMountsPath, ndevMountsPath) < 0)
> goto cleanup;
>
> ret = 0;
> cleanup:
> + virStringListFreeCount(devMountsPath, ndevMountsPath);
> + virObjectUnref(cfg);
> return ret;
> }
>
> @@ -8712,6 +8766,9 @@ qemuDomainNamespaceTeardownRNG(virQEMUDriverPtr driver,
> virDomainObjPtr vm,
> virDomainRNGDefPtr rng)
> {
> + virQEMUDriverConfigPtr cfg = NULL;
> + char **devMountsPath = NULL;
> + size_t ndevMountsPath = 0;
> int ret = -1;
> const char *path = NULL;
>
> @@ -8729,11 +8786,20 @@ qemuDomainNamespaceTeardownRNG(virQEMUDriverPtr driver,
> goto cleanup;
> }
>
> - if (qemuDomainDetachDeviceUnlink(driver, vm, path) < 0)
> + cfg = virQEMUDriverGetConfig(driver);
> + if (qemuDomainGetPreservedMounts(cfg, vm,
> + &devMountsPath, NULL,
> + &ndevMountsPath) < 0)
> + goto cleanup;
> +
> + if (qemuDomainDetachDeviceUnlink(driver, vm, path,
> + devMountsPath, ndevMountsPath) < 0)
> goto cleanup;
>
> ret = 0;
> cleanup:
> + virStringListFreeCount(devMountsPath, ndevMountsPath);
> + virObjectUnref(cfg);
> return ret;
> }
>
ACK
--
Cedric
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
