[libvirt] [PATCH] qemu: Don't increment jobs counter before DomainObj ref count is incremented
Currently, if domain is being destroyed, it's private data can be freed. If there's however another thread waiting to start a job, it may lead to a NULL dereference and SIGSEGV. Check if reference counter on domain object was successfully incremented. Reported-By: Scott Sullivan <ssullivan@liquidweb.com> --- Reported here: https://www.redhat.com/archives/libvir-list/2012-December/msg00931.html src/qemu/qemu_domain.c | 11 +++++++---- 1 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 8d8cf02..5cc5bf7 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -764,18 +764,21 @@ qemuDomainObjBeginJobInternal(virQEMUDriverPtr driver, enum qemuDomainJob job, enum qemuDomainAsyncJob asyncJob) { - qemuDomainObjPrivatePtr priv = obj->privateData; + qemuDomainObjPrivatePtr priv; unsigned long long now; unsigned long long then; bool nested = job == QEMU_JOB_ASYNC_NESTED; - priv->jobs_queued++; - if (virTimeMillisNow(&now) < 0) return -1; then = now + QEMU_JOB_WAIT_TIME; - virObjectRef(obj); + if (!virObjectRef(obj)) + return -1; + + priv = obj->privateData; + priv->jobs_queued++; + if (driver_locked) qemuDriverUnlock(driver); -- 1.7.8.6
On 12/14/2012 09:11 AM, Michal Privoznik wrote:
Currently, if domain is being destroyed, it's private data can be freed. If there's however another thread waiting to start a job, it may lead to a NULL dereference and SIGSEGV. Check if reference counter on domain object was successfully incremented.
Reported-By: Scott Sullivan <ssullivan@liquidweb.com> ---
Reported here:
https://www.redhat.com/archives/libvir-list/2012-December/msg00931.html
src/qemu/qemu_domain.c | 11 +++++++---- 1 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 8d8cf02..5cc5bf7 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -764,18 +764,21 @@ qemuDomainObjBeginJobInternal(virQEMUDriverPtr driver, enum qemuDomainJob job, enum qemuDomainAsyncJob asyncJob) { - qemuDomainObjPrivatePtr priv = obj->privateData; + qemuDomainObjPrivatePtr priv; unsigned long long now; unsigned long long then; bool nested = job == QEMU_JOB_ASYNC_NESTED;
- priv->jobs_queued++; - if (virTimeMillisNow(&now) < 0) return -1; then = now + QEMU_JOB_WAIT_TIME;
- virObjectRef(obj); + if (!virObjectRef(obj)) + return -1;
I'm not sure if this means that we have a bug somewhere else in passing NULL obj in the first place.
+ + priv = obj->privateData; + priv->jobs_queued++; +
This patch looks sane if passing a NULL obj is reasonable, but I'd rather get danpb's opinion before pushing. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (2)
-
Eric Blake -
Michal Privoznik