aboutsummaryrefslogtreecommitdiff
path: root/hw/vfio
diff options
context:
space:
mode:
authorMichal Privoznik <mprivozn@redhat.com>2019-11-18 10:41:47 -0700
committerAlex Williamson <alex.williamson@redhat.com>2019-11-18 10:41:47 -0700
commit1335d64323be87ee14c766c59fabfb7e9acd7af7 (patch)
tree1ad9d7c7b33eba26f5eb0cfaf4aae2481bde56f3 /hw/vfio
parent1bd0f1c9c149c2fb738f381099cec7ad0ee224a9 (diff)
downloadqemu-1335d64323be87ee14c766c59fabfb7e9acd7af7.zip
qemu-1335d64323be87ee14c766c59fabfb7e9acd7af7.tar.gz
qemu-1335d64323be87ee14c766c59fabfb7e9acd7af7.tar.bz2
hw/vfio/pci: Fix double free of migration_blocker
When user tries to hotplug a VFIO device, but the operation fails somewhere in the middle (in my testing it failed because of RLIMIT_MEMLOCK forbidding more memory allocation), then a double free occurs. In vfio_realize() the vdev->migration_blocker is allocated, then something goes wrong which causes control to jump onto 'error' label where the error is freed. But the pointer is left pointing to invalid memory. Later, when vfio_instance_finalize() is called, the memory is freed again. In my testing the second hunk was sufficient to fix the bug, but I figured the first hunk doesn't hurt either. ==169952== Invalid read of size 8 ==169952== at 0xA47DCD: error_free (error.c:266) ==169952== by 0x4E0A18: vfio_instance_finalize (pci.c:3040) ==169952== by 0x8DF74C: object_deinit (object.c:606) ==169952== by 0x8DF7BE: object_finalize (object.c:620) ==169952== by 0x8E0757: object_unref (object.c:1074) ==169952== by 0x45079C: memory_region_unref (memory.c:1779) ==169952== by 0x45376B: do_address_space_destroy (memory.c:2793) ==169952== by 0xA5C600: call_rcu_thread (rcu.c:283) ==169952== by 0xA427CB: qemu_thread_start (qemu-thread-posix.c:519) ==169952== by 0x80A8457: start_thread (in /lib64/libpthread-2.29.so) ==169952== by 0x81C96EE: clone (in /lib64/libc-2.29.so) ==169952== Address 0x143137e0 is 0 bytes inside a block of size 48 free'd ==169952== at 0x4A342BB: free (vg_replace_malloc.c:530) ==169952== by 0xA47E05: error_free (error.c:270) ==169952== by 0x4E0945: vfio_realize (pci.c:3025) ==169952== by 0x76A4FF: pci_qdev_realize (pci.c:2099) ==169952== by 0x689B9A: device_set_realized (qdev.c:876) ==169952== by 0x8E2C80: property_set_bool (object.c:2080) ==169952== by 0x8E0EF6: object_property_set (object.c:1272) ==169952== by 0x8E3FC8: object_property_set_qobject (qom-qobject.c:26) ==169952== by 0x8E11DB: object_property_set_bool (object.c:1338) ==169952== by 0x5E7BDD: qdev_device_add (qdev-monitor.c:673) ==169952== by 0x5E81E5: qmp_device_add (qdev-monitor.c:798) ==169952== by 0x9E18A8: do_qmp_dispatch (qmp-dispatch.c:132) ==169952== Block was alloc'd at ==169952== at 0x4A35476: calloc (vg_replace_malloc.c:752) ==169952== by 0x51B1158: g_malloc0 (in /usr/lib64/libglib-2.0.so.0.6000.6) ==169952== by 0xA47357: error_setv (error.c:61) ==169952== by 0xA475D9: error_setg_internal (error.c:97) ==169952== by 0x4DF8C2: vfio_realize (pci.c:2737) ==169952== by 0x76A4FF: pci_qdev_realize (pci.c:2099) ==169952== by 0x689B9A: device_set_realized (qdev.c:876) ==169952== by 0x8E2C80: property_set_bool (object.c:2080) ==169952== by 0x8E0EF6: object_property_set (object.c:1272) ==169952== by 0x8E3FC8: object_property_set_qobject (qom-qobject.c:26) ==169952== by 0x8E11DB: object_property_set_bool (object.c:1338) ==169952== by 0x5E7BDD: qdev_device_add (qdev-monitor.c:673) Fixes: f045a0104c8c ("vfio: unplug failover primary device before migration") Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Cornelia Huck <cohuck@redhat.com> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
Diffstat (limited to 'hw/vfio')
-rw-r--r--hw/vfio/pci.c2
1 files changed, 2 insertions, 0 deletions
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index e6569a7..9c16599 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -2740,6 +2740,7 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
if (err) {
error_propagate(errp, err);
error_free(vdev->migration_blocker);
+ vdev->migration_blocker = NULL;
return;
}
}
@@ -3023,6 +3024,7 @@ error:
if (vdev->migration_blocker) {
migrate_del_blocker(vdev->migration_blocker);
error_free(vdev->migration_blocker);
+ vdev->migration_blocker = NULL;
}
}