[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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 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.


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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]