From: Michal Privoznik <mpriv...@redhat.com>

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 <mpriv...@redhat.com>
---
 src/security/security_manager.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/security/security_manager.c b/src/security/security_manager.c
index c2460eae37..5fc4eb4872 100644
--- 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;
             }
 
-- 
2.49.0

Reply via email to