Re: [libvirt] [PATCH] libxl: fix vm lock overwritten bug

In libxl driver we do virObjectRef in libxlDomainObjBeginJob, If virCondWaitUntil failed, it goes to error, do virObjectUnref, There's a chance that someone undefine the vm at the same time, and refs unref to zero, vm is freed in libxlDomainObjBeginJob. But the vm outside function is not Null, we do virObjectUnlock(vm). That's how we overwrite the vm memory after it's freed. Because the coding amount is much, I fix it partly in libxlDomainCreateWithFlags. If my opinion is right and there're no problems, I'll fix them all later.

Signed-off-by: Wang Yufei <james.wangyufei(a)huawei.com&gt; --- .gnulib | 1 - src/libxl/libxl_domain.c | 6 +----- src/libxl/libxl_domain.h | 2 +- src/libxl/libxl_driver.c | 5 ++--- 4 files changed, 4 insertions(+), 10 deletions(-) delete mode 160000 .gnulib diff --git a/.gnulib b/.gnulib deleted file mode 160000 index 6cc32c6..0000000 --- a/.gnulib +++ /dev/null @@ -1 +0,0 @@ -Subproject commit 6cc32c63e80bc1a30c521b2f07f2b54909b59892 diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index 14a900c..a90ce53 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -123,7 +123,6 @@ libxlDomainObjBeginJob(libxlDriverPrivatePtr driver ATTRIBUTE_UNUSED, return -1; then = now + LIBXL_JOB_WAIT_TIME; - virObjectRef(obj); while (priv->job.active) { VIR_DEBUG("Wait normal job condition for starting job: %s", @@ -157,7 +156,6 @@ libxlDomainObjBeginJob(libxlDriverPrivatePtr driver ATTRIBUTE_UNUSED, virReportSystemError(errno, "%s", _("cannot acquire job mutex")); - virObjectUnref(obj); return -1; } @@ -171,7 +169,7 @@ libxlDomainObjBeginJob(libxlDriverPrivatePtr driver ATTRIBUTE_UNUSED, * non-zero, false if the reference count has dropped to zero * and obj is disposed. */ -bool +void libxlDomainObjEndJob(libxlDriverPrivatePtr driver ATTRIBUTE_UNUSED, virDomainObjPtr obj) { @@ -183,8 +181,6 @@ libxlDomainObjEndJob(libxlDriverPrivatePtr driver ATTRIBUTE_UNUSED, libxlDomainObjResetJob(priv); virCondSignal(&priv->job.cond); - - return virObjectUnref(obj); } int diff --git a/src/libxl/libxl_domain.h b/src/libxl/libxl_domain.h index 1c1eba3..ce28944 100644 --- a/src/libxl/libxl_domain.h +++ b/src/libxl/libxl_domain.h @@ -85,7 +85,7 @@ libxlDomainObjBeginJob(libxlDriverPrivatePtr driver, enum libxlDomainJob job) ATTRIBUTE_RETURN_CHECK; -bool +void libxlDomainObjEndJob(libxlDriverPrivatePtr driver, virDomainObjPtr obj) ATTRIBUTE_RETURN_CHECK; diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index bf97c9c..a46994a 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -294,7 +294,7 @@ libxlDomObjFromDomain(virDomainPtr dom) libxlDriverPrivatePtr driver = dom->conn->privateData; char uuidstr[VIR_UUID_STRING_BUFLEN]; - vm = virDomainObjListFindByUUID(driver->domains, dom->uuid); + vm = virDomainObjListFindByUUIDRef(driver->domains, dom->uuid); if (!vm) { virUUIDFormat(dom->uuid, uuidstr); virReportError(VIR_ERR_NO_DOMAIN, @@ -2691,8 +2691,7 @@ libxlDomainCreateWithFlags(virDomainPtr dom, vm = NULL; cleanup: - if (vm) - virObjectUnlock(vm); + virDomainObjEndAPI(&vm); return ret; }