From 61148074773ba180f17020b0b11c155b154726fd Mon Sep 17 00:00:00 2001
From: Roman Bogorodskiy <bogorodskiy@gmail.com>
Date: Wed, 24 Aug 2016 13:37:27 +0300
Subject: [PATCH] tests: fix segfault in objecteventtest

Test 12 from objecteventtest (createXML add event) segaults on FreeBSD
with bus error.

At some point it calls testNodeDeviceDestroy() from the test driver. And
it fails when it tries to unlock the device in the "out:" label of this
function.

Unlocking fails because the previous step was a call to
virNodeDeviceObjRemove from conf/node_device_conf.c. This function
removes the given device from the device list and cleans up the object,
including destroying of its mutex. However, it does not nullify the pointer
that was given to it.

As a result, we end up in testNodeDeviceDestroy() here:

 out:
    if (obj)
        virNodeDeviceObjUnlock(obj);

And instead of skipping this, we try to do Unlock and fail because of
malformed mutex.

Change virNodeDeviceObjRemove to use double pointer and set pointer to
NULL.
---
 src/conf/node_device_conf.c        | 13 +++++++------
 src/conf/node_device_conf.h        |  2 +-
 src/node_device/node_device_hal.c  |  4 ++--
 src/node_device/node_device_udev.c |  2 +-
 src/test/test_driver.c             |  2 +-
 5 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index a23d8ef99e..719abcd21c 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -207,22 +207,23 @@ virNodeDeviceObjPtr virNodeDeviceAssignDef(virNodeDeviceObjListPtr devs,
 }
 
 void virNodeDeviceObjRemove(virNodeDeviceObjListPtr devs,
-                            virNodeDeviceObjPtr dev)
+                            virNodeDeviceObjPtr *dev)
 {
     size_t i;
 
-    virNodeDeviceObjUnlock(dev);
+    virNodeDeviceObjUnlock(*dev);
 
     for (i = 0; i < devs->count; i++) {
-        virNodeDeviceObjLock(dev);
-        if (devs->objs[i] == dev) {
-            virNodeDeviceObjUnlock(dev);
+        virNodeDeviceObjLock(*dev);
+        if (devs->objs[i] == *dev) {
+            virNodeDeviceObjUnlock(*dev);
             virNodeDeviceObjFree(devs->objs[i]);
+            *dev = NULL;
 
             VIR_DELETE_ELEMENT(devs->objs, i, devs->count);
             break;
         }
-        virNodeDeviceObjUnlock(dev);
+        virNodeDeviceObjUnlock(*dev);
     }
 }
 
diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h
index 8f23a986cd..9f00500024 100644
--- a/src/conf/node_device_conf.h
+++ b/src/conf/node_device_conf.h
@@ -249,7 +249,7 @@ virNodeDeviceObjPtr virNodeDeviceAssignDef(virNodeDeviceObjListPtr devs,
                                            virNodeDeviceDefPtr def);
 
 void virNodeDeviceObjRemove(virNodeDeviceObjListPtr devs,
-                            virNodeDeviceObjPtr dev);
+                            virNodeDeviceObjPtr *dev);
 
 char *virNodeDeviceDefFormat(const virNodeDeviceDef *def);
 
diff --git a/src/node_device/node_device_hal.c b/src/node_device/node_device_hal.c
index ef0cd9c109..fb7bf255db 100644
--- a/src/node_device/node_device_hal.c
+++ b/src/node_device/node_device_hal.c
@@ -530,7 +530,7 @@ dev_refresh(const char *udi)
         /* Simply "rediscover" device -- incrementally handling changes
          * to sub-capabilities (like net.80203) is nasty ... so avoid it.
          */
-        virNodeDeviceObjRemove(&driver->devs, dev);
+        virNodeDeviceObjRemove(&driver->devs, &dev);
     } else {
         VIR_DEBUG("no device named %s", name);
     }
@@ -560,7 +560,7 @@ device_removed(LibHalContext *ctx ATTRIBUTE_UNUSED,
     dev = virNodeDeviceFindByName(&driver->devs, name);
     VIR_DEBUG("%s", name);
     if (dev)
-        virNodeDeviceObjRemove(&driver->devs, dev);
+        virNodeDeviceObjRemove(&driver->devs, &dev);
     else
         VIR_DEBUG("no device named %s", name);
     nodeDeviceUnlock();
diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
index ddf3d8868e..520269fbe9 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -1044,7 +1044,7 @@ static int udevRemoveOneDevice(struct udev_device *device)
 
     VIR_DEBUG("Removing device '%s' with sysfs path '%s'",
               dev->def->name, name);
-    virNodeDeviceObjRemove(&driver->devs, dev);
+    virNodeDeviceObjRemove(&driver->devs, &dev);
 
     ret = 0;
  cleanup:
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index bc1f93d89d..53cfa3cf78 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -5534,7 +5534,7 @@ testNodeDeviceDestroy(virNodeDevicePtr dev)
                                            0);
 
     virNodeDeviceObjLock(obj);
-    virNodeDeviceObjRemove(&driver->devs, obj);
+    virNodeDeviceObjRemove(&driver->devs, &obj);
 
  out:
     if (obj)
-- 
GitLab