On 5/28/19 6:11 PM, Nikolay Shirokovskiy wrote:
The function updates vm->newDef and frees previous pesistent definition
which can be used by in different thread by some API that unlocks domain
to communicate to qemu. So we have an access to freed memory in that
thread. Let's acquire job condition in case of updating exiting domain.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@xxxxxxxxxxxxx>
---
diff from v1 [1]:
- pay a bit of attention to the problem
Reproducer in the Michal's reply to the [1].
[1] https://www.redhat.com/archives/libvir-list/2019-May/msg00804.html
src/qemu/qemu_driver.c | 37 +++++++++++++++++++++++++++++++++++--
1 file changed, 35 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 42b1ce2..2dd4442 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7620,6 +7620,7 @@ qemuDomainDefineXMLFlags(virConnectPtr conn,
virDomainDefPtr def = NULL;
virDomainDefPtr oldDef = NULL;
virDomainObjPtr vm = NULL;
+ virDomainObjPtr exvm = NULL;
virDomainPtr dom = NULL;
virObjectEventPtr event = NULL;
virQEMUDriverConfigPtr cfg;
@@ -7647,10 +7648,34 @@ qemuDomainDefineXMLFlags(virConnectPtr conn,
if (virDomainDefineXMLFlagsEnsureACL(conn, def) < 0)
goto cleanup;
+ if ((exvm = virDomainObjListFindByUUID(driver->domains, def->uuid))) {
+ if (qemuDomainObjBeginJob(driver, exvm, QEMU_JOB_MODIFY) < 0) {
+ virDomainObjEndAPI(&exvm);
+ goto cleanup;
+ }
+ virObjectUnlock(exvm);
+ }
+
if (!(vm = virDomainObjListAdd(driver->domains, def,
driver->xmlopt,
- 0, &oldDef)))
+ 0, &oldDef))) {
+ if (exvm)
+ virObjectLock(exvm);
goto cleanup;
+ }
+
Almost. There are other places where virDomainObjListAdd() is called and
they require the same threatment. I'm posting a different approach.
Michal
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
