[libvirt] [PATCHv2 1/2] qemu: Add missing lock of virDomainObj before calling virDomainUnref
Daniel P. Berrange
berrange at redhat.com
Fri Mar 4 11:47:22 UTC 2011
On Thu, Mar 03, 2011 at 04:10:07PM -0500, Laine Stump wrote:
> (Eric pointed out in IRC that I should be acquiring the domainobj lock
> prior to modifying obj->privateData->mon)
>
> This was found while researching the root cause of:
>
> https://bugzilla.redhat.com/show_bug.cgi?id=670848
>
> virDomainUnref should only be called with the lock held for the
> virDomainObj in question. However, when a transient qemu domain gets
> EOF on its monitor socket, it queues an event which frees the monitor,
> which unref's the virDomainObj without first locking it. If another
> thread has already locked the virDomainObj, the modification of the
> refcount could potentially be corrupted. In an extreme case, it could
> also be potentially unlocked by virDomainObjFree, thus left open to
> modification by anyone else who would have otherwise waited for the
> lock (not to mention the fact that they would be accessing freed
> data!).
>
> The solution is to have qemuMonitorFree lock the domain object right
> before unrefing it. Since the caller to qemuMonitorFree doesn't expect
> this lock to be held, if the refcount doesn't go all the way to 0,
> qemuMonitorFree must unlock it after the unref.
> ---
> src/qemu/qemu_process.c | 8 ++++++--
> 1 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index c419c75..9084725 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -593,10 +593,14 @@ no_memory:
> static void qemuProcessHandleMonitorDestroy(qemuMonitorPtr mon,
> virDomainObjPtr vm)
> {
> - qemuDomainObjPrivatePtr priv = vm->privateData;
> + qemuDomainObjPrivatePtr priv;
> +
> + virDomainObjLock(vm);
> + priv = vm->privateData;
> if (priv->mon == mon)
> priv->mon = NULL;
> - virDomainObjUnref(vm);
> + if (virDomainObjUnref(vm) > 0)
> + virDomainObjUnlock(vm);
> }
>
> static qemuMonitorCallbacks monitorCallbacks = {
ACK
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
More information about the libvir-list
mailing list