[PATCH v1 02/20] node_device_udev: Set @def to NULL

[Marc Hartmayer <mhartmay@xxxxxxxxxxxxx>]

@def is owned by @obj after adding it the node device object list. As soon as
the @obj lock has been released, another thread could free @obj and therefore
@def. If now someone accesses @def this would lead to a heap-use-after-free and
therefore most likely to a segmentation fault, therefore set @def to NULL after
the ownership has moved.

While at it, add comments to other code places why @def is set to NULL.

Reviewed-by: Boris Fiuczynski <fiuczy@xxxxxxxxxxxxx>
Signed-off-by: Marc Hartmayer <mhartmay@xxxxxxxxxxxxx>
---
 src/node_device/node_device_udev.c | 4 ++++
 src/test/test_driver.c             | 3 ++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
index 4730a5b986ca..6613528d0e37 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -1588,6 +1588,8 @@ udevAddOneDevice(struct udev_device *device)
      * and the current definition will take its place. */
     if (!(obj = virNodeDeviceObjListAssignDef(driver->devs, def)))
         goto cleanup;
+    /* @def is now owned by @obj */
+    def = NULL;
     virNodeDeviceObjSetPersistent(obj, persistent);
     virNodeDeviceObjSetAutostart(obj, autostart);
     objdef = virNodeDeviceObjGetDef(obj);
@@ -1983,6 +1985,8 @@ udevSetupSystemDev(void)
     if (!(obj = virNodeDeviceObjListAssignDef(driver->devs, def)))
         goto cleanup;
 
+    /* @def is now owned by @obj */
+    def = NULL;
     virNodeDeviceObjSetActive(obj, true);
     virNodeDeviceObjSetAutostart(obj, true);
     virNodeDeviceObjSetPersistent(obj, true);
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 41828f86b656..81b1ba4294bd 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -7656,8 +7656,9 @@ testNodeDeviceMockCreateVport(testDriver *driver,
 
     if (!(obj = virNodeDeviceObjListAssignDef(driver->devs, def)))
         goto cleanup;
-    virNodeDeviceObjSetSkipUpdateCaps(obj, true);
+    /* @def is now owned by @obj */
     def = NULL;
+    virNodeDeviceObjSetSkipUpdateCaps(obj, true);
     objdef = virNodeDeviceObjGetDef(obj);
 
     event = virNodeDeviceEventLifecycleNew(objdef->name,
-- 
2.34.1
_______________________________________________
Devel mailing list -- devel@xxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx