* [PATCH v3] virtio-pci: Fix the use of an uninitialized irqfd
@ 2024-06-26 2:44 Cindy Lu
2024-06-26 7:44 ` Michael S. Tsirkin
0 siblings, 1 reply; 5+ messages in thread
From: Cindy Lu @ 2024-06-26 2:44 UTC (permalink / raw)
To: lulu, mst, jasowang, qemu-devel, philmd; +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
The root cause is the function virtio_pci_set_guest_notifiers() was not called
in the virtio_input device.So the vector_irqfd was not initialized
So the fix is add the check for vector_irqfd.
Change in V3
1) Move the vector_irqfd check to virtio_pci_get_notifier().
The function virtio_pci_get_notifier() can also be used while vdev->status
is not VIRTIO_CONFIG_S_DRIVER_OK. In that case, the vector_irqfd could be NULL,
so also add the status check here.
2) Add the return value check for kvm_virtio_pci_vector_use_one().
Since the return value of function virtio_pci_set_vector() is void,
just add the error message 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 | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index b1d02f4b3d..87307b9061 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -860,6 +860,11 @@ 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 == NULL) &&
+ (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;
@@ -1452,7 +1457,9 @@ static void virtio_pci_set_vector(VirtIODevice *vdev,
}
/* If the new vector changed need to set it up. */
if (kvm_irqfd && new_vector != VIRTIO_NO_VECTOR) {
- kvm_virtio_pci_vector_use_one(proxy, queue_no);
+ if (kvm_virtio_pci_vector_use_one(proxy, queue_no)) {
+ virtio_error(vdev, "fail to set the vector %d", new_vector);
+ }
}
}
--
2.45.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v3] virtio-pci: Fix the use of an uninitialized irqfd
2024-06-26 2:44 [PATCH v3] virtio-pci: Fix the use of an uninitialized irqfd Cindy Lu
@ 2024-06-26 7:44 ` Michael S. Tsirkin
2024-06-27 8:40 ` Cindy Lu
0 siblings, 1 reply; 5+ messages in thread
From: Michael S. Tsirkin @ 2024-06-26 7:44 UTC (permalink / raw)
To: Cindy Lu; +Cc: jasowang, qemu-devel, philmd, qemu-stable
On Wed, Jun 26, 2024 at 10:44:31AM +0800, Cindy Lu 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
>
> The root cause is the function virtio_pci_set_guest_notifiers() was not called
> in the virtio_input device.So the vector_irqfd was not initialized
>
> So the fix is add the check for vector_irqfd.
>
> Change in V3
Changelog belongs after ---
> 1) Move the vector_irqfd check to virtio_pci_get_notifier().
> The function virtio_pci_get_notifier() can also be used while vdev->status
> is not VIRTIO_CONFIG_S_DRIVER_OK. In that case, the vector_irqfd could be NULL,
> so also add the status check here.
> 2) Add the return value check for kvm_virtio_pci_vector_use_one().
> Since the return value of function virtio_pci_set_vector() is void,
> just add the error message 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
>
all trailers including s.o.b. should be together, with no empty lines
> Signed-off-by: Cindy Lu <lulu@redhat.com>
> ---
> hw/virtio/virtio-pci.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index b1d02f4b3d..87307b9061 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -860,6 +860,11 @@ 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 == NULL) &&
Preferable:
if (!proxy->vector_irqfd &&
and brackets not really needed here
> + (vdev->status & VIRTIO_CONFIG_S_DRIVER_OK)) {
brackets not really needed here
> + return -1;
> + }
> +
> if (queue_no == VIRTIO_CONFIG_IRQ_IDX) {
> *n = virtio_config_get_guest_notifier(vdev);
> *vector = vdev->config_vector;
> @@ -1452,7 +1457,9 @@ static void virtio_pci_set_vector(VirtIODevice *vdev,
> }
> /* If the new vector changed need to set it up. */
> if (kvm_irqfd && new_vector != VIRTIO_NO_VECTOR) {
> - kvm_virtio_pci_vector_use_one(proxy, queue_no);
> + if (kvm_virtio_pci_vector_use_one(proxy, queue_no)) {
> + virtio_error(vdev, "fail to set the vector %d", new_vector);
> + }
> }
> }
Is this an unrelated fix?
>
> --
> 2.45.0
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3] virtio-pci: Fix the use of an uninitialized irqfd
2024-06-26 7:44 ` Michael S. Tsirkin
@ 2024-06-27 8:40 ` Cindy Lu
2024-06-27 8:55 ` Michael S. Tsirkin
0 siblings, 1 reply; 5+ messages in thread
From: Cindy Lu @ 2024-06-27 8:40 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: jasowang, qemu-devel, philmd, qemu-stable
On Wed, Jun 26, 2024 at 3:45 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Wed, Jun 26, 2024 at 10:44:31AM +0800, Cindy Lu 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
> >
> > The root cause is the function virtio_pci_set_guest_notifiers() was not called
> > in the virtio_input device.So the vector_irqfd was not initialized
> >
> > So the fix is add the check for vector_irqfd.
> >
> > Change in V3
>
> Changelog belongs after ---
>
thanks micheal, will fix this
> > 1) Move the vector_irqfd check to virtio_pci_get_notifier().
> > The function virtio_pci_get_notifier() can also be used while vdev->status
> > is not VIRTIO_CONFIG_S_DRIVER_OK. In that case, the vector_irqfd could be NULL,
> > so also add the status check here.
> > 2) Add the return value check for kvm_virtio_pci_vector_use_one().
> > Since the return value of function virtio_pci_set_vector() is void,
> > just add the error message 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
> >
>
> all trailers including s.o.b. should be together, with no empty lines
>
will change this
> > Signed-off-by: Cindy Lu <lulu@redhat.com>
> > ---
> > hw/virtio/virtio-pci.c | 9 ++++++++-
> > 1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> > index b1d02f4b3d..87307b9061 100644
> > --- a/hw/virtio/virtio-pci.c
> > +++ b/hw/virtio/virtio-pci.c
> > @@ -860,6 +860,11 @@ 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 == NULL) &&
>
> Preferable:
> if (!proxy->vector_irqfd &&
>
> and brackets not really needed here
>
>
> > + (vdev->status & VIRTIO_CONFIG_S_DRIVER_OK)) {
>
> brackets not really needed here
>
sure, will fix this
> > + return -1;
> > + }
> > +
> > if (queue_no == VIRTIO_CONFIG_IRQ_IDX) {
> > *n = virtio_config_get_guest_notifier(vdev);
> > *vector = vdev->config_vector;
> > @@ -1452,7 +1457,9 @@ static void virtio_pci_set_vector(VirtIODevice *vdev,
> > }
> > /* If the new vector changed need to set it up. */
> > if (kvm_irqfd && new_vector != VIRTIO_NO_VECTOR) {
> > - kvm_virtio_pci_vector_use_one(proxy, queue_no);
> > + if (kvm_virtio_pci_vector_use_one(proxy, queue_no)) {
> > + virtio_error(vdev, "fail to set the vector %d", new_vector);
> > + }
> > }
> > }
>
>
> Is this an unrelated fix?
>
this is for last version's comment, there is an return value for
function kvm_virtio_pci_vector_use_one, but
we can do nothing here, so I just add an erorr message
Thanks
cindy
> >
> > --
> > 2.45.0
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3] virtio-pci: Fix the use of an uninitialized irqfd
2024-06-27 8:40 ` Cindy Lu
@ 2024-06-27 8:55 ` Michael S. Tsirkin
2024-06-27 8:57 ` Cindy Lu
0 siblings, 1 reply; 5+ messages in thread
From: Michael S. Tsirkin @ 2024-06-27 8:55 UTC (permalink / raw)
To: Cindy Lu; +Cc: jasowang, qemu-devel, philmd, qemu-stable
On Thu, Jun 27, 2024 at 04:40:33PM +0800, Cindy Lu wrote:
> On Wed, Jun 26, 2024 at 3:45 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > On Wed, Jun 26, 2024 at 10:44:31AM +0800, Cindy Lu 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
> > >
> > > The root cause is the function virtio_pci_set_guest_notifiers() was not called
> > > in the virtio_input device.So the vector_irqfd was not initialized
> > >
> > > So the fix is add the check for vector_irqfd.
> > >
> > > Change in V3
> >
> > Changelog belongs after ---
> >
> thanks micheal, will fix this
> > > 1) Move the vector_irqfd check to virtio_pci_get_notifier().
> > > The function virtio_pci_get_notifier() can also be used while vdev->status
> > > is not VIRTIO_CONFIG_S_DRIVER_OK. In that case, the vector_irqfd could be NULL,
> > > so also add the status check here.
> > > 2) Add the return value check for kvm_virtio_pci_vector_use_one().
> > > Since the return value of function virtio_pci_set_vector() is void,
> > > just add the error message 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
> > >
> >
> > all trailers including s.o.b. should be together, with no empty lines
> >
> will change this
> > > Signed-off-by: Cindy Lu <lulu@redhat.com>
> > > ---
> > > hw/virtio/virtio-pci.c | 9 ++++++++-
> > > 1 file changed, 8 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> > > index b1d02f4b3d..87307b9061 100644
> > > --- a/hw/virtio/virtio-pci.c
> > > +++ b/hw/virtio/virtio-pci.c
> > > @@ -860,6 +860,11 @@ 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 == NULL) &&
> >
> > Preferable:
> > if (!proxy->vector_irqfd &&
> >
> > and brackets not really needed here
> >
> >
> > > + (vdev->status & VIRTIO_CONFIG_S_DRIVER_OK)) {
> >
> > brackets not really needed here
> >
> sure, will fix this
> > > + return -1;
> > > + }
> > > +
> > > if (queue_no == VIRTIO_CONFIG_IRQ_IDX) {
> > > *n = virtio_config_get_guest_notifier(vdev);
> > > *vector = vdev->config_vector;
> > > @@ -1452,7 +1457,9 @@ static void virtio_pci_set_vector(VirtIODevice *vdev,
> > > }
> > > /* If the new vector changed need to set it up. */
> > > if (kvm_irqfd && new_vector != VIRTIO_NO_VECTOR) {
> > > - kvm_virtio_pci_vector_use_one(proxy, queue_no);
> > > + if (kvm_virtio_pci_vector_use_one(proxy, queue_no)) {
> > > + virtio_error(vdev, "fail to set the vector %d", new_vector);
failed.
and print errno?
> > > + }
> > > }
> > > }
> >
> >
> > Is this an unrelated fix?
> >
> this is for last version's comment, there is an return value for
> function kvm_virtio_pci_vector_use_one, but
> we can do nothing here, so I just add an erorr message
> Thanks
> cindy
I'd make it a separate patch, documenting in the commit log what
is going on.
> > >
> > > --
> > > 2.45.0
> >
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3] virtio-pci: Fix the use of an uninitialized irqfd
2024-06-27 8:55 ` Michael S. Tsirkin
@ 2024-06-27 8:57 ` Cindy Lu
0 siblings, 0 replies; 5+ messages in thread
From: Cindy Lu @ 2024-06-27 8:57 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: jasowang, qemu-devel, philmd, qemu-stable
On Thu, Jun 27, 2024 at 4:55 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Thu, Jun 27, 2024 at 04:40:33PM +0800, Cindy Lu wrote:
> > On Wed, Jun 26, 2024 at 3:45 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> > >
> > > On Wed, Jun 26, 2024 at 10:44:31AM +0800, Cindy Lu 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
> > > >
> > > > The root cause is the function virtio_pci_set_guest_notifiers() was not called
> > > > in the virtio_input device.So the vector_irqfd was not initialized
> > > >
> > > > So the fix is add the check for vector_irqfd.
> > > >
> > > > Change in V3
> > >
> > > Changelog belongs after ---
> > >
> > thanks micheal, will fix this
> > > > 1) Move the vector_irqfd check to virtio_pci_get_notifier().
> > > > The function virtio_pci_get_notifier() can also be used while vdev->status
> > > > is not VIRTIO_CONFIG_S_DRIVER_OK. In that case, the vector_irqfd could be NULL,
> > > > so also add the status check here.
> > > > 2) Add the return value check for kvm_virtio_pci_vector_use_one().
> > > > Since the return value of function virtio_pci_set_vector() is void,
> > > > just add the error message 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
> > > >
> > >
> > > all trailers including s.o.b. should be together, with no empty lines
> > >
> > will change this
> > > > Signed-off-by: Cindy Lu <lulu@redhat.com>
> > > > ---
> > > > hw/virtio/virtio-pci.c | 9 ++++++++-
> > > > 1 file changed, 8 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> > > > index b1d02f4b3d..87307b9061 100644
> > > > --- a/hw/virtio/virtio-pci.c
> > > > +++ b/hw/virtio/virtio-pci.c
> > > > @@ -860,6 +860,11 @@ 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 == NULL) &&
> > >
> > > Preferable:
> > > if (!proxy->vector_irqfd &&
> > >
> > > and brackets not really needed here
> > >
> > >
> > > > + (vdev->status & VIRTIO_CONFIG_S_DRIVER_OK)) {
> > >
> > > brackets not really needed here
> > >
> > sure, will fix this
> > > > + return -1;
> > > > + }
> > > > +
> > > > if (queue_no == VIRTIO_CONFIG_IRQ_IDX) {
> > > > *n = virtio_config_get_guest_notifier(vdev);
> > > > *vector = vdev->config_vector;
> > > > @@ -1452,7 +1457,9 @@ static void virtio_pci_set_vector(VirtIODevice *vdev,
> > > > }
> > > > /* If the new vector changed need to set it up. */
> > > > if (kvm_irqfd && new_vector != VIRTIO_NO_VECTOR) {
> > > > - kvm_virtio_pci_vector_use_one(proxy, queue_no);
> > > > + if (kvm_virtio_pci_vector_use_one(proxy, queue_no)) {
> > > > + virtio_error(vdev, "fail to set the vector %d", new_vector);
>
> failed.
> and print errno?
>
> > > > + }
> > > > }
> > > > }
> > >
> > >
> > > Is this an unrelated fix?
> > >
> > this is for last version's comment, there is an return value for
> > function kvm_virtio_pci_vector_use_one, but
> > we can do nothing here, so I just add an erorr message
> > Thanks
> > cindy
>
>
> I'd make it a separate patch, documenting in the commit log what
> is going on.
>
sure, will do
thanks
cindy
> > > >
> > > > --
> > > > 2.45.0
> > >
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-06-27 8:59 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-06-26 2:44 [PATCH v3] virtio-pci: Fix the use of an uninitialized irqfd Cindy Lu
2024-06-26 7:44 ` Michael S. Tsirkin
2024-06-27 8:40 ` Cindy Lu
2024-06-27 8:55 ` Michael S. Tsirkin
2024-06-27 8:57 ` Cindy Lu
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).