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@xxxxxxxxxx>
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.
I'm trying to understand what the behavior was before this patch went in.
Well this was just fixing a deadlock introduced in a previous patch.
You need to look further back than just this patch. Originally the
global QEMU driver lock would be held preventing any kind of concurrent
execution.
Regards,
Daniel
Peter
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list