Re: [PATCH v1] hw/vfio: Fix liveness check in vfio_connect_kvm_msi_virq()
"Maciej S. Szmigiero" <[email protected]>
| Newsgroups | gmane.comp.emulators.qemu |
|---|---|
| Message-ID | <[email protected]> |
On 27.07.2026 17:27, Cédric Le Goater wrote: > + CPR maintainers, > > On 7/27/26 17:00, Tycho Andersen wrote: >> From: "Tycho Andersen (AMD)" <[email protected]> >> >> While working on savevm/loadvm for a new vfio device, I encountered the >> crash below. Since vfio_connect_kvm_msi_virq() didn't check the ->use flag >> for the vector, it would pass an unused vector down to >> vfio_cpr_load_vector_fd() which would crash. >> >> Fix this by checking the ->use flag along with the virq number to detect >> whether a vector is valid or not. >> >> Thread 1 "qemu-system-x86" received signal SIGSEGV, Segmentation fault. >> 0x0000555555a891ef in vfio_cpr_load_vector_fd (vdev=vdev@entry=0x0, >> name=name@entry=0x555555eeb27e "kvm_interrupt", nr=nr@entry=1) at ../hw/vfio/cpr.c:44 >> 44 g_autofree char *fdname = STRDUP_VECTOR_FD_NAME(vdev, name); >> (gdb) bt >> #0 0x0000555555a891ef in vfio_cpr_load_vector_fd >> (vdev=vdev@entry=0x0, name=name@entry=0x555555eeb27e "kvm_interrupt", nr=nr@entry=1) >> at ../hw/vfio/cpr.c:44 >> #1 0x0000555555ce64a1 in vfio_notifier_init >> (vdev=0x0, e=e@entry=0x5555586971b4, name=name@entry=0x555555eeb27e "kvm_interrupt", nr=nr@entry=1, errp=errp@entry=0x0) at ../hw/vfio/pci.c:79 >> #2 0x0000555555ce721e in vfio_connect_kvm_msi_virq (vector=0x5555586971a8, nr=nr@entry=1) >> at ../hw/vfio/pci.c:601 >> #3 0x0000555555cea5a5 in vfio_connect_kvm_msi_virq (nr=1, vector=<optimized out>) >> at ../hw/vfio/pci.c:597 >> #4 vfio_pci_commit_kvm_msi_virq_batch (vdev=0x55555906de40) at ../hw/vfio/pci.c:822 >> #5 0x0000555555cea9f2 in vfio_msix_enable (vdev=vdev@entry=0x55555906de40) at ../hw/vfio/pci.c:850 >> #6 0x0000555555ceb152 in vfio_pci_load_config (vbasedev=0x55555906e900, f=<optimized out>) >> at ../hw/vfio/pci.c:3088 >> #7 0x0000555555a8c765 in vfio_load_device_config_state (f=0x5555574a43d0, opaque=0x55555906e900) >> at ../hw/vfio/migration.c:278 >> #8 0x0000555555b3a522 in vmstate_load >> (f=f@entry=0x5555574a43d0, se=se@entry=0x5555591edd40, errp=errp@entry=0x7fffffffe130) >> at ../migration/savevm.c:971 >> #9 0x0000555555b3ab1a in qemu_loadvm_section_start_full >> (f=f@entry=0x5555574a43d0, type=type@entry=4 '\004', errp=errp@entry=0x7fffffffe130) >> at ../migration/savevm.c:2654 >> #10 0x0000555555b3e1ee in qemu_loadvm_state_main >> (f=f@entry=0x5555574a43d0, mis=mis@entry=0x5555571de5a0, errp=0x7fffffffe130, >> errp@entry=0x555557157c10 <error_fatal>) at ../migration/savevm.c:2973 >> #11 0x0000555555b3f7b7 in qemu_loadvm_state >> (f=f@entry=0x5555574a43d0, errp=errp@entry=0x555557157c10 <error_fatal>) >> at ../migration/savevm.c:3058 >> #12 0x0000555555b40863 in load_snapshot >> (name=0x7fffffffecc9 "foo", vmstate=vmstate@entry=0x0, has_devices=has_devices@entry=false, devices=devices@entry=0x0, errp=errp@entry=0x555557157c10 <error_fatal>) at ../migration/savevm.c:3452 >> #13 0x0000555555adc211 in qmp_x_exit_preconfig (errp=0x555557157c10 <error_fatal>) at ../system/vl.c:2817 >> #14 qmp_x_exit_preconfig (errp=0x555557157c10 <error_fatal>) at ../system/vl.c:2802 >> #15 0x0000555555adf8ed in qemu_init (argc=<optimized out>, argv=<optimized out>) at ../system/vl.c:3849 >> #16 0x00005555558903fd in main (argc=<optimized out>, argv=<optimized out>) at ../system/main.c:71 >> >> Fixes: 30edcb4d4e7a ("vfio-pci: preserve MSI") >> Signed-off-by: Tycho Andersen (AMD) <[email protected]> >> --- >> An alternative implementation of this would be to set ->virq = -1 after >> the allocation in vfio_msix_enable(), but since other locations that >> checks ->virq >= 0 also check ->use, I added it here too. Maybe it would >> be useful to do both. >> --- >> hw/vfio/pci.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c >> index 380dd8c15f..b5280c3d2a 100644 >> --- a/hw/vfio/pci.c >> +++ b/hw/vfio/pci.c >> @@ -589,7 +589,7 @@ static void vfio_connect_kvm_msi_virq(VFIOMSIVector *vector, int nr) >> { >> const char *name = "kvm_interrupt"; >> - if (vector->virq < 0) { >> + if (!vector->use || vector->virq < 0) { >> return; >> } >> >> base-commit: 300438ffbb8d9430cac2fcc15cba6f482b2c0587 > > > Looks ok to me. Looks ok to me from the CPR side too, so: Acked-by: Maciej S. Szmigiero <[email protected]> # for CPR Thanks, Maciej