[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[libvirt] [PATCH 01/20] Remove PATH_MAX sized stack allocations related to virFileBuildPath



Make virFileBuildPath operate on the heap instead of the stack. It
allocates a buffer instead of expecting a preexisting buffer.
---
 src/conf/nwfilter_conf.c |   21 +++++++--------------
 src/conf/storage_conf.c  |   44 +++++++++++++++-----------------------------
 src/util/util.c          |   29 +++++++++++++++--------------
 src/util/util.h          |    8 +++-----
 src/xen/xen_inotify.c    |   16 +++++++++-------
 src/xen/xm_internal.c    |   33 +++++++++++++++++----------------
 6 files changed, 66 insertions(+), 85 deletions(-)

diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c
index 13b5b38..df8e20f 100644
--- a/src/conf/nwfilter_conf.c
+++ b/src/conf/nwfilter_conf.c
@@ -2484,7 +2484,7 @@ virNWFilterLoadAllConfigs(virConnectPtr conn,
     }
 
     while ((entry = readdir(dir))) {
-        char path[PATH_MAX];
+        char *path;
         virNWFilterObjPtr nwfilter;
 
         if (entry->d_name[0] == '.')
@@ -2493,17 +2493,16 @@ virNWFilterLoadAllConfigs(virConnectPtr conn,
         if (!virFileHasSuffix(entry->d_name, ".xml"))
             continue;
 
-        if (virFileBuildPath(configDir, entry->d_name,
-                             NULL, path, PATH_MAX) < 0) {
-            virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
-                                   _("config filename '%s/%s' is too long"),
-                                   configDir, entry->d_name);
+        if (!(path = virFileBuildPath(configDir, entry->d_name, NULL))) {
+            virReportOOMError();
             continue;
         }
 
         nwfilter = virNWFilterObjLoad(conn, nwfilters, entry->d_name, path);
         if (nwfilter)
             virNWFilterObjUnlock(nwfilter);
+
+        VIR_FREE(path);
     }
 
     closedir(dir);
@@ -2523,7 +2522,6 @@ virNWFilterObjSaveDef(virNWFilterDriverStatePtr driver,
 
     if (!nwfilter->configFile) {
         int err;
-        char path[PATH_MAX];
 
         if ((err = virFileMakePath(driver->configDir))) {
             virReportSystemError(err,
@@ -2532,13 +2530,8 @@ virNWFilterObjSaveDef(virNWFilterDriverStatePtr driver,
             return -1;
         }
 
-        if (virFileBuildPath(driver->configDir, def->name, ".xml",
-                             path, sizeof(path)) < 0) {
-            virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
-                                  "%s", _("cannot construct config file path"));
-            return -1;
-        }
-        if (!(nwfilter->configFile = strdup(path))) {
+        if (!(nwfilter->configFile = virFileBuildPath(driver->configDir,
+                                                      def->name, ".xml"))) {
             virReportOOMError();
             return -1;
         }
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 13a3622..5a069f5 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -1473,8 +1473,8 @@ virStoragePoolLoadAllConfigs(virStoragePoolObjListPtr pools,
     }
 
     while ((entry = readdir(dir))) {
-        char path[PATH_MAX];
-        char autostartLink[PATH_MAX];
+        char *path;
+        char *autostartLink;
         virStoragePoolObjPtr pool;
 
         if (entry->d_name[0] == '.')
@@ -1483,19 +1483,15 @@ virStoragePoolLoadAllConfigs(virStoragePoolObjListPtr pools,
         if (!virFileHasSuffix(entry->d_name, ".xml"))
             continue;
 
-        if (virFileBuildPath(configDir, entry->d_name,
-                             NULL, path, PATH_MAX) < 0) {
-            virStorageReportError(VIR_ERR_INTERNAL_ERROR,
-                                  _("Config filename '%s/%s' is too long"),
-                                  configDir, entry->d_name);
+        if (!(path = virFileBuildPath(configDir, entry->d_name, NULL))) {
+            virReportOOMError();
             continue;
         }
 
-        if (virFileBuildPath(autostartDir, entry->d_name,
-                             NULL, autostartLink, PATH_MAX) < 0) {
-            virStorageReportError(VIR_ERR_INTERNAL_ERROR,
-                                  _("Autostart link path '%s/%s' is too long"),
-                                  autostartDir, entry->d_name);
+        if (!(autostartLink = virFileBuildPath(autostartDir, entry->d_name,
+                                               NULL))) {
+            virReportOOMError();
+            VIR_FREE(path);
             continue;
         }
 
@@ -1503,6 +1499,9 @@ virStoragePoolLoadAllConfigs(virStoragePoolObjListPtr pools,
                                      autostartLink);
         if (pool)
             virStoragePoolObjUnlock(pool);
+
+        VIR_FREE(path);
+        VIR_FREE(autostartLink);
     }
 
     closedir(dir);
@@ -1520,7 +1519,6 @@ virStoragePoolObjSaveDef(virStorageDriverStatePtr driver,
 
     if (!pool->configFile) {
         int err;
-        char path[PATH_MAX];
 
         if ((err = virFileMakePath(driver->configDir))) {
             virReportSystemError(err,
@@ -1529,26 +1527,14 @@ virStoragePoolObjSaveDef(virStorageDriverStatePtr driver,
             return -1;
         }
 
-        if (virFileBuildPath(driver->configDir, def->name, ".xml",
-                             path, sizeof(path)) < 0) {
-            virStorageReportError(VIR_ERR_INTERNAL_ERROR,
-                                  "%s", _("cannot construct config file path"));
-            return -1;
-        }
-        if (!(pool->configFile = strdup(path))) {
+        if (!(pool->configFile = virFileBuildPath(driver->configDir,
+                                                  def->name, ".xml"))) {
             virReportOOMError();
             return -1;
         }
 
-        if (virFileBuildPath(driver->autostartDir, def->name, ".xml",
-                             path, sizeof(path)) < 0) {
-            virStorageReportError(VIR_ERR_INTERNAL_ERROR,
-                                  "%s", _("cannot construct "
-                                          "autostart link path"));
-            VIR_FREE(pool->configFile);
-            return -1;
-        }
-        if (!(pool->autostartLink = strdup(path))) {
+        if (!(pool->autostartLink = virFileBuildPath(driver->autostartDir,
+                                                     def->name, ".xml"))) {
             virReportOOMError();
             VIR_FREE(pool->configFile);
             return -1;
diff --git a/src/util/util.c b/src/util/util.c
index 43794b1..31feecc 100644
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -1841,23 +1841,24 @@ cleanup:
     return err;
 }
 
-/* Build up a fully qualfiied path for a config file to be
+/* Build up a fully qualified path for a config file to be
  * associated with a persistent guest or network */
-int virFileBuildPath(const char *dir,
-                     const char *name,
-                     const char *ext,
-                     char *buf,
-                     unsigned int buflen)
+char *
+virFileBuildPath(const char *dir, const char *name, const char *ext)
 {
-    if ((strlen(dir) + 1 + strlen(name) + (ext ? strlen(ext) : 0) + 1) >= (buflen-1))
-        return -1;
+    char *path;
 
-    strcpy(buf, dir);
-    strcat(buf, "/");
-    strcat(buf, name);
-    if (ext)
-        strcat(buf, ext);
-    return 0;
+    if (ext == NULL) {
+        if (virAsprintf(&path, "%s/%s", dir, name) < 0) {
+            return NULL;
+        }
+    } else {
+        if (virAsprintf(&path, "%s/%s%s", dir, name, ext) < 0) {
+            return NULL;
+        }
+    }
+
+    return path;
 }
 
 
diff --git a/src/util/util.h b/src/util/util.h
index 7b7722a..d320c40 100644
--- a/src/util/util.h
+++ b/src/util/util.h
@@ -146,11 +146,9 @@ int virDirCreate(const char *path, mode_t mode, uid_t uid, gid_t gid,
                  unsigned int flags) ATTRIBUTE_RETURN_CHECK;
 int virFileMakePath(const char *path) ATTRIBUTE_RETURN_CHECK;
 
-int virFileBuildPath(const char *dir,
-                     const char *name,
-                     const char *ext,
-                     char *buf,
-                     unsigned int buflen) ATTRIBUTE_RETURN_CHECK;
+char *virFileBuildPath(const char *dir,
+                       const char *name,
+                       const char *ext) ATTRIBUTE_RETURN_CHECK;
 
 int virFileAbsPath(const char *path,
                    char **abspath) ATTRIBUTE_RETURN_CHECK;
diff --git a/src/xen/xen_inotify.c b/src/xen/xen_inotify.c
index 7d4ba4c..5a997e6 100644
--- a/src/xen/xen_inotify.c
+++ b/src/xen/xen_inotify.c
@@ -387,7 +387,7 @@ xenInotifyOpen(virConnectPtr conn,
 {
     DIR *dh;
     struct dirent *ent;
-    char path[PATH_MAX];
+    char *path;
     xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData;
 
     if (priv->configDir) {
@@ -414,19 +414,21 @@ xenInotifyOpen(virConnectPtr conn,
                 continue;
 
             /* Build the full file path */
-            if ((strlen(priv->configDir) + 1 +
-                 strlen(ent->d_name) + 1) > PATH_MAX)
-                continue;
-            strcpy(path, priv->configDir);
-            strcat(path, "/");
-            strcat(path, ent->d_name);
+            if (!(path = virFileBuildPath(priv->configDir, ent->d_name, NULL))) {
+                virReportOOMError();
+                closedir(dh);
+                return -1;
+            }
 
             if (xenInotifyAddDomainConfigInfo(conn, path) < 0 ) {
                 virXenInotifyError(VIR_ERR_INTERNAL_ERROR,
                                    "%s", _("Error adding file to config list"));
                 closedir(dh);
+                VIR_FREE(path);
                 return -1;
             }
+
+            VIR_FREE(path);
         }
         closedir(dh);
     }
diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c
index 7f73588..9225808 100644
--- a/src/xen/xm_internal.c
+++ b/src/xen/xm_internal.c
@@ -358,7 +358,7 @@ int xenXMConfigCacheRefresh (virConnectPtr conn) {
 
     while ((ent = readdir(dh))) {
         struct stat st;
-        char path[PATH_MAX];
+        char *path;
 
         /*
          * Skip a bunch of crufty files that clearly aren't config files
@@ -387,15 +387,16 @@ int xenXMConfigCacheRefresh (virConnectPtr conn) {
             continue;
 
         /* Build the full file path */
-        if ((strlen(priv->configDir) + 1 + strlen(ent->d_name) + 1) > PATH_MAX)
-            continue;
-        strcpy(path, priv->configDir);
-        strcat(path, "/");
-        strcat(path, ent->d_name);
+        if (!(path = virFileBuildPath(priv->configDir, ent->d_name, NULL))) {
+            virReportOOMError();
+            closedir(dh);
+            return -1;
+        }
 
         /* Skip anything which isn't a file (takes care of scripts/ subdir */
         if ((stat(path, &st) < 0) ||
             (!S_ISREG(st.st_mode))) {
+            VIR_FREE(path);
             continue;
         }
 
@@ -404,6 +405,8 @@ int xenXMConfigCacheRefresh (virConnectPtr conn) {
         if (xenXMConfigCacheAddFile(conn, path) < 0) {
             /* Ignoring errors, since alot of stuff goes wrong in /etc/xen */
         }
+
+        VIR_FREE(path);
     }
 
     /* Reap all entries which were not changed, by comparing
@@ -1046,10 +1049,11 @@ int xenXMDomainCreate(virDomainPtr domain) {
  * Create a config file for a domain, based on an XML
  * document describing its config
  */
-virDomainPtr xenXMDomainDefineXML(virConnectPtr conn, const char *xml) {
+virDomainPtr xenXMDomainDefineXML(virConnectPtr conn, const char *xml)
+{
     virDomainPtr ret;
-    char filename[PATH_MAX];
-    const char * oldfilename;
+    char *filename;
+    const char *oldfilename;
     virDomainDefPtr def = NULL;
     xenXMConfCachePtr entry = NULL;
     xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData;
@@ -1130,16 +1134,11 @@ virDomainPtr xenXMDomainDefineXML(virConnectPtr conn, const char *xml) {
         entry = NULL;
     }
 
-    if ((strlen(priv->configDir) + 1 + strlen(def->name) + 1) > PATH_MAX) {
-        xenXMError(VIR_ERR_INTERNAL_ERROR,
-                   "%s", _("config file name is too long"));
+    if (!(filename = virFileBuildPath(priv->configDir, def->name, NULL))) {
+        virReportOOMError();
         goto error;
     }
 
-    strcpy(filename, priv->configDir);
-    strcat(filename, "/");
-    strcat(filename, def->name);
-
     if (xenXMConfigSaveFile(conn, filename, def) < 0)
         goto error;
 
@@ -1172,9 +1171,11 @@ virDomainPtr xenXMDomainDefineXML(virConnectPtr conn, const char *xml) {
 
     ret = virGetDomain(conn, def->name, def->uuid);
     xenUnifiedUnlock(priv);
+    VIR_FREE(filename);
     return (ret);
 
  error:
+    VIR_FREE(filename);
     VIR_FREE(entry);
     virDomainDefFree(def);
     xenUnifiedUnlock(priv);
-- 
1.7.0.4


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]