[libvirt] Power Hypervisor: Fix potential segfault and memleak in phypOpen

[Matthias Bolte <matthias.bolte@xxxxxxxxxxxxxx>]

Hi,

I came across this line in the phypOpen function:

char string[strlen(conn->uri->path)];

Here the path part of the given URI is used without checking it for
NULL, this can cause a segfault as strlen expects a string != NULL.
Beside that uuid_db and connection_data leak in case of an error.

In this line

conn->uri->path = string;

the original path of the URI leaks. The patch adds a VIR_FREE call
before setting the new path.

The attached patch is compile-tested but I don't have a Power
Hypervisor installation at hand to test it for real.

Matthias

diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c
index cbfd31b..f21ae64 100644
--- a/src/phyp/phyp_driver.c
+++ b/src/phyp/phyp_driver.c
@@ -61,25 +61,17 @@ static virDrvOpenStatus
 phypOpen(virConnectPtr conn,
          virConnectAuthPtr auth, int flags ATTRIBUTE_UNUSED)
 {
-    SSH_SESSION *session;
-    ConnectionData *connection_data;
-    char string[strlen(conn->uri->path)];
-
+    SSH_SESSION *session = NULL;
+    ConnectionData *connection_data = NULL;
+    char *string = NULL;
     uuid_dbPtr uuid_db = NULL;
 
-    if (VIR_ALLOC(uuid_db) < 0)
-        virReportOOMError(conn);
-
-    if (VIR_ALLOC(connection_data) < 0)
-        virReportOOMError(conn);
-
     if (!conn || !conn->uri)
         return VIR_DRV_OPEN_DECLINED;
 
     if (conn->uri->scheme == NULL || STRNEQ(conn->uri->scheme, "phyp"))
         return VIR_DRV_OPEN_DECLINED;
 
-
     if (conn->uri->server == NULL) {
         virRaiseError(conn, NULL, NULL, 0, VIR_FROM_PHYP,
                       VIR_ERR_ERROR, NULL, NULL, NULL, 0, 0, "%s",
@@ -94,20 +86,36 @@ phypOpen(virConnectPtr conn,
         return VIR_DRV_OPEN_ERROR;
     }
 
+    if (VIR_ALLOC(uuid_db) < 0) {
+        virReportOOMError(conn);
+        goto failure;
+    }
+
+    if (VIR_ALLOC(connection_data) < 0) {
+        virReportOOMError(conn);
+        goto failure;
+    }
+
+    if (VIR_ALLOC_N(string, strlen(conn->uri->path) + 1) < 0) {
+        virReportOOMError(conn);
+        goto failure;
+    }
+
     if (escape_specialcharacters(conn->uri->path, string) == -1) {
         virRaiseError(conn, NULL, NULL, 0, VIR_FROM_PHYP,
                       VIR_ERR_ERROR, NULL, NULL, NULL, 0, 0, "%s",
                       _("Error parsing 'path'. Invalid characters."));
-        return VIR_DRV_OPEN_ERROR;
+        goto failure;
     }
 
     if ((session = openSSHSession(conn, auth)) == NULL) {
         virRaiseError(conn, NULL, NULL, 0, VIR_FROM_PHYP,
                       VIR_ERR_ERROR, NULL, NULL, NULL, 0, 0, "%s",
                       _("Error while opening SSH session."));
-        return VIR_DRV_OPEN_ERROR;
+        goto failure;
     }
 
+    VIR_FREE(conn->uri->path);
     conn->uri->path = string;
     connection_data->session = session;
     connection_data->auth = auth;
@@ -120,6 +128,13 @@ phypOpen(virConnectPtr conn,
     init_uuid_db(conn);
 
     return VIR_DRV_OPEN_SUCCESS;
+
+  failure:
+    VIR_FREE(uuid_db);
+    VIR_FREE(connection_data);
+    VIR_FREE(string);
+
+    return VIR_DRV_OPEN_ERROR;
 }
 
 static int

--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list