On Tue, Apr 09, 2013 at 12:22:38PM +0200, Peter Krempa wrote:
On 04/09/13 11:08, Daniel P. Berrange wrote:
On Mon, Apr 08, 2013 at 09:15:28PM -0600, Eric Blake wrote:
On 02/11/2013 09:46 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
When removing a VM from the virDomainObjListPtr, we must not be holding the VM lock while acquiring the list lock. Re-order code to ensure that we can release the VM lock early. --- src/conf/domain_conf.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 5e16ddf..d92e54a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2115,11 +2115,10 @@ void virDomainObjListRemove(virDomainObjListPtr doms, { char uuidstr[VIR_UUID_STRING_BUFLEN];
- virObjectLock(doms); virUUIDFormat(dom->def->uuid, uuidstr); - virObjectUnlock(dom);
+ virObjectLock(doms);
This patch seems to be implicated in Peter's latest proof of a use-after-free data race: https://www.redhat.com/archives/libvir-list/2013-April/msg00674.html
The caller of this method should own a reference on virDomainObjPtr. This method will result in that reference being released, as the dom is removed from the list.
If any other thread experiances a use-after-free, then that thread must have been missing a reference.
Actually, by default no reference updating is being done when the domain object is taken from the list. The only place where references are updated on domain objects is monitor code, right before the locks are dropped.
I'm working on patches that adds reference tracking when the domain object pointer is acquired from the list to avoid this kind of issue.
Well that's a big change in the locking model. I agree it would actually make sense to do that, but I think we need a more targetted fix for this particular problem first which is practical to backport to stable releases. 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 :|