From: Peter Krempa <pkrempa@redhat.com> A special case in qemuDomainAttachDeviceDiskLive causes disk media to be changed. This code has different semantics than the real hotplug code where the hotplugged device definition is absorbed into the domain definition and thus the pointer is still valid. On media change we just use the disk source and discard everything else from the disk definition. Later in qemuDomainAttachDeviceLive we then attempt to extract the alias of the attached device for emiting an event. Since in case of media change the main definition was freed this causes an use-after-free on the disk data pointer. To address this the media change code will clear the disk definition pointer from the device wrapper and the caller will extract the device alias only when the disk definition pointer is non-NULL. The semantics of the event will not change because the device alias wouldn't be assigned for the media change code at all. The use-after-free is observable via valgrind when attempting a media change via 'virsh attach-device', as otherwise in most cases it doesn't cause any ill efect as only the pointer to a NULL string is accessed: ==2763495== Invalid read of size 8 ==2763495== at 0xEA4102A: qemuDomainAttachDeviceLive (qemu_hotplug.c:3455) ==2763495== by 0xEA28ECD: qemuDomainAttachDeviceLiveAndConfig (qemu_driver.c:7408) ==2763495== by 0xEA28ECD: qemuDomainAttachDeviceFlags (qemu_driver.c:7456) ==2763495== by 0x4BC5BE6: virDomainAttachDevice (libvirt-domain.c:8951) ==2763495== by 0x402579D: remoteDispatchDomainAttachDevice (remote_daemon_dispatch_stubs.h:3763) [snip] ==2763495== Address 0x6df57c8 is 360 bytes inside a block of size 608 free'd ==2763495== at 0x48F7E43: free (vg_replace_malloc.c:990) ==2763495== by 0x4EC7EC4: g_free (in /usr/lib64/libglib-2.0.so.0.8600.3) ==2763495== by 0xEA4101E: qemuDomainAttachDeviceDiskLive (qemu_hotplug.c:1150) ==2763495== by 0xEA4101E: qemuDomainAttachDeviceLive (qemu_hotplug.c:3453) ==2763495== by 0xEA28ECD: qemuDomainAttachDeviceLiveAndConfig (qemu_driver.c:7408) ==2763495== by 0xEA28ECD: qemuDomainAttachDeviceFlags (qemu_driver.c:7456) ==2763495== by 0x4BC5BE6: virDomainAttachDevice (libvirt-domain.c:8951) [snip] Closes: https://gitlab.com/libvirt/libvirt/-/issues/859 Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_hotplug.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 6235f6c27c..df4bb49af7 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1146,7 +1146,7 @@ qemuDomainAttachDeviceDiskLive(virQEMUDriver *driver, return -1; dev->data.disk->src = NULL; - virDomainDiskDefFree(dev->data.disk); + g_clear_pointer(&dev->data.disk, virDomainDiskDefFree); return 0; } @@ -3450,7 +3450,7 @@ qemuDomainAttachDeviceLive(virDomainObj *vm, case VIR_DOMAIN_DEVICE_DISK: qemuDomainObjCheckDiskTaint(driver, vm, dev->data.disk, NULL); ret = qemuDomainAttachDeviceDiskLive(driver, vm, dev); - if (!ret) { + if (ret == 0 && dev->data.disk) { alias = dev->data.disk->info.alias; dev->data.disk = NULL; } -- 2.53.0