Bug 1011330

Summary: Some memory leaks in libvirtd.
Product: Red Hat Enterprise Linux 7 Reporter: Hao Liu <hliu>
Component: libvirtAssignee: Jiri Denemark <jdenemar>
Status: CLOSED CURRENTRELEASE QA Contact: Virtualization Bugs <virt-bugs>
Severity: medium Docs Contact:
Priority: medium    
Version: 7.0CC: acathrow, ajia, bili, dyuan, hliu, jdenemar, jiahu, lsu, zhwang
Target Milestone: rc   
Target Release: ---   
Hardware: x86_64   
OS: Linux   
Whiteboard:
Fixed In Version: libvirt-1.1.1-8.el7 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2014-06-13 12:52:56 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description Hao Liu 2013-09-24 06:09:03 UTC
Description of problem:
There are some memory leaks in libvirtd daemon.
                       
Version-Release number of selected component:
libvirt-1.1.1-6.el7.x86_64

Steps to reproduce:
1. Stop libvirtd daemon:
# service libvirtd stop                  

2. Start libvirtd memory leak checking in terminal 1:
# valgrind --leak-check=full libvirtd

3. Run some virsh commands in terminal 2:
# cat domain.xml                               
<domain type='qemu'>
  <name>domain</name>
  <memory unit='MiB'>32</memory>
  <os> 
    <type arch='x86_64' machine='pc'>hvm</type>
  </os>
</domain>

# virsh create domain.xml
# virsh destroy domain
# virsh create domain.xml

4. Interrupt libvirtd daemon in terminal 1:
Press Ctrl+C 

Results:
a)
Leak:
==14967== 72 bytes in 1 blocks are definitely lost in loss record 676 of 1,265
==14967==    at 0x4A08121: calloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==14967==    by 0x5081A84: virAllocVar (viralloc.c:544)
==14967==    by 0x50B43C5: virObjectNew (virobject.c:190)
==14967==    by 0x50B4783: virObjectLockableNew (virobject.c:216)
==14967==    by 0x1BC564E0: qemuStateInitialize (qemu_driver.c:703)
==14967==    by 0x5135539: virStateInitialize (libvirt.c:834)
==14967==    by 0x11C1CA: daemonRunStateInit (libvirtd.c:906)
==14967==    by 0x50C41B0: virThreadHelper (virthreadpthread.c:161)
==14967==    by 0x775ADE2: start_thread (pthread_create.c:308)
==14967==    by 0x7E6D0DC: clone (clone.S:113)

==14967== 7,576 (72 direct, 7,504 indirect) bytes in 1 blocks are definitely lost in loss record 1,233 of 1,265
==14967==    at 0x4A08121: calloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==14967==    by 0x5081A84: virAllocVar (viralloc.c:544)
==14967==    by 0x50B43C5: virObjectNew (virobject.c:190)
==14967==    by 0x50B4783: virObjectLockableNew (virobject.c:216)
==14967==    by 0x50BA204: virPortAllocatorNew (virportallocator.c:82)
==14967==    by 0x1BC561D5: qemuStateInitialize (qemu_driver.c:687)
==14967==    by 0x5135539: virStateInitialize (libvirt.c:834)
==14967==    by 0x11C1CA: daemonRunStateInit (libvirtd.c:906)
==14967==    by 0x50C41B0: virThreadHelper (virthreadpthread.c:161)
==14967==    by 0x775ADE2: start_thread (pthread_create.c:308)
==14967==    by 0x7E6D0DC: clone (clone.S:113)

Cause:
webSocketPorts and activeScsiHostdevs need to be freed in qemuStateCleanup() src/qemu/qemu_driver.c

b)
Leak:
==18476== 968 bytes in 1 blocks are definitely lost in loss record 1,640 of 1,823
==18476==    at 0x4A06409: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==18476==    by 0x6273CE1: xmlGetGlobalState (threads.c:583)
==18476==    by 0x62735B4: __xmlKeepBlanksDefaultValue (globals.c:955)
==18476==    by 0x6201F6C: xmlKeepBlanksDefault (parserInternals.c:2119)
==18476==    by 0x50F7848: virDomainDefParse (domain_conf.c:12529)
==18476==    by 0x1BC6AA67: qemuDomainCreateXML (qemu_driver.c:1577)
==18476==    by 0x5136A46: virDomainCreateXML (libvirt.c:2008)
==18476==    by 0x13CE9E: remoteDispatchDomainCreateXMLHelper (remote_dispatch.h:3016)
==18476==    by 0x519FD59: virNetServerProgramDispatch (virnetserverprogram.c:435)
==18476==    by 0x519ABC7: virNetServerHandleJob (virnetserver.c:165)
==18476==    by 0x50C48F4: virThreadPoolWorker (virthreadpool.c:144)
==18476==    by 0x50C4370: virThreadHelper (virthreadpthread.c:161)

Cause:
Nullified def before freed in qemuDomainCreateXML() /src/qemu/qemu_driver.c
This also appears in qemuDomainRestoreFlags() qemuDomainObjRestore()
qemuDomainDefineXML() qemuDomainSnapshotCreateXML() qemuDomainQemuAttach()
Some of these might influence RHEL-6.5.

c)
Leak:
==26583== 4 bytes in 2 blocks are definitely lost in loss record 15 of 424
==26583==    at 0x4A06409: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==26583==    by 0x70263AF: __vasprintf_chk (vasprintf_chk.c:80)
==26583==    by 0x50B3285: virVasprintfInternal (stdio2.h:210)
==26583==    by 0x50B337A: virAsprintfInternal (virstring.c:358)
==26583==    by 0x51A7A11: virNetServerClientGetIdentity (virnetserverclient.c:677)
==26583==    by 0x51A94ED: virNetServerProgramDispatch (virnetserverprogram.c:419)
==26583==    by 0x51A45A7: virNetServerHandleJob (virnetserver.c:165)
==26583==    by 0x50B7464: virThreadPoolWorker (virthreadpool.c:144)
==26583==    by 0x50B6ABD: virThreadHelper (virthreadpthread.c:161)
==26583==    by 0x68FCDE2: start_thread (pthread_create.c:308)
==26583==    by 0x700F0DC: clone (clone.S:113)

Cause:
A typo. s/userid/groupid/ in virNetServerClientCreateIdentity() in src/rpc/virnetserverclient.c

d)
Leak:
==26583== 709 (336 direct, 373 indirect) bytes in 1 blocks are definitely lost in loss record 388 of 424
==26583==    at 0x4A08121: calloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==26583==    by 0x506D439: virAllocVar (viralloc.c:544)
==26583==    by 0x50A4905: virObjectNew (virobject.c:190)
==26583==    by 0x191E7B6E: virQEMUDriverConfigNew (qemu_conf.c:98)
==26583==    by 0x192215E4: qemuStateInitialize (qemu_driver.c:609)
==26583==    by 0x5134BA9: virStateInitialize (libvirt.c:834)
==26583==    by 0x11C25A: daemonRunStateInit (libvirtd.c:906)
==26583==    by 0x50B6ABD: virThreadHelper (virthreadpthread.c:161)
==26583==    by 0x68FCDE2: start_thread (pthread_create.c:308)
==26583==    by 0x700F0DC: clone (clone.S:113)

Cause:
Re-reference cfg without unreference at qemuProcessStart() in src/qemu/qemu_process.c:
hookData.cfg = virObjectRef(cfg);

Comment 2 Hao Liu 2013-09-24 08:39:10 UTC
Leak c) have already been fixed with commit e4697b9 in upstream.

https://www.redhat.com/archives/libvir-list/2013-September/msg01245.html

Comment 3 Jiri Denemark 2013-09-27 13:53:26 UTC
b) is not a memory leak as the domain is still running (def = NULL is correct, the pointer was stolen by virDomainObjListAdd(). I sent patches for a) and d).

Comment 4 Jiri Denemark 2013-09-27 13:59:27 UTC
The leaks are now fixed upstream as of v1.1.3-rc1-13-g9e03f31 by the following commits:

commit e4697b92abaad16e8e6b41a1e55be9b084d48d5a
Author: Daniel P. Berrange <berrange>
Date:   Mon Sep 23 12:46:25 2013 +0100

    Fix typo in identity code which is pre-requisite for CVE-2013-4311
    
    The fix for CVE-2013-4311 had a pre-requisite enhancement
    to the identity code
    
      commit db7a5688c05f3fd60d9d2b74c72427eb9ee9c176
      Author: Daniel P. Berrange <berrange>
      Date:   Thu Aug 22 16:00:01 2013 +0100
    
        Also store user & group ID values in virIdentity
    
    This had a typo which caused the group ID to overwrite the
    user ID string. This meant any checks using this would have
    the wrong ID value. This only affected the ACL code, not the
    initial polkit auth. It also leaked memory.
    
    Signed-off-by: Daniel P. Berrange <berrange>

commit 833cdab6d2ad7521c954948adf3c7d3c3b42ae9f
Author: Jiri Denemark <jdenemar>
Date:   Fri Sep 27 15:07:38 2013 +0200

    qemu: Don't leak reference to virQEMUDriverConfigPtr
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1011330 (case D)
    
    qemuProcessStart created two references to virQEMUDriverConfigPtr before
    calling fork():
    
        cfg = virQEMUDriverGetConfig(driver);
        ...
        hookData.cfg = virObjectRef(cfg);
    
    However, the child only unreferenced hookData.cfg and the parent only
    removed the cfg reference. That said, we don't need to increment the
    reference counter when assigning cfg to hookData. Both the child and the
    parent will correctly remove the reference on cfg (the child will do
    that through hookData).
    
    Signed-off-by: Jiri Denemark <jdenemar>

commit 9e03f313b80d6b7daef90235b607e7ed80dd7235
Author: Jiri Denemark <jdenemar>
Date:   Fri Sep 27 15:34:43 2013 +0200

    qemu: Free all driver data in qemuStateCleanup
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1011330 (case A)
    
    While activeScsiHostdevs and webSocketPorts were allocated in
    qemuStateInitialize, they were not freed in qemuStateCleanup.
    
    Signed-off-by: Jiri Denemark <jdenemar>

Comment 6 Hu Jianwei 2013-09-29 02:23:26 UTC
Hi All,

For one memory leak issue below, I have verified using patch http://post-office.corp.redhat.com/archives/rhvirt-patches/2013-September/msg00742.html
in https://bugzilla.redhat.com/show_bug.cgi?id=1006272#c6

I can't find below mem leak issue with libvirt-1.1.1-7.el7.x86_64.

Reproduced steps are same as https://bugzilla.redhat.com/show_bug.cgi?id=1011330#c0

[root@localhost ~]# rpm -q libvirt
libvirt-1.1.1-7.el7.x86_64

305 ==9402== 6 bytes in 3 blocks are definitely lost in loss record 137 of 1,775
306 ==9402==    at 0x4A06409: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
307 ==9402==    by 0x7E7C40F: __vasprintf_chk (in /usr/lib64/libc-2.17.so)
308 ==9402==    by 0x50BDE65: virVasprintfInternal (stdio2.h:210)
309 ==9402==    by 0x50BDF4A: virAsprintfInternal (virstring.c:358)
310 ==9402==    by 0x519A9B6: virNetServerClientGetIdentity (virnetserverclient.c:677)
311 ==9402==    by 0x519CD1D: virNetServerProgramDispatch (virnetserverprogram.c:419)
312 ==9402==    by 0x5197BC7: virNetServerHandleJob (virnetserver.c:165)
313 ==9402==    by 0x50C18F4: virThreadPoolWorker (virthreadpool.c:144)
314 ==9402==    by 0x50C1370: virThreadHelper (virthreadpthread.c:161)
315 ==9402==    by 0x7753C52: start_thread (in /usr/lib64/libpthread-2.17.so)
316 ==9402==    by 0x7E6513C: clone (in /usr/lib64/libc-2.17.so)

But, still have a similar leak as below:
464 ==10172== 108 bytes in 2 blocks are definitely lost in loss record 1,107 of 1,777
465 ==10172==    at 0x4A06409: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
466 ==10172==    by 0x7DF6349: strdup (in /usr/lib64/libc-2.17.so)
467 ==10172==    by 0x7539FB4: selinux_raw_to_trans_context (in /usr/lib64/libselinux.so.1)
468 ==10172==    by 0x753A882: getpeercon (in /usr/lib64/libselinux.so.1)
469 ==10172==    by 0x51A3BBD: virNetSocketGetSELinuxContext (virnetsocket.c:1182)
470 ==10172==    by 0x519AC5C: virNetServerClientGetIdentity (virnetserverclient.c:708)
471 ==10172==    by 0x519CDAD: virNetServerProgramDispatch (virnetserverprogram.c:419)
472 ==10172==    by 0x5197C27: virNetServerHandleJob (virnetserver.c:165)
473 ==10172==    by 0x50C18F4: virThreadPoolWorker (virthreadpool.c:144)
474 ==10172==    by 0x50C1370: virThreadHelper (virthreadpthread.c:161)
475 ==10172==    by 0x7753C52: start_thread (in /usr/lib64/libpthread-2.17.so)
476 ==10172==    by 0x7E6513C: clone (in /usr/lib64/libc-2.17.so)

Comment 8 Hao Liu 2013-10-09 06:14:03 UTC
Hi Jiri,

I verified fix of leak A, C, D using
libvirt-1.1.1-8.el7.x86_64

But got following leak along with the one mentioned in
https://bugzilla.redhat.com/show_bug.cgi?id=1011330#c6

==12538== 832 (32 direct, 800 indirect) bytes in 1 blocks are definitely lost in loss record 609 of 646
==12538==    at 0x4A06409: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==12538==    by 0x75456A1: selabel_subs_init (label.c:100)
==12538==    by 0x75413A8: init (label_file.c:499)
==12538==    by 0x7545850: selabel_open (label.c:182)
==12538==    by 0x520B4E8: virSecuritySELinuxSecurityDriverOpen (security_selinux.c:491)
==12538==    by 0x5208033: virSecurityManagerNewDriver (security_manager.c:98)
==12538==    by 0x1BA569EB: qemuStateInitialize (qemu_driver.c:358)
==12538==    by 0x5136779: virStateInitialize (libvirt.c:834)
==12538==    by 0x11C1CA: daemonRunStateInit (libvirtd.c:906)
==12538==    by 0x50C5390: virThreadHelper (virthreadpthread.c:161)
==12538==    by 0x775CDE2: start_thread (pthread_create.c:308)
==12538==    by 0x7E6F1AC: clone (clone.S:113)

Are these need to be fixed?

Comment 9 Jiri Denemark 2013-11-01 11:12:53 UTC
It would be nice if that was fixed but it's not really important. It leaks a memory that is allocated when libvirtd starts and is used all the time the daemon is running. Freeing the memory when the daemon goes down would be nice and clean but not doing so does not actually leak any memory anyway.

Comment 10 Hao Liu 2013-11-04 01:51:38 UTC
Thanks Jiri. Considering all leaks mentioned in original post are fixed and verified, I mark this thread as VERIFIED.

Comment 11 Ludek Smid 2014-06-13 12:52:56 UTC
This request was resolved in Red Hat Enterprise Linux 7.0.

Contact your manager or support representative in case you have further questions about the request.