network: fix memory leak in networkAddFirewallRules()

[PATCH] network: fix memory leak in networkAddFirewallRules()

Posted by Elizaveta Tereshkina 1 month, 3 weeks ago

Add g_clear_pointer() to networkAddFirewallRules()
after using virSetError() to free errInitV4 and errInitV6
to avoid memory leaks.

Fixes: ef760a4133 (Revert "network: support setting firewalld zone for bridge device of open networks")
Signed-off-by: Elizaveta Tereshkina <etereshkina@astralinux.ru>
---
 src/network/bridge_driver_linux.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/network/bridge_driver_linux.c b/src/network/bridge_driver_linux.c
index 86f6a5915f..93d4d2c8ee 100644
--- a/src/network/bridge_driver_linux.c
+++ b/src/network/bridge_driver_linux.c
@@ -413,6 +413,7 @@ networkAddFirewallRules(virNetworkDef *def,
             (virNetworkDefGetIPByIndex(def, AF_INET, 0) ||
              virNetworkDefGetRouteByIndex(def, AF_INET, 0))) {
             virSetError(errInitV4);
+            g_clear_pointer(&errInitV4, virFreeError);
             return -1;
         }
 
@@ -421,6 +422,7 @@ networkAddFirewallRules(virNetworkDef *def,
              virNetworkDefGetRouteByIndex(def, AF_INET6, 0) ||
              def->ipv6nogw)) {
             virSetError(errInitV6);
+            g_clear_pointer(&errInitV6, virFreeError);
             return -1;
         }
 
-- 
2.39.2

Re: [PATCH] network: fix memory leak in networkAddFirewallRules()

Posted by Daniel P. Berrangé via Devel 1 month, 3 weeks ago

On Thu, Jul 17, 2025 at 04:49:21PM +0300, Elizaveta Tereshkina wrote:
> Add g_clear_pointer() to networkAddFirewallRules()
> after using virSetError() to free errInitV4 and errInitV6
> to avoid memory leaks.

There is no memory leak. These are global variables that are intended
to remain allocated for the whole lifetime of the process.

> 
> Fixes: ef760a4133 (Revert "network: support setting firewalld zone for bridge device of open networks")
> Signed-off-by: Elizaveta Tereshkina <etereshkina@astralinux.ru>
> ---
>  src/network/bridge_driver_linux.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/src/network/bridge_driver_linux.c b/src/network/bridge_driver_linux.c
> index 86f6a5915f..93d4d2c8ee 100644
> --- a/src/network/bridge_driver_linux.c
> +++ b/src/network/bridge_driver_linux.c
> @@ -413,6 +413,7 @@ networkAddFirewallRules(virNetworkDef *def,
>              (virNetworkDefGetIPByIndex(def, AF_INET, 0) ||
>               virNetworkDefGetRouteByIndex(def, AF_INET, 0))) {
>              virSetError(errInitV4);
> +            g_clear_pointer(&errInitV4, virFreeError);
>              return -1;
>          }
>  
> @@ -421,6 +422,7 @@ networkAddFirewallRules(virNetworkDef *def,
>               virNetworkDefGetRouteByIndex(def, AF_INET6, 0) ||
>               def->ipv6nogw)) {
>              virSetError(errInitV6);
> +            g_clear_pointer(&errInitV6, virFreeError);
>              return -1;
>          }

No, this breaks the code. networkAddFirewallRules can be called many
times and freeing errInitV4/6 will break this.

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 :|