From: Marc Hartmayer <mhartmay@linux.ibm.com>
Date: Tue, 23 Apr 2024 18:08:48 +0000 (+0200)
Subject: node_device_udev: Set @def to NULL
X-Git-Tag: v10.5.0-rc1~71
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=30354f5b1f71f914846245f8a25e70d83753e107;p=thirdparty%2Flibvirt.git

node_device_udev: Set @def to NULL

@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@linux.ibm.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
---

diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
index 85a8d4e206..3a7a02cf15 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 d2d1bc43e3..e93f3f481c 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -7674,8 +7674,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,