Re: [libvirt] [PATCH] Release VM lock before acquiring virDomainObjListPtr lock

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(a)redhat.com&gt; >>> >>>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.