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

On Fri, Jun 14, 2024 at 03:06:59PM +0200, Michal Privoznik wrote:
> 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(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|