On Mon, 22 Jul 2024 at 15:24, Jason Wang <
jasowang@redhat.com> wrote:
>
> Hi Cindy
>
> On Fri, Jul 19, 2024 at 1:25 PM Cindy Lu <
lulu@redhat.com> wrote:
> >
> > The crash was reported in MAC OS and NixOS, here is the link for this bug
> >
https://gitlab.com/qemu-project/qemu/-/issues/2334> >
https://gitlab.com/qemu-project/qemu/-/issues/2321> >
> > In this bug, they are using the virtio_input device. The guest notifier was
> > not supported for this device, The function virtio_pci_set_guest_notifiers()
> > was not called, and the vector_irqfd was not initialized.
> >
> > So the fix is adding the check for vector_irqfd in virtio_pci_get_notifier()
> >
> > But The function virtio_pci_get_notifier(),it can also be used in all kinds of device.
> > If the vector_irqfd still didn't initial after the VIRTIO_CONFIG_S_DRIVER_OK is set
> > means this device is not using guest notifier. We can let the check fail here
> >
> > This fix is verified in vyatta,MacOS,NixOS,fedora system.
> >
> > The bt tree for this bug is:
> > Thread 6 "CPU 0/KVM" received signal SIGSEGV, Segmentation fault.
> > [Switching to Thread 0x7c817be006c0 (LWP 1269146)]
> > kvm_virtio_pci_vq_vector_use () at ../qemu-9.0.0/hw/virtio/virtio-pci.c:817
> > 817 if (irqfd->users == 0) {
> > (gdb) thread apply all bt
> > ...
> > Thread 6 (Thread 0x7c817be006c0 (LWP 1269146) "CPU 0/KVM"):
> > 0 kvm_virtio_pci_vq_vector_use () at ../qemu-9.0.0/hw/virtio/virtio-pci.c:817
> > 1 kvm_virtio_pci_vector_use_one () at ../qemu-9.0.0/hw/virtio/virtio-pci.c:893
> > 2 0x00005983657045e2 in memory_region_write_accessor () at ../qemu-9.0.0/system/memory.c:497
> > 3 0x0000598365704ba6 in access_with_adjusted_size () at ../qemu-9.0.0/system/memory.c:573
> > 4 0x0000598365705059 in memory_region_dispatch_write () at ../qemu-9.0.0/system/memory.c:1528
> > 5 0x00005983659b8e1f in flatview_write_continue_step.isra.0 () at ../qemu-9.0.0/system/physmem.c:2713
> > 6 0x000059836570ba7d in flatview_write_continue () at ../qemu-9.0.0/system/physmem.c:2743
> > 7 flatview_write () at ../qemu-9.0.0/system/physmem.c:2774
> > 8 0x000059836570bb76 in address_space_write () at ../qemu-9.0.0/system/physmem.c:2894
> > 9 0x0000598365763afe in address_space_rw () at ../qemu-9.0.0/system/physmem.c:2904
> > 10 kvm_cpu_exec () at ../qemu-9.0.0/accel/kvm/kvm-all.c:2917
> > 11 0x000059836576656e in kvm_vcpu_thread_fn () at ../qemu-9.0.0/accel/kvm/kvm-accel-ops.c:50
> > 12 0x0000598365926ca8 in qemu_thread_start () at ../qemu-9.0.0/util/qemu-thread-posix.c:541
> > 13 0x00007c8185bcd1cf in ??? () at /usr/lib/libc.so.6
> > 14 0x00007c8185c4e504 in clone () at /usr/lib/libc.so.6
> >
> > Fixes: 2ce6cff94d ("virtio-pci: fix use of a released vector")
> > Cc:
qemu-stable@nongnu.org> > Signed-off-by: Cindy Lu <
lulu@redhat.com>
> > ---
> > hw/virtio/virtio-pci.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> > index 592fdaa10f..dc31a37ec0 100644
> > --- a/hw/virtio/virtio-pci.c
> > +++ b/hw/virtio/virtio-pci.c
> > @@ -860,6 +860,9 @@ static int virtio_pci_get_notifier(VirtIOPCIProxy *proxy, int queue_no,
> > VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
> > VirtQueue *vq;
> >
> > + if (!proxy->vector_irqfd && vdev->status & VIRTIO_CONFIG_S_DRIVER_OK)
> > + return -1;
> > +
>
> Did this mean !proxy->vector_irqfd && !(vdev->status &
> VIRTIO_CONFIG_S_DRIVER_OK)) is legal?
>
> Thanks
>
yes, for my test, I didn't meet this kind of environment. However, since this function is used widely in many environments, I cannot cover all devices for testing.
I think we can change the patch back to the first version, which is to check the proxy->vector_irqfd in virtio_pci_set_vector(). This will have a lower risk and make more sense
thanks
cindy