[libvirt] [PATCH 0/2] Two small memleak fixes
*** BLURB HERE *** Michal Privoznik (2): virSysinfoParseX86Chassis: Store asset tag into correct pointer virSecurityDACChownListFree: Don't leak list->items array src/security/security_dac.c | 1 + src/util/virsysinfo.c | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) -- 2.16.1
Probably due to copy-paste error we're storing asset tag into def->sku which we even use in the next step to store SKU number and thus the asset tag leaks. ==19200== 41 bytes in 1 blocks are definitely lost in loss record 849 of 1,059 ==19200== at 0x4C2AF0F: malloc (vg_replace_malloc.c:299) ==19200== by 0x8785419: strndup (in /lib64/libc-2.25.so) ==19200== by 0x53588D7: virStrndup (virstring.c:1023) ==19200== by 0x535C6CF: virSysinfoParseX86Chassis (virsysinfo.c:882) ==19200== by 0x535DA67: virSysinfoReadX86 (virsysinfo.c:1149) ==19200== by 0x535DB3A: virSysinfoRead (virsysinfo.c:1206) ==19200== by 0x2BE0B27E: qemuStateInitialize (qemu_driver.c:641) ==19200== by 0x556AB4F: virStateInitialize (libvirt.c:770) ==19200== by 0x1299B9: daemonRunStateInit (remote_daemon.c:846) ==19200== by 0x5361A58: virThreadHelper (virthread.c:206) ==19200== by 0x84E3896: start_thread (in /lib64/libpthread-2.25.so) ==19200== by 0x87F4CFE: clone (in /lib64/libc-2.25.so) Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/util/virsysinfo.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/virsysinfo.c b/src/util/virsysinfo.c index 137f14ae4..43a4c8835 100644 --- a/src/util/virsysinfo.c +++ b/src/util/virsysinfo.c @@ -879,7 +879,7 @@ virSysinfoParseX86Chassis(const char *base, if ((cur = strstr(base, "Asset Tag: ")) != NULL) { cur += 11; eol = strchr(cur, '\n'); - if (eol && VIR_STRNDUP(def->sku, cur, eol - cur) < 0) + if (eol && VIR_STRNDUP(def->asset, cur, eol - cur) < 0) goto cleanup; } if ((cur = strstr(base, "SKU Number: ")) != NULL) { -- 2.16.1
On Tue, Mar 13, 2018 at 14:37:04 +0100, Michal Privoznik wrote:
Probably due to copy-paste error we're storing asset tag into def->sku which we even use in the next step to store SKU number and thus the asset tag leaks.
==19200== 41 bytes in 1 blocks are definitely lost in loss record 849 of 1,059 ==19200== at 0x4C2AF0F: malloc (vg_replace_malloc.c:299) ==19200== by 0x8785419: strndup (in /lib64/libc-2.25.so) ==19200== by 0x53588D7: virStrndup (virstring.c:1023) ==19200== by 0x535C6CF: virSysinfoParseX86Chassis (virsysinfo.c:882) ==19200== by 0x535DA67: virSysinfoReadX86 (virsysinfo.c:1149) ==19200== by 0x535DB3A: virSysinfoRead (virsysinfo.c:1206) ==19200== by 0x2BE0B27E: qemuStateInitialize (qemu_driver.c:641) ==19200== by 0x556AB4F: virStateInitialize (libvirt.c:770) ==19200== by 0x1299B9: daemonRunStateInit (remote_daemon.c:846) ==19200== by 0x5361A58: virThreadHelper (virthread.c:206) ==19200== by 0x84E3896: start_thread (in /lib64/libpthread-2.25.so) ==19200== by 0x87F4CFE: clone (in /lib64/libc-2.25.so)
This backtrace is not really relevant IMO, since the problem is that we'd not parse the asset tag properly. The memory leak is only a side effect of that bug. ACK if you fix the commit message.
We're freeing individual items in it but not the array itself. ==19200== 40 bytes in 1 blocks are definitely lost in loss record 847 of 1,059 ==19200== at 0x4C2D12F: realloc (vg_replace_malloc.c:785) ==19200== by 0x52C5532: virReallocN (viralloc.c:245) ==19200== by 0x52C5628: virExpandN (viralloc.c:294) ==19200== by 0x52C58FC: virInsertElementsN (viralloc.c:436) ==19200== by 0x542856B: virSecurityDACChownListAppend (security_dac.c:115) ==19200== by 0x54286B4: virSecurityDACTransactionAppend (security_dac.c:167) ==19200== by 0x542902F: virSecurityDACSetOwnershipInternal (security_dac.c:560) ==19200== by 0x54295D6: virSecurityDACSetOwnership (security_dac.c:650) ==19200== by 0x542AEE0: virSecurityDACSetInputLabel (security_dac.c:1472) ==19200== by 0x542B61D: virSecurityDACSetAllLabel (security_dac.c:1693) ==19200== by 0x542DD67: virSecurityManagerSetAllLabel (security_manager.c:869) ==19200== by 0x54279C2: virSecurityStackSetAllLabel (security_stack.c:361) Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/security/security_dac.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 74446d664..663c8c9ea 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -137,6 +137,7 @@ virSecurityDACChownListFree(void *opaque) VIR_FREE(list->items[i]->path); VIR_FREE(list->items[i]); } + VIR_FREE(list->items); VIR_FREE(list); } -- 2.16.1
On Tue, Mar 13, 2018 at 14:37:05 +0100, Michal Privoznik wrote:
We're freeing individual items in it but not the array itself.
==19200== 40 bytes in 1 blocks are definitely lost in loss record 847 of 1,059 ==19200== at 0x4C2D12F: realloc (vg_replace_malloc.c:785) ==19200== by 0x52C5532: virReallocN (viralloc.c:245) ==19200== by 0x52C5628: virExpandN (viralloc.c:294) ==19200== by 0x52C58FC: virInsertElementsN (viralloc.c:436) ==19200== by 0x542856B: virSecurityDACChownListAppend (security_dac.c:115) ==19200== by 0x54286B4: virSecurityDACTransactionAppend (security_dac.c:167) ==19200== by 0x542902F: virSecurityDACSetOwnershipInternal (security_dac.c:560) ==19200== by 0x54295D6: virSecurityDACSetOwnership (security_dac.c:650) ==19200== by 0x542AEE0: virSecurityDACSetInputLabel (security_dac.c:1472) ==19200== by 0x542B61D: virSecurityDACSetAllLabel (security_dac.c:1693) ==19200== by 0x542DD67: virSecurityManagerSetAllLabel (security_manager.c:869) ==19200== by 0x54279C2: virSecurityStackSetAllLabel (security_stack.c:361)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/security/security_dac.c | 1 + 1 file changed, 1 insertion(+)
ACK
participants (2)
-
Michal Privoznik -
Peter Krempa