Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
On 4/12/24 15:36, Marc Hartmayer wrote:
> @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.
>
> Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
> ---
> 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 ed0cdc0dabe8..8972212e4ddb 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,
--
Mit freundlichen Grüßen/Kind regards
Boris Fiuczynski
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Wolfgang Wendt
Geschäftsführung: David Faller
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org