virStorageTranslateDiskSourcePool: Avoid double free

https://bugzilla.redhat.com/show_bug.cgi?id=1316370

Consider the following disk for a domain:

    <disk type='volume' device='cdrom'>
      <driver name='qemu' type='raw'/>
      <auth username='libvirt'>
        <secret type='iscsi' usage='libvirtiscsi'/>
      </auth>
      <source pool='iscsi-secret-pool' volume='unit:0:0:0' mode='direct' startupPolicy='optional'/>
      <target dev='sda' bus='scsi'/>
      <readonly/>
      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
    </disk>

Now, startupPolicy is currently not allowed for iscsi disks, so
one would expect an error message to be thrown. But what a
surprise is waiting for users if they try to start up such
domain:

==15724== Invalid free() / delete / delete[] / realloc()
==15724==    at 0x4C2B1F0: free (vg_replace_malloc.c:473)
==15724==    by 0x54B7A69: virFree (viralloc.c:582)
==15724==    by 0x552DC90: virStorageAuthDefFree (virstoragefile.c:1549)
==15724==    by 0x552F023: virStorageSourceClear (virstoragefile.c:2055)
==15724==    by 0x552F054: virStorageSourceFree (virstoragefile.c:2067)
==15724==    by 0x55556AA: virDomainDiskDefFree (domain_conf.c:1562)
==15724==    by 0x5557ABE: virDomainDefFree (domain_conf.c:2547)
==15724==    by 0x1B43CC42: qemuProcessStop (qemu_process.c:5918)
==15724==    by 0x1B43BA2E: qemuProcessStart (qemu_process.c:5511)
==15724==    by 0x1B48993E: qemuDomainObjStart (qemu_driver.c:7050)
==15724==    by 0x1B489B9A: qemuDomainCreateWithFlags (qemu_driver.c:7104)
==15724==    by 0x1B489C01: qemuDomainCreate (qemu_driver.c:7122)
==15724==  Address 0x21cfbb90 is 0 bytes inside a block of size 48 free'd
==15724==    at 0x4C2B1F0: free (vg_replace_malloc.c:473)
==15724==    by 0x54B7A69: virFree (viralloc.c:582)
==15724==    by 0x552DC90: virStorageAuthDefFree (virstoragefile.c:1549)
==15724==    by 0x12D1C8D4: virStorageTranslateDiskSourcePool (storage_driver.c:3475)
==15724==    by 0x1B4396E4: qemuProcessPrepareDomain (qemu_process.c:4896)
==15724==    by 0x1B43B880: qemuProcessStart (qemu_process.c:5466)
==15724==    by 0x1B48993E: qemuDomainObjStart (qemu_driver.c:7050)
==15724==    by 0x1B489B9A: qemuDomainCreateWithFlags (qemu_driver.c:7104)
==15724==    by 0x1B489C01: qemuDomainCreate (qemu_driver.c:7122)
==15724==    by 0x561CA97: virDomainCreate (libvirt-domain.c:6787)
==15724==    by 0x12B6FD: remoteDispatchDomainCreate (remote_dispatch.h:4116)
==15724==    by 0x12B61A: remoteDispatchDomainCreateHelper (remote_dispatch.h:4092)

The problem is, in virStorageTranslateDiskSourcePool disk
def->src->auth is freed, but the pointer is not set to NULL. So
later, when qemuProcessStop starts to free the domain definition,
virStorageAuthDefFree() tries to free the memory again, instead
of jumping out immediately.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 831e6b04ecab943584ae6fe745dffeaf476411a3..cb9d5784d6b558bf6a019184a8b459218dd93dda 100644 (file)
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -3472,7 +3472,10 @@ virStorageTranslateDiskSourcePool(virConnectPtr conn,
 
     VIR_FREE(def->src->path);
     virStorageNetHostDefFree(def->src->nhosts, def->src->hosts);
+    def->src->nhosts = 0;
+    def->src->hosts = NULL;
     virStorageAuthDefFree(def->src->auth);
+    def->src->auth = NULL;
 
     switch ((virStoragePoolType) pooldef->type) {
     case VIR_STORAGE_POOL_DIR: