[PATCH v1 30/31] qemu: Don't leak storage perms on failure in qemuDomainAttachDiskGeneric

[Michal Privoznik <mprivozn@xxxxxxxxxx>]

At the very beginning of the attach function the
qemuDomainStorageSourceChainAccessAllow() is called which
modifies CGroups, locks and seclabels for new disk and its
backing chain. This must be followed by a counterpart which
reverts back all the changes if something goes wrong. This boils
down to calling qemuDomainStorageSourceChainAccessRevoke() which
is done under 'error' label. But not all failure branches jump
there. They just jump onto 'cleanup' label where no revoke is
done. Such mistake is easy to do because 'cleanup' label does
exist. Therefore, dissolve 'error' block in 'cleanup' and have
everything jump onto 'cleanup' label.

Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
---
 src/qemu/qemu_hotplug.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 7e9c1a1649..3c6c0da3a0 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -624,13 +624,13 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver,
     VIR_AUTOFREE(char *) corAlias = NULL;
 
     if (qemuDomainStorageSourceChainAccessAllow(driver, vm, disk->src) < 0)
-        goto cleanup;
+        return -1;
 
     if (qemuAssignDeviceDiskAlias(vm->def, disk, priv->qemuCaps) < 0)
-        goto error;
+        goto cleanup;
 
     if (qemuDomainPrepareDiskSource(disk, priv, cfg) < 0)
-        goto error;
+        goto cleanup;
 
     if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV)) {
         if (disk->copy_on_read == VIR_TRISTATE_SWITCH_ON &&
@@ -647,13 +647,13 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver,
     }
 
     if (!(devstr = qemuBuildDiskDeviceStr(vm->def, disk, 0, priv->qemuCaps)))
-        goto error;
+        goto cleanup;
 
     if (VIR_REALLOC_N(vm->def->disks, vm->def->ndisks + 1) < 0)
-        goto error;
+        goto cleanup;
 
     if (qemuHotplugAttachManagedPR(driver, vm, disk->src, QEMU_ASYNC_JOB_NONE) < 0)
-        goto error;
+        goto cleanup;
 
     qemuDomainObjEnterMonitor(driver, vm);
 
@@ -674,7 +674,7 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver,
 
     if (qemuDomainObjExitMonitor(driver, vm) < 0) {
         ret = -2;
-        goto error;
+        goto cleanup;
     }
 
     virDomainAuditDisk(vm, NULL, disk->src, "attach", true);
@@ -683,6 +683,8 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver,
     ret = 0;
 
  cleanup:
+    if (ret < 0)
+        ignore_value(qemuDomainStorageSourceChainAccessRevoke(driver, vm, disk->src));
     qemuDomainSecretDiskDestroy(disk);
     VIR_FREE(devstr);
     return ret;
@@ -700,9 +702,6 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver,
         ret = -2;
 
     virDomainAuditDisk(vm, NULL, disk->src, "attach", false);
-
- error:
-    ignore_value(qemuDomainStorageSourceChainAccessRevoke(driver, vm, disk->src));
     goto cleanup;
 }
 
-- 
2.21.0

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list