| Message ID | 20210216052917.5712-1-yuri.benditovich@daynix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | virtio-pci: add check for vdev in virtio_pci_isr_read | expand |
Hi Yuri, On 2/16/21 6:29 AM, Yuri Benditovich wrote: > https://bugzilla.redhat.com/show_bug.cgi?id=1743098 Maybe add backtrace in patch description? (gdb) bt #0 0xc5bbdf0d in virtio_pci_notify_write (opaque=0x55b6c6dff170, addr=0, val=<...>, size=<...>) at hw/virtio/virtio-pci.c:1360 #1 0xc59fe596 in memory_region_write_accessor (mr=<...>, addr=<...>, value=<...>, size=<...>, shift=<...>, mask=<...>, attrs=...) at memory.c:530 #2 0xc59fc9e6 in access_with_adjusted_size (addr=addr@entry=0, value=..., size=size@entry=2, access_size_min=<...>, access_size_max=<...>, access_fn=<memory_region_write_accessor>, mr=0x55b6c6df7cd0, attrs=...) at memory.c:597 #3 0xc5a0084a in memory_region_dispatch_write (mr=0x55b6c6df7cd0, addr=0, data=<...>, size=2, attrs=...) at memory.c:1474 #4 0xc59aebbc in flatview_write (fv=0x7f1abc0407c0, addr=<...>, attrs=..., buf=<...>, len=<...>) at exec.c:3099 #5 0xc59b3243 in address_space_write (as=<...>, addr=<...>, attrs=..., buf=<...>, len=<...>) at exec.c:3265 #6 0xc5a0f878 in kvm_cpu_exec (cpu=<...>) at accel/kvm/kvm-all.c:2004 #7 0xc59ec21e in qemu_kvm_cpu_thread_fn (arg=0x55b6c6bdddd0) at cpus.c:1215 #8 0x00007f1adb4732de in start_thread (arg=<...>) at pthread_create.c:486 #9 0x00007f1adb1a4133 in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95 > There is missing check for vdev in this procedure. > QEMU crash happens in it in hot unplug flow. > > Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com> > --- > hw/virtio/virtio-pci.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c > index 094c36aa3e..2f19301267 100644 > --- a/hw/virtio/virtio-pci.c > +++ b/hw/virtio/virtio-pci.c > @@ -1364,7 +1364,13 @@ static uint64_t virtio_pci_isr_read(void *opaque, hwaddr addr, > { > VirtIOPCIProxy *proxy = opaque; > VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus); > - uint64_t val = qatomic_xchg(&vdev->isr, 0); > + uint64_t val = 0; uint64_t val; > + > + if (vdev == NULL) { > + return val; return 0; > + } > + > + val = qatomic_xchg(&vdev->isr, 0); > pci_irq_deassert(&proxy->pci_dev); > > return val; > This pattern is present in other functions of this file. Can you fix them too, or add an assert() if it is unlikely to happen? Thanks, Phil.
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c index 094c36aa3e..2f19301267 100644 --- a/hw/virtio/virtio-pci.c +++ b/hw/virtio/virtio-pci.c @@ -1364,7 +1364,13 @@ static uint64_t virtio_pci_isr_read(void *opaque, hwaddr addr, { VirtIOPCIProxy *proxy = opaque; VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus); - uint64_t val = qatomic_xchg(&vdev->isr, 0); + uint64_t val = 0; + + if (vdev == NULL) { + return val; + } + + val = qatomic_xchg(&vdev->isr, 0); pci_irq_deassert(&proxy->pci_dev); return val;
https://bugzilla.redhat.com/show_bug.cgi?id=1743098 There is missing check for vdev in this procedure. QEMU crash happens in it in hot unplug flow. Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com> --- hw/virtio/virtio-pci.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)