On Mon, 11 Nov 2019 11:37:41 +0100
Michal Privoznik <mpriv...@redhat.com> wrote:
> 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.
Better safe than sorry, I guess.
>
> ==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)
>
> Signed-off-by: Michal Privoznik <mpriv...@redhat.com>
Fixes: f045a0104c8c ("vfio: unplug failover primary device before migration")
> ---
> hw/vfio/pci.c | 2 ++
> 1 file changed, 2 insertions(+)
Reviewed-by: Cornelia Huck <coh...@redhat.com>
