qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5] virtio-pci: Fix the use of an uninitialized irqfd
@ 2024-07-19  5:25 Cindy Lu
  2024-07-22  7:23 ` Jason Wang
  0 siblings, 1 reply; 4+ messages in thread
From: Cindy Lu @ 2024-07-19  5:25 UTC (permalink / raw)
  To: lulu, mst, jasowang, qemu-devel; +Cc: qemu-stable

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;
+
     if (queue_no == VIRTIO_CONFIG_IRQ_IDX) {
         *n = virtio_config_get_guest_notifier(vdev);
         *vector = vdev->config_vector;
-- 
2.45.0



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v5] virtio-pci: Fix the use of an uninitialized irqfd
  2024-07-19  5:25 [PATCH v5] virtio-pci: Fix the use of an uninitialized irqfd Cindy Lu
@ 2024-07-22  7:23 ` Jason Wang
  2024-07-22  8:41   ` Cindy Lu
  0 siblings, 1 reply; 4+ messages in thread
From: Jason Wang @ 2024-07-22  7:23 UTC (permalink / raw)
  To: Cindy Lu; +Cc: mst, qemu-devel, qemu-stable

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

>      if (queue_no == VIRTIO_CONFIG_IRQ_IDX) {
>          *n = virtio_config_get_guest_notifier(vdev);
>          *vector = vdev->config_vector;
> --
> 2.45.0
>



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v5] virtio-pci: Fix the use of an uninitialized irqfd
  2024-07-22  7:23 ` Jason Wang
@ 2024-07-22  8:41   ` Cindy Lu
  2024-07-23  6:06     ` Jason Wang
  0 siblings, 1 reply; 4+ messages in thread
From: Cindy Lu @ 2024-07-22  8:41 UTC (permalink / raw)
  To: Jason Wang; +Cc: mst, qemu-devel, qemu-stable

[-- Attachment #1: Type: text/plain, Size: 7714 bytes --]

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


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
>
> >      if (queue_no == VIRTIO_CONFIG_IRQ_IDX) {
> >          *n = virtio_config_get_guest_notifier(vdev);
> >          *vector = vdev->config_vector;
> > --
> > 2.45.0
> >
>
>

[-- Attachment #2: Type: text/html, Size: 10142 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v5] virtio-pci: Fix the use of an uninitialized irqfd
  2024-07-22  8:41   ` Cindy Lu
@ 2024-07-23  6:06     ` Jason Wang
  0 siblings, 0 replies; 4+ messages in thread
From: Jason Wang @ 2024-07-23  6:06 UTC (permalink / raw)
  To: Cindy Lu; +Cc: mst, qemu-devel, qemu-stable

On Mon, Jul 22, 2024 at 4:42 PM Cindy Lu <lulu@redhat.com> wrote:
>
>
>
> 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.

This needs to be checked, we can use qtest to poc this condition if you wish.

But I meant we need to explain why there is a check for DRIVER_OK
here. For example, did the current Qemu guarantee that?

> 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

Thanks

>
>
> 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
>>
>> >      if (queue_no == VIRTIO_CONFIG_IRQ_IDX) {
>> >          *n = virtio_config_get_guest_notifier(vdev);
>> >          *vector = vdev->config_vector;
>> > --
>> > 2.45.0
>> >
>>



^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2024-07-23  6:07 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-07-19  5:25 [PATCH v5] virtio-pci: Fix the use of an uninitialized irqfd Cindy Lu
2024-07-22  7:23 ` Jason Wang
2024-07-22  8:41   ` Cindy Lu
2024-07-23  6:06     ` Jason Wang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).