>-----Original Message----- >From: Joao Martins <joao.m.mart...@oracle.com> >Sent: Wednesday, June 21, 2023 7:09 PM >To: Duan, Zhenzhong <zhenzhong.d...@intel.com> >Cc: alex.william...@redhat.com; c...@redhat.com; qemu-devel@nongnu.org; >avih...@nvidia.com; Peng, Chao P <chao.p.p...@intel.com> >Subject: Re: [PATCH v3 2/3] vfio/pci: Fix a segfault in vfio_realize > > > >On 21/06/2023 09:02, Zhenzhong Duan wrote: >> In case irqchip_change_notifier isn't added, removing it triggers segfault. >> >> Signed-off-by: Zhenzhong Duan <zhenzhong.d...@intel.com> >> --- >> hw/vfio/pci.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c index >> c71b0955d81c..82c4cf4f7609 100644 >> --- a/hw/vfio/pci.c >> +++ b/hw/vfio/pci.c >> @@ -3222,7 +3222,9 @@ static void vfio_realize(PCIDevice *pdev, Error >> **errp) >> >> out_deregister: >> pci_device_set_intx_routing_notifier(&vdev->pdev, NULL); >> - kvm_irqchip_remove_change_notifier(&vdev->irqchip_change_notifier); >> + if (vdev->irqchip_change_notifier.notify) { >> + kvm_irqchip_remove_change_notifier(&vdev- >>irqchip_change_notifier); >> + } > >If the first patch ends up being pursued (which I am not quite sure) it should >be folded in the previous patch, as the out_deregister is used starting your >patch 1. Sorry for late response, just back from vacation.
out_deregister isn't only for vfio migration, there are some other jump sites to out_deregister in vfio_realize. Take below code for example: if (vdev->display_xres || vdev->display_yres) { if (vdev->dpy == NULL) { error_setg(errp, "xres and yres properties require display=on"); goto out_deregister; } I can reproduce a segmentation fault when hotplug a vfio device using below cmd: (qemu) device_add vfio-pci,host=81:11.1,id=vfio1,bus=root1,xres=1 Connection closed by foreign host. After fix: (qemu) device_add vfio-pci,host=81:11.1,id=vfio1,bus=root1,xres=1 Error: vfio 0000:81:11.1: xres and yres properties require display=on (qemu) Thanks Zhenzhong