On 09/09/2022 14:10, Jiacheng Jiang wrote:
> From: jiangjiacheng <jiangjiacheng@huawei.com>
>
> The qemuDomainAttachDeviceLive interface is invoked for attaching cdrom in
> the same way as common disks. The difference is that attach cdrom only update
> the src of the original device while common disk will add new disk to vm's
> device list. Therefore, the dev->data.disk should be freed to avoid memory leak
> when attach cdrom as well as floppy.
I think your colleague has fixed it in 2f470a4fb1e ("qemu: fix memleak
in qemuDomainAttachDeviceLive()").
But I think there might be another UAF problem in the code. If updating
the src of cdrom/floppy successfully, then dev->data.disk should be
freed in `qemuDomainAttachDeviceDiskLive`, however we access
dev->data.disk->info.alias after that.
Thanks,
Peng
>
> Signed-off-by: jiangjiacheng <jiangjiacheng@huawei.com>
> ---
> src/qemu/qemu_driver.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index c43bc4070e..64b1ca3f39 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -6882,6 +6882,9 @@ qemuDomainAttachDeviceLive(virDomainObj *vm,
> ret = qemuDomainAttachDeviceDiskLive(driver, vm, dev);
> if (!ret) {
> alias = dev->data.disk->info.alias;
> + if ((virDomainDiskDevice)dev->data.disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM ||
> + (virDomainDiskDevice)dev->data.disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY)
> + virDomainDiskDefFree(dev->data.disk);
> dev->data.disk = NULL;
> }
> break;