Re: [PATCH 3/3] qemu: Fix memory leak fix memory leak in the condition of attaching cdrom

Peng Liang <tcx4c70@xxxxxxxxx> · Fri, 9 Sep 2022 22:00:50 +0800

On 09/09/2022 14:10, Jiacheng Jiang wrote:
From: jiangjiacheng <jiangjiacheng@xxxxxxxxxx>

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@xxxxxxxxxx>
---
  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;







