>-----Original Message----- >From: Steven Sistare <steven.sist...@oracle.com> >Subject: Re: [PATCH V4 04/43] vfio/pci: vfio_pci_put_device on failure > >On 6/3/2025 6:40 AM, Duan, Zhenzhong wrote: >>> -----Original Message----- >>> From: Steve Sistare <steven.sist...@oracle.com> >>> Subject: [PATCH V4 04/43] vfio/pci: vfio_pci_put_device on failure >>> >>> If vfio_realize fails after vfio_device_attach, it should call >>> vfio_device_detach during error recovery. If it fails after >>> vfio_device_get_name, it should free vbasedev->name. If it fails >>> after vfio_pci_config_setup, it should free vdev->msix. >>> >>> To fix all, call vfio_pci_put_device(). >>> >>> Signed-off-by: Steve Sistare <steven.sist...@oracle.com> >>> --- >>> hw/vfio/pci.c | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c >>> index a1bfdfe..7d3b9ff 100644 >>> --- a/hw/vfio/pci.c >>> +++ b/hw/vfio/pci.c >>> @@ -3296,6 +3296,7 @@ out_teardown: >>> vfio_bars_exit(vdev); >>> error: >>> error_prepend(errp, VFIO_MSG_PREFIX, vbasedev->name); >>> + vfio_pci_put_device(vdev); >> >> Double free, vfio_pci_put_device() is also called in >> vfio_instance_finalize(). > >If vfio_realize fails with an error, vfio_instance_finalize is not called. >I tested that.
Have you tried with hot plugged device? > >> Early free of vdev->vbasedev.name will also break something, e.g., >trace_vfio_region_finalize(region->vbasedev->name, region->nr); > >All unwinding and calling functions that might use the name is done in the >vfio_realize >failure path, and the very last operation is vfio_pci_put_device, and the last >operation >of that function is freeing the name string. > >- Steve > >>> static void vfio_instance_finalize(Object *obj) >>> -- >>> 1.8.3.1 >>
