[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 2/3] vfio/pci: Fix a segfault in vfio_realize
From: |
Joao Martins |
Subject: |
Re: [PATCH v3 2/3] vfio/pci: Fix a segfault in vfio_realize |
Date: |
Mon, 26 Jun 2023 10:58:03 +0100 |
On 25/06/2023 07:01, Duan, Zhenzhong wrote:
>> -----Original Message-----
>> From: Joao Martins <joao.m.martins@oracle.com>
>> Sent: Wednesday, June 21, 2023 7:09 PM
>> To: Duan, Zhenzhong <zhenzhong.duan@intel.com>
>> Cc: alex.williamson@redhat.com; clg@redhat.com; qemu-devel@nongnu.org;
>> avihaih@nvidia.com; Peng, Chao P <chao.p.peng@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.duan@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)
>
Makes sense. Let's keep it separate then.
> Thanks
> Zhenzhong
>
>
- [PATCH v3 0/3] VFIO migration related refactor and bug fix, Zhenzhong Duan, 2023/06/21
- [PATCH v3 1/3] vfio/pci: Fix resource leak in vfio_realize, Zhenzhong Duan, 2023/06/21
- Re: [PATCH v3 1/3] vfio/pci: Fix resource leak in vfio_realize, Joao Martins, 2023/06/21
- RE: [PATCH v3 1/3] vfio/pci: Fix resource leak in vfio_realize, Duan, Zhenzhong, 2023/06/25
- RE: [PATCH v3 1/3] vfio/pci: Fix resource leak in vfio_realize, Duan, Zhenzhong, 2023/06/26
- Re: [PATCH v3 1/3] vfio/pci: Fix resource leak in vfio_realize, Joao Martins, 2023/06/26
- RE: [PATCH v3 1/3] vfio/pci: Fix resource leak in vfio_realize, Duan, Zhenzhong, 2023/06/26
- Re: [PATCH v3 1/3] vfio/pci: Fix resource leak in vfio_realize, Joao Martins, 2023/06/27
- RE: [PATCH v3 1/3] vfio/pci: Fix resource leak in vfio_realize, Duan, Zhenzhong, 2023/06/27
[PATCH v3 3/3] vfio/migration: vfio/migration: Refactor and fix print of "Migration disabled", Zhenzhong Duan, 2023/06/21