[Qemu-commits] [qemu/qemu] 1335d6: hw/vfio/pci: Fix double free of migration_blocker

[Peter Maydell]

  Branch: refs/heads/master
  Home:   https://github.com/qemu/qemu
  Commit: 1335d64323be87ee14c766c59fabfb7e9acd7af7
      
https://github.com/qemu/qemu/commit/1335d64323be87ee14c766c59fabfb7e9acd7af7
  Author: Michal Privoznik <address@hidden>
  Date:   2019-11-18 (Mon, 18 Nov 2019)

  Changed paths:
    M hw/vfio/pci.c

  Log Message:
  -----------
  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 <address@hidden>
Reviewed-by: Cornelia Huck <address@hidden>
Signed-off-by: Alex Williamson <address@hidden>


  Commit: ed92369a5725ae42c28128778f154798f1464c26
      
https://github.com/qemu/qemu/commit/ed92369a5725ae42c28128778f154798f1464c26
  Author: Jens Freimann <address@hidden>
  Date:   2019-11-18 (Mon, 18 Nov 2019)

  Changed paths:
    M hw/vfio/pci.c

  Log Message:
  -----------
  vfio: don't ignore return value of migrate_add_blocker

When an error occurs in migrate_add_blocker() it sets a
negative return value and uses error pointer we pass in.
Instead of just looking at the error pointer check for a negative return
value and avoid a coverity error because the return value is
set but never used. This fixes CID 1407219.

Reported-by: Coverity (CID 1407219)
Fixes: f045a0104c8c ("vfio: unplug failover primary device before migration")
Signed-off-by: Jens Freimann <address@hidden>
Reviewed-by: Stefano Garzarella <address@hidden>
Reviewed-by: Philippe Mathieu-Daudé <address@hidden>
Signed-off-by: Alex Williamson <address@hidden>


  Commit: 29b95c992a569818294478b4616e44b45c67d34e
      
https://github.com/qemu/qemu/commit/29b95c992a569818294478b4616e44b45c67d34e
  Author: Paolo Bonzini <address@hidden>
  Date:   2019-11-18 (Mon, 18 Nov 2019)

  Changed paths:
    M hw/vfio/Kconfig

  Log Message:
  -----------
  vfio: vfio-pci requires EDID

hw/vfio/display.c needs the EDID subsystem, select it.

Cc: Alex Williamson <address@hidden>
Signed-off-by: Paolo Bonzini <address@hidden>
Signed-off-by: Alex Williamson <address@hidden>


  Commit: f086f22d6c068ba151b0f6e81e75a64f130df712
      
https://github.com/qemu/qemu/commit/f086f22d6c068ba151b0f6e81e75a64f130df712
  Author: Peter Maydell <address@hidden>
  Date:   2019-11-18 (Mon, 18 Nov 2019)

  Changed paths:
    M hw/vfio/Kconfig
    M hw/vfio/pci.c

  Log Message:
  -----------
  Merge remote-tracking branch 'remotes/awilliam/tags/vfio-fixes-20191118.0' 
into staging

VFIO fixes 2019-11-18

 - Fix migration blocker double free (Michal Privoznik)

 - Use migration_add_blocker() return value (Jens Freimann)

 - Depend on EDID for display support (Paolo Bonzini)

# gpg: Signature made Mon 18 Nov 2019 17:44:52 GMT
# gpg:                using RSA key 239B9B6E3BB08B22
# gpg: Good signature from "Alex Williamson <address@hidden>" [full]
# gpg:                 aka "Alex Williamson <address@hidden>" [full]
# gpg:                 aka "Alex Williamson <address@hidden>" [full]
# gpg:                 aka "Alex Williamson <address@hidden>" [full]
# Primary key fingerprint: 42F6 C04E 540B D1A9 9E7B  8A90 239B 9B6E 3BB0 8B22

* remotes/awilliam/tags/vfio-fixes-20191118.0:
  vfio: vfio-pci requires EDID
  vfio: don't ignore return value of migrate_add_blocker
  hw/vfio/pci: Fix double free of migration_blocker

Signed-off-by: Peter Maydell <address@hidden>


Compare: https://github.com/qemu/qemu/compare/a5c2a235103a...f086f22d6c06