Re: Potential deadlock in libvirt lxc driver

[Thomas Hunger <tehunger@xxxxxxxxx>]

Hi,

last email hopefully. My last patch was broken because it double
locked vm. I discovered that virDomainFindBy* locks vm before
returning it. The new patch does not double-lock.

best,
Tom

From f2fd4cb72c8f2e01567320a643fe2d665308119e Mon Sep 17 00:00:00 2001
From: Thomas Hunger <tehunger@xxxxxxxxx>
Date: Thu, 15 Mar 2012 00:08:00 +0000
Subject: [PATCH] Use virDomainFindbyID and pass id instead of a pointer to lxcMontitorEvent.

This fixes a race condition where lxcDomainDestroyFlags would acquire the
driver lock. It would then kill processes in the cgroup and trigger
VIR_EVENT_HANDLE_HANGUP, which in turn triggers lxcMonitorEvent.

lxcMonitorEvent tries to acquire the driver lock itself, so it would block
until destroy has invalidated the data "vm" points to.

By using virDomainFindbyID lxcMonitorEvent avoids using an invalid vm
pointer.
---
 src/lxc/lxc_driver.c |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 3af8084..c55e164 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -1523,12 +1523,17 @@ static void lxcMonitorEvent(int watch,
                             void *data)
 {
     lxc_driver_t *driver = lxc_driver;
-    virDomainObjPtr vm = data;
+    int id = (int)data;
+    virDomainObjPtr vm = NULL;
     virDomainEventPtr event = NULL;
     lxcDomainObjPrivatePtr priv;
 
     lxcDriverLock(driver);
-    virDomainObjLock(vm);
+    vm = virDomainFindByID(&driver->domains, id);
+    if (vm == NULL) {
+        lxcDriverUnlock(driver);
+        goto cleanup;
+    }
     lxcDriverUnlock(driver);
 
     priv = vm->privateData;
@@ -1950,7 +1955,7 @@ static int lxcVmStart(virConnectPtr conn,
              priv->monitor,
              VIR_EVENT_HANDLE_ERROR | VIR_EVENT_HANDLE_HANGUP,
              lxcMonitorEvent,
-             vm, NULL)) < 0) {
+             (void*)vm->def->id, NULL)) < 0) {
         goto error;
     }
 
@@ -2513,7 +2518,7 @@ lxcReconnectVM(void *payload, const void *name ATTRIBUTE_UNUSED, void *opaque)
                  priv->monitor,
                  VIR_EVENT_HANDLE_ERROR | VIR_EVENT_HANDLE_HANGUP,
                  lxcMonitorEvent,
-                 vm, NULL)) < 0)
+                 (void*)vm->def->id, NULL)) < 0)
             goto error;
 
         if (virSecurityManagerReserveLabel(driver->securityManager,
-- 
1.7.2.5

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list