[PATCH] Revert "esx: switch VIR_FREE->g_free in esx*Free*()"

Martin Kletzander via Devel posted 1 patch 19 hours ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/55a7afabbc2f0e5e8c7b5fc46d3f4bb9db547d3c.1778767009.git.mkletzan@redhat.com
src/esx/esx_driver.c |  2 +-
src/esx/esx_stream.c |  4 ++--
src/esx/esx_util.c   | 13 +++++++------
3 files changed, 10 insertions(+), 9 deletions(-)
[PATCH] Revert "esx: switch VIR_FREE->g_free in esx*Free*()"
Posted by Martin Kletzander via Devel 19 hours ago
From: Martin Kletzander <mkletzan@redhat.com>

This reverts commit 443c79dd7f7d4051fc0084baaa6c56a55d2aace4.

Change from VIR_FREE() to g_free meant there is a possible double free
when there is an error during parsing because the parsing it done
directly into the parsedUri member of the esxPrivate, free'd when it
fails and then the caller calls free on it again.  Changing back to
VIR_FREE() means there is no double free and no crash.

Reproducible easily with `virsh -c esx://l?no_verify=2`.

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
---
 src/esx/esx_driver.c |  2 +-
 src/esx/esx_stream.c |  4 ++--
 src/esx/esx_util.c   | 13 +++++++------
 3 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index 010c62b8e880..6ff0db48ac02 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -63,7 +63,7 @@ esxFreePrivate(esxPrivate **priv)
     esxUtil_FreeParsedUri(&(*priv)->parsedUri);
     virObjectUnref((*priv)->caps);
     virObjectUnref((*priv)->xmlopt);
-    g_free(*priv);
+    VIR_FREE(*priv);
 }
 
 
diff --git a/src/esx/esx_stream.c b/src/esx/esx_stream.c
index 143b2405ed49..c1dd80806feb 100644
--- a/src/esx/esx_stream.c
+++ b/src/esx/esx_stream.c
@@ -321,8 +321,8 @@ esxFreeStreamPrivate(esxStreamPrivate **priv)
         return;
 
     esxVI_CURL_Free(&(*priv)->curl);
-    g_free((*priv)->backlog);
-    g_free(*priv);
+    VIR_FREE((*priv)->backlog);
+    VIR_FREE(*priv);
 }
 
 static int
diff --git a/src/esx/esx_util.c b/src/esx/esx_util.c
index a6275babd542..1443ec3b9e46 100644
--- a/src/esx/esx_util.c
+++ b/src/esx/esx_util.c
@@ -165,13 +165,14 @@ esxUtil_FreeParsedUri(esxUtil_ParsedUri **parsedUri)
     if (!parsedUri || !(*parsedUri))
         return;
 
-    g_free((*parsedUri)->transport);
-    g_free((*parsedUri)->vCenter);
-    g_free((*parsedUri)->proxy_hostname);
-    g_free((*parsedUri)->path);
-    g_free((*parsedUri)->cacert);
 
-    g_free(*parsedUri);
+    VIR_FREE((*parsedUri)->transport);
+    VIR_FREE((*parsedUri)->vCenter);
+    VIR_FREE((*parsedUri)->proxy_hostname);
+    VIR_FREE((*parsedUri)->path);
+    VIR_FREE((*parsedUri)->cacert);
+
+    VIR_FREE(*parsedUri);
 }
 
 
-- 
2.54.0
Re: [PATCH] Revert "esx: switch VIR_FREE->g_free in esx*Free*()"
Posted by Pavel Hrdina via Devel 18 hours ago
On Thu, May 14, 2026 at 03:56:49PM +0200, Martin Kletzander via Devel wrote:
> From: Martin Kletzander <mkletzan@redhat.com>
> 
> This reverts commit 443c79dd7f7d4051fc0084baaa6c56a55d2aace4.
> 
> Change from VIR_FREE() to g_free meant there is a possible double free
> when there is an error during parsing because the parsing it done
> directly into the parsedUri member of the esxPrivate, free'd when it
> fails and then the caller calls free on it again.  Changing back to
> VIR_FREE() means there is no double free and no crash.
> 
> Reproducible easily with `virsh -c esx://l?no_verify=2`.
> 
> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
> ---
>  src/esx/esx_driver.c |  2 +-
>  src/esx/esx_stream.c |  4 ++--
>  src/esx/esx_util.c   | 13 +++++++------
>  3 files changed, 10 insertions(+), 9 deletions(-)

Reviewed-by: Pavel Hrdina <phrdina@redhat.com>