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.

diff --git a/ChangeLog b/ChangeLog
index ac13db5d281f1479519ae9a12b73f0724c2bf62d..69b184dc995b8d71fca7133fd4fc1ae3b490b5ed 100644 (file)
--- 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 b214f6030a1346bf5c7abcdcea9e7636ebd2fd44..e9ea163b98391543faaf2818a70d178b8256a4a1 100644 (file)
--- 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);
 }