From 6244c173baf80ab6152dc20f475596a232617361 Mon Sep 17 00:00:00 2001
From: Cole Robinson <crobinso@redhat.com>
Date: Tue, 19 May 2009 13:26:14 +0000
Subject: [PATCH] Fix a possible deadlock in HAL nodedev driver.

The cap_lost and prop_modified callbacks could deadlock if an existing
device needed to be refreshed, since dev_create expects the driver to
be unlocked.
---
 ChangeLog             |  5 +++++
 src/node_device_hal.c | 51 ++++++++++++++++++++-----------------------
 2 files changed, 29 insertions(+), 27 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index ac13db5d28..69b184dc99 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+Tue May 19 09:24:54 EDT 2009 Cole Robinson <crobinso@redhat.com>
+
+	* src/node_device_hal.c: Fix a possible deadlock in HAL nodedev
+	driver.
+
 Tue May 19 09:22:43 EDT 2009 Cole Robinson <crobinso@redhat.com>
 
 	* Makefile.am: Fix rpm build: add 'mylibtool' to EXTRADIST.
diff --git a/src/node_device_hal.c b/src/node_device_hal.c
index b214f6030a..e9ea163b98 100644
--- a/src/node_device_hal.c
+++ b/src/node_device_hal.c
@@ -462,6 +462,26 @@ cleanup:
     nodeDeviceUnlock(driverState);
 }
 
+static void dev_refresh(const char *udi)
+{
+    const char *name = hal_name(udi);
+    virNodeDeviceObjPtr dev;
+
+    nodeDeviceLock(driverState);
+    dev = virNodeDeviceFindByName(&driverState->devs, name);
+    if (dev) {
+        /* Simply "rediscover" device -- incrementally handling changes
+         * to sub-capabilities (like net.80203) is nasty ... so avoid it.
+         */
+        virNodeDeviceObjRemove(&driverState->devs, dev);
+    } else
+        DEBUG("no device named %s", name);
+    nodeDeviceUnlock(driverState);
+
+    if (dev) {
+        dev_create(udi);
+    }
+}
 
 static void device_added(LibHalContext *ctx ATTRIBUTE_UNUSED,
                          const char *udi)
@@ -512,20 +532,9 @@ static void device_cap_lost(LibHalContext *ctx ATTRIBUTE_UNUSED,
                             const char *cap)
 {
     const char *name = hal_name(udi);
-    virNodeDeviceObjPtr dev;
-
-    nodeDeviceLock(driverState);
-    dev = virNodeDeviceFindByName(&driverState->devs,name);
     DEBUG("%s %s", cap, name);
-    if (dev) {
-        /* Simply "rediscover" device -- incrementally handling changes
-         * to sub-capabilities (like net.80203) is nasty ... so avoid it.
-         */
-        virNodeDeviceObjRemove(&driverState->devs, dev);
-        dev_create(udi);
-    } else
-        DEBUG("no device named %s", name);
-    nodeDeviceUnlock(driverState);
+
+    dev_refresh(udi);
 }
 
 
@@ -536,21 +545,9 @@ static void device_prop_modified(LibHalContext *ctx ATTRIBUTE_UNUSED,
                                  dbus_bool_t is_added ATTRIBUTE_UNUSED)
 {
     const char *name = hal_name(udi);
-    virNodeDeviceObjPtr dev;
+    DEBUG("%s %s", name, key);
 
-    nodeDeviceLock(driverState);
-    dev = virNodeDeviceFindByName(&driverState->devs,name);
-    DEBUG("%s %s", key, name);
-    if (dev) {
-        /* Simply "rediscover" device -- incrementally handling changes
-         * to properties (which are mapped into caps in very capability-
-         * specific ways) is nasty ... so avoid it.
-         */
-        virNodeDeviceObjRemove(&driverState->devs, dev);
-        dev_create(udi);
-    } else
-        DEBUG("no device named %s", name);
-    nodeDeviceUnlock(driverState);
+    dev_refresh(udi);
 }
 
 
-- 
GitLab