[v2] lxc: fix variable storage order before call

[PATCH v2] lxc: fix variable storage order before call

Posted by Adam Julis 1 year, 3 months ago

virDomainConfNWFilterInstantiate() was called without updated
net->ifname, it caused in some cases throwing error message. If
function failed, change is reverted.

Resolves: https://gitlab.com/libvirt/libvirt/-/issues/658
Signed-off-by: Adam Julis <ajulis@redhat.com>
---
 src/lxc/lxc_process.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index 205ab96ebb..0233d17f4e 100644
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -271,6 +271,7 @@ virLXCProcessSetupInterfaceTap(virDomainDef *vm,
 {
     g_autofree char *parentVeth = NULL;
     g_autofree char *containerVeth = NULL;
+    g_autofree char *backupIfname = NULL;
     const virNetDevVPortProfile *vport = virDomainNetGetActualVirtPortProfile(net);
 
     VIR_DEBUG("calling vethCreate()");
@@ -315,14 +316,17 @@ virLXCProcessSetupInterfaceTap(virDomainDef *vm,
             return NULL;
     }
 
-    if (net->filter &&
-        virDomainConfNWFilterInstantiate(vm->name, vm->uuid, net, false) < 0)
-        return NULL;
-
-    /* success is guaranteed, so update the interface object */
-    g_free(net->ifname);
+    /* success almost guaranteed, next function needs updated net->ifname */
+    backupIfname = g_steal_pointer(net->ifname);
     net->ifname = g_steal_pointer(&parentVeth);
 
+    if (net->filter &&
+        virDomainConfNWFilterInstantiate(vm->name, vm->uuid, net, false) < 0) {
+        g_free(net->ifname);
+        net->ifname = g_steal_pointer(&backupIfname);
+        return NULL;
+    }
+
     return g_steal_pointer(&containerVeth);
 }
 
-- 
2.45.2

Re: [PATCH v2] lxc: fix variable storage order before call

Posted by Jiri Denemark 1 year, 3 months ago

On Tue, Oct 15, 2024 at 11:51:38 +0200, Adam Julis wrote:
> virDomainConfNWFilterInstantiate() was called without updated
> net->ifname, it caused in some cases throwing error message. If
> function failed, change is reverted.
> 
> Resolves: https://gitlab.com/libvirt/libvirt/-/issues/658
> Signed-off-by: Adam Julis <ajulis@redhat.com>
> ---
>  src/lxc/lxc_process.c | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
> index 205ab96ebb..0233d17f4e 100644
> --- a/src/lxc/lxc_process.c
> +++ b/src/lxc/lxc_process.c
> @@ -271,6 +271,7 @@ virLXCProcessSetupInterfaceTap(virDomainDef *vm,
>  {
>      g_autofree char *parentVeth = NULL;
>      g_autofree char *containerVeth = NULL;
> +    g_autofree char *backupIfname = NULL;
>      const virNetDevVPortProfile *vport = virDomainNetGetActualVirtPortProfile(net);
>  
>      VIR_DEBUG("calling vethCreate()");
> @@ -315,14 +316,17 @@ virLXCProcessSetupInterfaceTap(virDomainDef *vm,
>              return NULL;
>      }
>  
> -    if (net->filter &&
> -        virDomainConfNWFilterInstantiate(vm->name, vm->uuid, net, false) < 0)
> -        return NULL;
> -
> -    /* success is guaranteed, so update the interface object */
> -    g_free(net->ifname);
> +    /* success almost guaranteed, next function needs updated net->ifname */
> +    backupIfname = g_steal_pointer(net->ifname);

g_steal_pointer should be called on &net->ifname

>      net->ifname = g_steal_pointer(&parentVeth);
>  
> +    if (net->filter &&
> +        virDomainConfNWFilterInstantiate(vm->name, vm->uuid, net, false) < 0) {
> +        g_free(net->ifname);
> +        net->ifname = g_steal_pointer(&backupIfname);
> +        return NULL;
> +    }
> +
>      return g_steal_pointer(&containerVeth);
>  }
>  

Reviewed-by: Jiri Denemark <jdenemar@redhat.com>

and pushed, thanks.