[PATCH QEMU HCI-8.1 v3 18/28] kvm: tolerate ioctls after kvm_close()

[Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>]

Since commit 7ed0919119b0 ("migration: close kvm after cpr") a source
QEMU that has handed its VFIO devices over via cpr-transfer (or
cpr-exec) calls kvm_close() once the migration completes, so that the
destination can register its irq bypass consumers.  kvm_close() closes
the VM and vCPU descriptors and sets kvm_state to NULL, but the source
process keeps living with all its device models intact until the
management layer terminates it.

Nothing prevents those device models from raising interrupts in the
meantime, and the regular exit path even guarantees one: net_cleanup()
deletes the netdev behind an emulated virtio-net, the NIC sees the link
go down and calls virtio_notify_config(), MSI-X delivers through the
in-kernel APIC, and kvm_irqchip_send_msi() dereferences the NULL
kvm_state:

  #0  kvm_vm_ioctl (s=0x0, type=KVM_SIGNAL_MSI)   accel/kvm/kvm-all.c
  #1  kvm_irqchip_send_msi (s=0x0)
  #2  kvm_send_msi                                hw/i386/kvm/apic.c
  #3  kvm_apic_mem_write
  ...
  #9  pci_msi_trigger
  #11 msix_notify
  #12 virtio_pci_notify
  #14 virtio_notify_config
  #15 virtio_net_set_link_status
  #16 qemu_del_net_client
  #17 net_cleanup
  #18 qemu_cleanup

Every retired cpr-transfer source with a VFIO device thus dies with
SIGSEGV on SIGTERM/quit and leaves a core dump behind.  Without a VFIO
device kvm_close() never runs and the same exit is harmless.

Make the ioctl wrappers honour the closed state instead of assuming a
live KVMState: kvm_ioctl(), kvm_vm_ioctl() and kvm_vcpu_ioctl() return
-ENODEV when kvm_close() has already been called, so any late interrupt
or query on the retired source fails gracefully.  Teach kvm_send_msi()
that -ENODEV in this situation is expected and not worth the "MSI lost"
warning on every such interrupt.

Fixes: 7ed0919119b0 ("migration: close kvm after cpr")
Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
 accel/kvm/kvm-all.c | 15 +++++++++++++++
 hw/i386/kvm/apic.c  |  6 +++++-
 2 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index a39c1ba260a..ea027c37829 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -3371,6 +3371,11 @@ int kvm_ioctl(KVMState *s, unsigned long type, ...)
     arg = va_arg(ap, void *);
     va_end(ap);
 
+    /* kvm_close() has been called: there is no KVM to talk to any more */
+    if (!s || s->fd == -1) {
+        return -ENODEV;
+    }
+
     trace_kvm_ioctl(type, arg);
     ret = ioctl(s->fd, type, arg);
     if (ret == -1) {
@@ -3389,6 +3394,11 @@ int kvm_vm_ioctl(KVMState *s, unsigned long type, ...)
     arg = va_arg(ap, void *);
     va_end(ap);
 
+    /* kvm_close() has been called: there is no VM to talk to any more */
+    if (!s || s->vmfd == -1) {
+        return -ENODEV;
+    }
+
     trace_kvm_vm_ioctl(type, arg);
     accel_ioctl_begin();
     ret = ioctl(s->vmfd, type, arg);
@@ -3409,6 +3419,11 @@ int kvm_vcpu_ioctl(CPUState *cpu, unsigned long type, ...)
     arg = va_arg(ap, void *);
     va_end(ap);
 
+    /* kvm_close() has been called: the vCPU fds are gone */
+    if (cpu->kvm_fd == -1) {
+        return -ENODEV;
+    }
+
     trace_kvm_vcpu_ioctl(cpu->cpu_index, type, arg);
     accel_cpu_ioctl_begin(cpu);
     ret = ioctl(cpu->kvm_fd, type, arg);
diff --git a/hw/i386/kvm/apic.c b/hw/i386/kvm/apic.c
index 1be9bfe36e9..0c9f7759b6e 100644
--- a/hw/i386/kvm/apic.c
+++ b/hw/i386/kvm/apic.c
@@ -196,7 +196,11 @@ static void kvm_send_msi(MSIMessage *msg)
     msg->address = kvm_swizzle_msi_ext_dest_id(msg->address);
 
     ret = kvm_irqchip_send_msi(kvm_state, *msg);
-    if (ret < 0) {
+    /*
+     * -ENODEV means kvm_close() has already run (the VM was handed over
+     * via CPR); the interrupt is meaningless here and not worth a warning.
+     */
+    if (ret < 0 && ret != -ENODEV) {
         fprintf(stderr, "KVM: injection failed, MSI lost (%s)\n",
                 strerror(-ret));
     }

-- 
2.47.1