Changeset
src/security/security_dac.c | 1 +
src/util/virsysinfo.c       | 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)
Git apply log
Switched to a new branch 'cover.1520948163.git.mprivozn@redhat.com'
Applying: virSysinfoParseX86Chassis: Store asset tag into correct pointer
Applying: virSecurityDACChownListFree: Don't leak list->items array
To https://github.com/patchew-project/libvirt
 * [new tag]         patchew/cover.1520948163.git.mprivozn@redhat.com -> patchew/cover.1520948163.git.mprivozn@redhat.com
Test passed: syntax-check

loading

[libvirt] [PATCH 0/2] Two small memleak fixes
Posted by Michal Privoznik, 14 weeks ago
*** 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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] virSysinfoParseX86Chassis: Store asset tag into correct pointer
Posted by Michal Privoznik, 14 weeks ago
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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] virSysinfoParseX86Chassis: Store asset tag into correct pointer
Posted by Peter Krempa, 14 weeks ago
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.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] virSecurityDACChownListFree: Don't leak list->items array
Posted by Michal Privoznik, 14 weeks ago
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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] virSecurityDACChownListFree: Don't leak list->items array
Posted by Peter Krempa, 14 weeks ago
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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list