As a part of parsing XML, virFirewallParseXML() calls virXMLNodeContentString() and then passes the return value further. But virXMLNodeContentString() is documented so that it's the caller's responsibility to free the returned string, which virFirewallParseXML() never does. This leads to a memory leak: 14,300 bytes in 220 blocks are definitely lost in loss record 1,879 of 1,891 at 0x4841858: malloc (vg_replace_malloc.c:442) by 0x5491E3C: xmlBufCreateSize (in /usr/lib64/libxml2.so.2.12.6) by 0x54C2401: xmlNodeGetContent (in /usr/lib64/libxml2.so.2.12.6) by 0x49F7791: virXMLNodeContentString (virxml.c:354) by 0x4979F25: virFirewallParseXML (virfirewall.c:1134) by 0x4ABEB1E: virNetworkLoadState (virnetworkobj.c:938) by 0x4ABF35C: virNetworkObjLoadAllState (virnetworkobj.c:1072) by 0x4E9A25E: networkStateInitialize (bridge_driver.c:624) by 0x4CB1FA6: virStateInitialize (libvirt.c:665) by 0x15A6C6: daemonRunStateInit (remote_daemon.c:611) by 0x49E69F0: virThreadHelper (virthread.c:256) by 0x532B428: start_thread (in /lib64/libc.so.6) Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/util/virfirewall.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c index 2219506b18..ae81ca52c6 100644 --- a/src/util/virfirewall.c +++ b/src/util/virfirewall.c @@ -1131,7 +1131,7 @@ virFirewallParseXML(virFirewall **firewall, NULL, NULL, NULL); for (i = 0; i < nargs; i++) { - char *arg = virXMLNodeContentString(argsNodes[i]); + g_autofree char *arg = virXMLNodeContentString(argsNodes[i]); if (!arg) return -1; -- 2.44.2

The virNetworkObjSetFwRemoval() function is called at least two times when there's a network running and network driver initializes: 1) when loading state XMLs: #0 virNetworkObjSetFwRemoval (obj=0x7fffd4028250, fwRemoval=0x7fffd4020ad0) at ../src/conf/virnetworkobj.c:258 #1 0x00007ffff7a69c68 in virNetworkLoadState (...) at ../src/conf/virnetworkobj.c:952 #2 0x00007ffff7a6a35d in virNetworkObjLoadAllState (...) at ../src/conf/virnetworkobj.c:1072 #3 0x00007ffff7f9625f in networkStateInitialize (...) at ../src/network/bridge_driver.c:624 2) when firewall rules are being reloaded: #0 virNetworkObjSetFwRemoval (obj=0x7fffd4028250, fwRemoval=0x7fffd402e5b0) at ../src/conf/virnetworkobj.c:258 #1 0x00007ffff7f997b4 in networkReloadFirewallRulesHelper (obj=0x7fffd4028250, opaque=0x0) at ../src/network/bridge_driver.c:1703 #2 0x00007ffff7a6b09b in virNetworkObjListForEachHelper (payload=0x7fffd4028250, ...) at ../src/conf/virnetworkobj.c:1414 #3 0x00007ffff79287b6 in virHashForEachSafe (...) at ../src/util/virhash.c:387 #4 0x00007ffff7a6b119 in virNetworkObjListForEach (...) at ../src/conf/virnetworkobj.c:1441 #5 0x00007ffff7f99978 in networkReloadFirewallRules (...) at ../src/network/bridge_driver.c:1742 #6 0x00007ffff7f962f2 in networkStateInitialize (...) at ../src/network/bridge_driver.c:645 Since virNetworkObjSetFwRemoval() does not free the object stored in the first call, the second call just overwrites the stored pointer leading to a memory leak: 5,530 (48 direct, 5,482 indirect) bytes in 1 blocks are definitely lost in loss record 1,863 of 1,880 at 0x4848C43: calloc (vg_replace_malloc.c:1595) by 0x4F1E979: g_malloc0 (in /usr/lib64/libglib-2.0.so.0.7800.6) by 0x4976E32: virFirewallNew (virfirewall.c:118) by 0x4979BA9: virFirewallParseXML (virfirewall.c:1071) by 0x4ABEB1E: virNetworkLoadState (virnetworkobj.c:938) by 0x4ABF35C: virNetworkObjLoadAllState (virnetworkobj.c:1072) by 0x4E9A25E: networkStateInitialize (bridge_driver.c:624) by 0x4CB1FA6: virStateInitialize (libvirt.c:665) by 0x15A6C6: daemonRunStateInit (remote_daemon.c:611) by 0x49E69F0: virThreadHelper (virthread.c:256) by 0x532B428: start_thread (in /lib64/libc.so.6) by 0x5397373: clone (in /lib64/libc.so.6) Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/conf/virnetworkobj.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c index 19305798cb..5abc295506 100644 --- a/src/conf/virnetworkobj.c +++ b/src/conf/virnetworkobj.c @@ -255,6 +255,7 @@ void virNetworkObjSetFwRemoval(virNetworkObj *obj, virFirewall *fwRemoval) { + virFirewallFree(obj->fwRemoval); obj->fwRemoval = fwRemoval; /* give it a name so it's identifiable in the XML */ if (fwRemoval) -- 2.44.2

In ideal world, where clients close connection gracefully their SASL session is freed in virNetServerClientDispose() as it's stored in client->sasl. Unfortunately, if client connection is closed prematurely (e.g. the moment virsh asks for credentials), the _virNetServerClient member is never set and corresponding SASL session is never freed. The handler is still stored in client private data, so free it in remoteClientCloseFunc(). 20,862 (288 direct, 20,574 indirect) bytes in 3 blocks are definitely lost in loss record 1,763 of 1,772 at 0x50390C4: g_type_create_instance (in /usr/lib64/libgobject-2.0.so.0.7800.6) by 0x501BDAF: g_object_new_internal.part.0 (in /usr/lib64/libgobject-2.0.so.0.7800.6) by 0x501D43D: g_object_new_with_properties (in /usr/lib64/libgobject-2.0.so.0.7800.6) by 0x501E318: g_object_new (in /usr/lib64/libgobject-2.0.so.0.7800.6) by 0x49BAA63: virObjectNew (virobject.c:252) by 0x49BABC6: virObjectLockableNew (virobject.c:274) by 0x4B0526C: virNetSASLSessionNewServer (virnetsaslcontext.c:230) by 0x18EEFC: remoteDispatchAuthSaslInit (remote_daemon_dispatch.c:3696) by 0x15E128: remoteDispatchAuthSaslInitHelper (remote_daemon_dispatch_stubs.h:74) by 0x4B0FA5E: virNetServerProgramDispatchCall (virnetserverprogram.c:423) by 0x4B0F591: virNetServerProgramDispatch (virnetserverprogram.c:299) by 0x4B18AE3: virNetServerProcessMsg (virnetserver.c:135) Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/remote/remote_daemon_dispatch.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c index cfc1067e40..c88c9f5b15 100644 --- a/src/remote/remote_daemon_dispatch.c +++ b/src/remote/remote_daemon_dispatch.c @@ -1776,6 +1776,10 @@ static void remoteClientCloseFunc(virNetServerClient *client) daemonRemoveAllClientStreams(priv->streams); remoteClientFreePrivateCallbacks(priv); + +#if WITH_SASL + virObjectUnref(priv->sasl); +#endif } -- 2.44.2