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 <mpriv...@redhat.com> > --- > 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@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list