security_manager: Don't leak seclabel in virSecurityManagerGenLabel()

When a domain is being started, seclabels are generated for it.
This is handled in virSecurityManagerGenLabel() which can either
find pre-existing seclabel in domain def or generate a new one.
At any rate, domainGenSecurityLabel() callback is called and if
it fails then the seclabel is removed from domain definition
using VIR_DELETE_ELEMENT(). While this shrinks down the seclabels
array, it does not free individual item. It has to be freed
manually.

80 bytes in 2 blocks are definitely lost in loss record 1,359 of 1,876
   at 0x484CEF3: calloc (vg_replace_malloc.c:1675)
   by 0x4F19B29: g_malloc0 (in /usr/lib64/libglib-2.0.so.0.8200.5)
   by 0x49E4953: virSecurityLabelDefNew (virseclabel.c:59)
   by 0x4BDE0A4: virSecurityManagerGenLabel (security_manager.c:638)
   by 0xBA029B7: qemuProcessPrepareDomain (qemu_process.c:6760)
   by 0xBA07DF2: qemuProcessStart (qemu_process.c:8369)
   by 0xB93DAC0: qemuDomainObjStart (qemu_driver.c:6371)
   by 0xB93DE08: qemuDomainCreateWithFlags (qemu_driver.c:6420)
   by 0xB93DE86: qemuDomainCreate (qemu_driver.c:6438)
   by 0x4CECEA8: virDomainCreate (libvirt-domain.c:7142)

Now, you might think this may lead to a double free, because
@seclabel is freed under the 'cleanup' label (if @generated is
true). But if @generated is true, then just before calling the
callback there's VIR_APPEND_ELEMENT() which clears @seclabel out
turning the free under 'cleanup' label into a NOP.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

diff --git a/src/security/security_manager.c b/src/security/security_manager.c
index c2460eae379d23a9b8af1bdc879f3b0286f22995..5fc4eb4872719ac9852d705c3fc110794e72f72c 100644 (file)
--- a/src/security/security_manager.c
+++ b/src/security/security_manager.c
@@ -669,9 +669,14 @@ virSecurityManagerGenLabel(virSecurityManager *mgr,
                 VIR_APPEND_ELEMENT(vm->seclabels, vm->nseclabels, seclabel);
 
             if (sec_managers[i]->drv->domainGenSecurityLabel(sec_managers[i], vm) < 0) {
+                virSecurityLabelDef *tmp = vm->seclabels[vm->nseclabels - 1];
+
                 if (VIR_DELETE_ELEMENT(vm->seclabels,
-                                       vm->nseclabels -1, vm->nseclabels) < 0)
+                                       vm->nseclabels - 1, vm->nseclabels) < 0) {
                     vm->nseclabels--;
+                }
+
+                virSecurityLabelDefFree(tmp);
                 goto cleanup;
             }