From: Michal Privoznik Date: Mon, 16 Jun 2025 10:05:24 +0000 (+0200) Subject: security_manager: Don't leak seclabel in virSecurityManagerGenLabel() X-Git-Tag: v11.5.0-rc1~30 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=be04898d18c8b084130fbee4fc2d885154b4e2ed;p=thirdparty%2Flibvirt.git 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 Reviewed-by: Ján Tomko --- 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; }