[PATCH] lxc: fix variable storage order before call

Adam Julis posted 1 patch 1 year, 4 months ago
Failed in applying to current master (apply log)
There is a newer version of this series
src/lxc/lxc_process.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
[PATCH] lxc: fix variable storage order before call
Posted by Adam Julis 1 year, 4 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 | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index 205ab96ebb..b00608e30a 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,18 @@ 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 */
+    /* success almost guaranteed, next function needs updated net->ifname */
+    backupIfname = g_strdup(net->ifname);
     g_free(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] lxc: fix variable storage order before call
Posted by Jiri Denemark 1 year, 4 months ago
On Thu, Oct 10, 2024 at 11:18:50 +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 | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
> index 205ab96ebb..b00608e30a 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,18 @@ 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 */
> +    /* success almost guaranteed, next function needs updated net->ifname */
> +    backupIfname = g_strdup(net->ifname);
>      g_free(net->ifname);

Just storing the net-ifname pointer to backupIfname would be better than
copying the string and freeing the original.

>      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);

Jirka