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

Use virAsprintf instead of snprintf. --- src/util/pci.c | 172 +++++++++++++++++++++++++++++++++++++++++--------------- 1 files changed, 127 insertions(+), 45 deletions(-) diff --git a/src/util/pci.c b/src/util/pci.c index 8d2dbb0..6ed96f4 100644 --- a/src/util/pci.c +++ b/src/util/pci.c @@ -775,38 +775,72 @@ pciResetDevice(pciDevice *dev, } -static void -pciDriverDir(char *buf, size_t buflen, const char *driver) +static int +pciDriverDir(char **buffer, const char *driver) { - snprintf(buf, buflen, PCI_SYSFS "drivers/%s", driver); + VIR_FREE(*buffer); + + if (virAsprintf(buffer, PCI_SYSFS "drivers/%s", driver) < 0) { + virReportOOMError(); + return -1; + } + + return 0; } -static void -pciDriverFile(char *buf, size_t buflen, const char *driver, const char *file) +static int +pciDriverFile(char **buffer, const char *driver, const char *file) { - snprintf(buf, buflen, PCI_SYSFS "drivers/%s/%s", driver, file); + VIR_FREE(*buffer); + + if (virAsprintf(buffer, PCI_SYSFS "drivers/%s/%s", driver, file) < 0) { + virReportOOMError(); + return -1; + } + + return 0; } -static void -pciDeviceFile(char *buf, size_t buflen, const char *device, const char *file) +static int +pciDeviceFile(char **buffer, const char *device, const char *file) { - snprintf(buf, buflen, PCI_SYSFS "devices/%s/%s", device, file); + VIR_FREE(*buffer); + + if (virAsprintf(buffer, PCI_SYSFS "devices/%s/%s", device, file) < 0) { + virReportOOMError(); + return -1; + } + + return 0; } static const char * pciFindStubDriver(void) { - char drvpath[PATH_MAX]; + char *drvpath = NULL; int probed = 0; recheck: - pciDriverDir(drvpath, sizeof(drvpath), "pci-stub"); - if (virFileExists(drvpath)) + if (pciDriverDir(&drvpath, "pci-stub") < 0) { + return NULL; + } + + if (virFileExists(drvpath)) { + VIR_FREE(drvpath); return "pci-stub"; - pciDriverDir(drvpath, sizeof(drvpath), "pciback"); - if (virFileExists(drvpath)) + } + + if (pciDriverDir(&drvpath, "pciback") < 0) { + return NULL; + } + + if (virFileExists(drvpath)) { + VIR_FREE(drvpath); return "pciback"; + } + + VIR_FREE(drvpath); if (!probed) { const char *const stubprobe[] = { MODPROBE, "pci-stub", NULL }; @@ -837,8 +871,9 @@ recheck: static int pciBindDeviceToStub(pciDevice *dev, const char *driver) { - char drvdir[PATH_MAX]; - char path[PATH_MAX]; + int result = -1; + char *drvdir = NULL; + char *path = NULL; /* Add the PCI device ID to the stub's dynamic ID table; * this is needed to allow us to bind the device to the stub. @@ -848,12 +883,15 @@ pciBindDeviceToStub(pciDevice *dev, const char *driver) * is triggered for such a device, it will also be immediately * bound by the stub. */ - pciDriverFile(path, sizeof(path), driver, "new_id"); + if (pciDriverFile(&path, driver, "new_id") < 0) { + return -1; + } + if (virFileWriteStr(path, dev->id, 0) < 0) { virReportSystemError(errno, _("Failed to add PCI device ID '%s' to %s"), dev->id, driver); - return -1; + goto cleanup; } /* If the device is already bound to a driver, unbind it. @@ -861,48 +899,69 @@ pciBindDeviceToStub(pciDevice *dev, const char *driver) * PCI device happens to be IDE controller for the disk hosting * your root filesystem. */ - pciDeviceFile(path, sizeof(path), dev->name, "driver/unbind"); + if (pciDeviceFile(&path, dev->name, "driver/unbind") < 0) { + goto cleanup; + } + if (virFileExists(path) && virFileWriteStr(path, dev->name, 0) < 0) { virReportSystemError(errno, _("Failed to unbind PCI device '%s'"), dev->name); - return -1; + goto cleanup; } /* If the device isn't already bound to pci-stub, try binding it now. */ - pciDriverDir(drvdir, sizeof(drvdir), driver); - pciDeviceFile(path, sizeof(path), dev->name, "driver"); + if (pciDriverDir(&drvdir, driver) < 0 || + pciDeviceFile(&path, dev->name, "driver") < 0) { + goto cleanup; + } + if (!virFileLinkPointsTo(path, drvdir)) { /* Xen's pciback.ko wants you to use new_slot first */ - pciDriverFile(path, sizeof(path), driver, "new_slot"); + if (pciDriverFile(&path, driver, "new_slot") < 0) { + goto cleanup; + } + if (virFileExists(path) && virFileWriteStr(path, dev->name, 0) < 0) { virReportSystemError(errno, _("Failed to add slot for PCI device '%s' to %s"), dev->name, driver); - return -1; + goto cleanup; + } + + if (pciDriverFile(&path, driver, "bind") < 0) { + goto cleanup; } - pciDriverFile(path, sizeof(path), driver, "bind"); if (virFileWriteStr(path, dev->name, 0) < 0) { virReportSystemError(errno, _("Failed to bind PCI device '%s' to %s"), dev->name, driver); - return -1; + goto cleanup; } } /* If 'remove_id' exists, remove the device id from pci-stub's dynamic * ID table so that 'drivers_probe' works below. */ - pciDriverFile(path, sizeof(path), driver, "remove_id"); + if (pciDriverFile(&path, driver, "remove_id") < 0) { + goto cleanup; + } + if (virFileExists(path) && virFileWriteStr(path, dev->id, 0) < 0) { virReportSystemError(errno, _("Failed to remove PCI ID '%s' from %s"), dev->id, driver); - return -1; + goto cleanup; } - return 0; + result = 0; + +cleanup: + VIR_FREE(drvdir); + VIR_FREE(path); + + return result; } int @@ -927,49 +986,67 @@ pciDettachDevice(pciDevice *dev, pciDeviceList *activeDevs) static int pciUnBindDeviceFromStub(pciDevice *dev, const char *driver) { - char drvdir[PATH_MAX]; - char path[PATH_MAX]; + int result = -1; + char *drvdir = NULL; + char *path = NULL; /* If the device is bound to stub, unbind it. */ - pciDriverDir(drvdir, sizeof(drvdir), driver); - pciDeviceFile(path, sizeof(path), dev->name, "driver"); + if (pciDriverDir(&drvdir, driver) < 0 || + pciDeviceFile(&path, dev->name, "driver") < 0) { + goto cleanup; + } + if (virFileExists(drvdir) && virFileLinkPointsTo(path, drvdir)) { - pciDriverFile(path, sizeof(path), driver, "unbind"); + if (pciDriverFile(&path, driver, "unbind") < 0) { + goto cleanup; + } + if (virFileWriteStr(path, dev->name, 0) < 0) { virReportSystemError(errno, _("Failed to bind PCI device '%s' to %s"), dev->name, driver); - return -1; + goto cleanup; } } /* Xen's pciback.ko wants you to use remove_slot on the specific device */ - pciDriverFile(path, sizeof(path), driver, "remove_slot"); + if (pciDriverFile(&path, driver, "remove_slot") < 0) { + goto cleanup; + } + if (virFileExists(path) && virFileWriteStr(path, dev->name, 0) < 0) { virReportSystemError(errno, _("Failed to remove slot for PCI device '%s' to %s"), dev->name, driver); - return -1; + goto cleanup; } - /* Trigger a re-probe of the device is not in the stub's dynamic * ID table. If the stub is available, but 'remove_id' isn't * available, then re-probing would just cause the device to be * re-bound to the stub. */ - pciDriverFile(path, sizeof(path), driver, "remove_id"); + if (pciDriverFile(&path, driver, "remove_id") < 0) { + goto cleanup; + } + if (!virFileExists(drvdir) || virFileExists(path)) { if (virFileWriteStr(PCI_SYSFS "drivers_probe", dev->name, 0) < 0) { virReportSystemError(errno, _("Failed to trigger a re-probe for PCI device '%s'"), dev->name); - return -1; + goto cleanup; } } - return 0; + result = 0; + +cleanup: + VIR_FREE(drvdir); + VIR_FREE(path); + + return result; } int @@ -1103,15 +1180,20 @@ pciWaitForDeviceCleanup(pciDevice *dev, const char *matcher) static char * pciReadDeviceID(pciDevice *dev, const char *id_name) { - char path[PATH_MAX]; + char *path; char *id_str; - snprintf(path, sizeof(path), PCI_SYSFS "devices/%s/%s", - dev->name, id_name); + if (pciDeviceFile(&path, dev->name, id_name) < 0) { + return NULL; + } /* ID string is '0xNNNN\n' ... i.e. 7 bytes */ - if (virFileReadAll(path, 7, &id_str) < 0) + if (virFileReadAll(path, 7, &id_str) < 0) { + VIR_FREE(path); return NULL; + } + + VIR_FREE(path); /* Check for 0x suffix */ if (id_str[0] != '0' || id_str[1] != 'x') { -- 1.7.0.4

--- src/util/ebtables.c | 44 +++++++++++++++++++++++++++++--------------- 1 files changed, 29 insertions(+), 15 deletions(-) diff --git a/src/util/ebtables.c b/src/util/ebtables.c index e3b8da4..27dce5d 100644 --- a/src/util/ebtables.c +++ b/src/util/ebtables.c @@ -266,29 +266,43 @@ ebtablesAddRemoveRule(ebtRules *rules, int action, const char *arg, ...) ebtablesContext * ebtablesContextNew(const char *driver) { - ebtablesContext *ctx; - char chain[PATH_MAX]; + bool success = false; + ebtablesContext *ctx = NULL; + char *input_chain = NULL; + char *forward_chain = NULL; + char *nat_chain = NULL; if (VIR_ALLOC(ctx) < 0) return NULL; - snprintf(chain, sizeof(chain), "libvirt_%s_INPUT", driver); - if (!(ctx->input_filter = ebtRulesNew("filter", chain))) - goto error; + if (virAsprintf(&input_chain, "libvirt_%s_INPUT", driver) < 0 || + virAsprintf(&forward_chain, "libvirt_%s_FORWARD", driver) < 0 || + virAsprintf(&nat_chain, "libvirt_%s_POSTROUTING", driver) < 0) { + goto cleanup; + } - snprintf(chain, sizeof(chain), "libvirt_%s_FORWARD", driver); - if (!(ctx->forward_filter = ebtRulesNew("filter", chain))) - goto error; + if (!(ctx->input_filter = ebtRulesNew("filter", input_chain))) + goto cleanup; - snprintf(chain, sizeof(chain), "libvirt_%s_POSTROUTING", driver); - if (!(ctx->nat_postrouting = ebtRulesNew("nat", chain))) - goto error; + if (!(ctx->forward_filter = ebtRulesNew("filter", forward_chain))) + goto cleanup; - return ctx; + if (!(ctx->nat_postrouting = ebtRulesNew("nat", nat_chain))) + goto cleanup; - error: - ebtablesContextFree(ctx); - return NULL; + success = true; + +cleanup: + VIR_FREE(input_chain); + VIR_FREE(forward_chain); + VIR_FREE(nat_chain); + + if (!success) { + ebtablesContextFree(ctx); + ctx = NULL; + } + + return ctx; } /** -- 1.7.0.4

--- src/security/virt-aa-helper.c | 46 +++++++++++++++++++++++++---------------- 1 files changed, 28 insertions(+), 18 deletions(-) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 77df514..bb716e6 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -188,8 +188,9 @@ replace_string(char *orig, const size_t len, const char *oldstr, static int parserCommand(const char *profile_name, const char cmd) { + int result = -1; char flag[3]; - char profile[PATH_MAX]; + char *profile; int status; int ret; @@ -200,15 +201,15 @@ parserCommand(const char *profile_name, const char cmd) snprintf(flag, 3, "-%c", cmd); - if (snprintf(profile, PATH_MAX, "%s/%s", - APPARMOR_DIR "/libvirt", profile_name) > PATH_MAX - 1) { + if (virAsprintf(&profile, "%s/%s", + APPARMOR_DIR "/libvirt", profile_name) < 0) { vah_error(NULL, 0, _("profile name exceeds maximum length")); return -1; } if (!virFileExists(profile)) { vah_error(NULL, 0, _("profile does not exist")); - return -1; + goto cleanup; } else { const char * const argv[] = { "/sbin/apparmor_parser", flag, profile, NULL @@ -217,18 +218,23 @@ parserCommand(const char *profile_name, const char cmd) (WIFEXITED(status) && WEXITSTATUS(status) != 0)) { if (ret != 0) { vah_error(NULL, 0, _("failed to run apparmor_parser")); - return -1; + goto cleanup; } else if (cmd == 'R' && WIFEXITED(status) && WEXITSTATUS(status) == 234) { vah_warning(_("unable to unload already unloaded profile")); } else { vah_error(NULL, 0, _("apparmor_parser exited with error")); - return -1; + goto cleanup; } } } - return 0; + result = 0; + +cleanup: + VIR_FREE(profile); + + return result; } /* @@ -308,7 +314,7 @@ static int create_profile(const char *profile, const char *profile_name, const char *profile_files) { - char template[PATH_MAX]; + char *template; char *tcontent = NULL; char *pcontent = NULL; char *replace_name = NULL; @@ -324,8 +330,7 @@ create_profile(const char *profile, const char *profile_name, goto end; } - if (snprintf(template, PATH_MAX, "%s/TEMPLATE", - APPARMOR_DIR "/libvirt") > PATH_MAX - 1) { + if (virAsprintf(&template, "%s/TEMPLATE", APPARMOR_DIR "/libvirt") < 0) { vah_error(NULL, 0, _("template name exceeds maximum length")); goto end; } @@ -409,6 +414,7 @@ create_profile(const char *profile, const char *profile_name, clean_tcontent: VIR_FREE(tcontent); end: + VIR_FREE(template); return rc; } @@ -1134,8 +1140,8 @@ main(int argc, char **argv) vahControl _ctl, *ctl = &_ctl; virBuffer buf = VIR_BUFFER_INITIALIZER; int rc = -1; - char profile[PATH_MAX]; - char include_file[PATH_MAX]; + char *profile = NULL; + char *include_file = NULL; if (setlocale(LC_ALL, "") == NULL || bindtextdomain(PACKAGE, LOCALEDIR) == NULL || @@ -1164,13 +1170,13 @@ main(int argc, char **argv) if (vahParseArgv(ctl, argc, argv) != 0) vah_error(ctl, 1, _("could not parse arguments")); - if (snprintf(profile, PATH_MAX, "%s/%s", - APPARMOR_DIR "/libvirt", ctl->uuid) > PATH_MAX - 1) - vah_error(ctl, 1, _("profile name exceeds maximum length")); + if (virAsprintf(&profile, "%s/%s", + APPARMOR_DIR "/libvirt", ctl->uuid) < 0) + vah_error(ctl, 0, _("could not allocate memory")); - if (snprintf(include_file, PATH_MAX, "%s/%s.files", - APPARMOR_DIR "/libvirt", ctl->uuid) > PATH_MAX - 1) - vah_error(ctl, 1, _("disk profile name exceeds maximum length")); + if (virAsprintf(&include_file, "%s/%s.files", + APPARMOR_DIR "/libvirt", ctl->uuid) < 0) + vah_error(ctl, 0, _("could not allocate memory")); if (ctl->cmd == 'a') rc = parserLoad(ctl->uuid); @@ -1258,5 +1264,9 @@ main(int argc, char **argv) } vahDeinit(ctl); + + VIR_FREE(profile); + VIR_FREE(include_file); + exit(rc == 0 ? EXIT_SUCCESS : EXIT_FAILURE); } -- 1.7.0.4

Allocate on the heap instead. --- src/phyp/phyp_driver.c | 13 +++++++++++-- 1 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index 51f9ff6..fe2e99d 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -115,12 +115,18 @@ phypExec(LIBSSH2_SESSION * session, char *cmd, int *exit_status, LIBSSH2_CHANNEL *channel; ConnectionData *connection_data = conn->networkPrivateData; virBuffer tex_ret = VIR_BUFFER_INITIALIZER; - char buffer[0x4000] = { 0 }; + char *buffer = NULL; + size_t buffer_size = 16384; int exitcode; int bytecount = 0; int sock = connection_data->sock; int rc = 0; + if (VIR_ALLOC_N(buffer, buffer_size) < 0) { + virReportOOMError(); + return NULL; + } + /* Exec non-blocking on the remove host */ while ((channel = libssh2_channel_open_session(session)) == NULL && libssh2_session_last_error(session, NULL, NULL, 0) == @@ -144,7 +150,7 @@ phypExec(LIBSSH2_SESSION * session, char *cmd, int *exit_status, for (;;) { /* loop until we block */ do { - rc = libssh2_channel_read(channel, buffer, sizeof(buffer)); + rc = libssh2_channel_read(channel, buffer, buffer_size); if (rc > 0) { bytecount += rc; virBufferVSprintf(&tex_ret, "%s", buffer); @@ -179,9 +185,12 @@ phypExec(LIBSSH2_SESSION * session, char *cmd, int *exit_status, err: (*exit_status) = SSH_CMD_ERR; virBufferFreeAndReset(&tex_ret); + VIR_FREE(buffer); return NULL; exit: + VIR_FREE(buffer); + if (virBufferError(&tex_ret)) { virBufferFreeAndReset(&tex_ret); virReportOOMError(); -- 1.7.0.4

Removing a 4kb stack allocation. Reduce stack buffer for virStrerror to the common 1kb instead of 4kb. --- src/qemu/qemu_process.c | 16 ++++++++++++---- 1 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 90fcea0..fc9fdae 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1012,7 +1012,8 @@ static int qemuProcessWaitForMonitor(struct qemud_driver* driver, virDomainObjPtr vm, off_t pos) { - char buf[4096] = ""; /* Plenty of space to get startup greeting */ + char *buf; + size_t buf_size = 4096; /* Plenty of space to get startup greeting */ int logfd; int ret = -1; virHashTablePtr paths = NULL; @@ -1020,7 +1021,12 @@ qemuProcessWaitForMonitor(struct qemud_driver* driver, if ((logfd = qemuProcessLogReadFD(driver->logDir, vm->def->name, pos)) < 0) return -1; - if (qemuProcessReadLogOutput(vm, logfd, buf, sizeof(buf), + if (VIR_ALLOC_N(buf, buf_size) < 0) { + virReportOOMError(); + return -1; + } + + if (qemuProcessReadLogOutput(vm, logfd, buf, buf_size, qemuProcessFindCharDevicePTYs, "console", 30) < 0) goto closelog; @@ -1053,16 +1059,18 @@ cleanup: if (kill(vm->pid, 0) == -1 && errno == ESRCH) { /* VM is dead, any other error raised in the interim is probably * not as important as the qemu cmdline output */ - qemuProcessReadLogFD(logfd, buf, sizeof(buf), strlen(buf)); + qemuProcessReadLogFD(logfd, buf, buf_size, strlen(buf)); qemuReportError(VIR_ERR_INTERNAL_ERROR, _("process exited while connecting to monitor: %s"), buf); ret = -1; } + VIR_FREE(buf); + closelog: if (VIR_CLOSE(logfd) < 0) { - char ebuf[4096]; + char ebuf[1024]; VIR_WARN("Unable to close logfile: %s", virStrerror(errno, ebuf, sizeof ebuf)); } -- 1.7.0.4

Move the buffers to the heap allocated client/private data structs. --- daemon/libvirtd.c | 10 ++++++---- daemon/libvirtd.h | 1 + src/remote/remote_driver.c | 8 +++++--- 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 024f56f..42cbe5d 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -1766,15 +1766,17 @@ static ssize_t qemudClientReadSASL(struct qemud_client *client) { /* Need to read some more data off the wire */ if (client->saslDecoded == NULL) { int ret; - char encoded[8192]; - ssize_t encodedLen = sizeof(encoded); - encodedLen = qemudClientReadBuf(client, encoded, encodedLen); + ssize_t encodedLen; + + encodedLen = qemudClientReadBuf(client, client->saslTemporary, + sizeof(client->saslTemporary)); if (encodedLen <= 0) return encodedLen; - ret = sasl_decode(client->saslconn, encoded, encodedLen, + ret = sasl_decode(client->saslconn, client->saslTemporary, encodedLen, &client->saslDecoded, &client->saslDecodedLength); + if (ret != SASL_OK) { VIR_ERROR(_("failed to decode SASL data %s"), sasl_errstring(ret, NULL, NULL)); diff --git a/daemon/libvirtd.h b/daemon/libvirtd.h index 7da3cfd..d37c3fd 100644 --- a/daemon/libvirtd.h +++ b/daemon/libvirtd.h @@ -213,6 +213,7 @@ struct qemud_client { unsigned int saslEncodedLength; unsigned int saslEncodedOffset; char *saslUsername; + char saslTemporary[8192]; /* temorary holds data to be decoded */ # endif /* Count of meages in 'dx' or 'tx' queue diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index bf94e70..942ff4b 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -174,6 +174,8 @@ struct private_data { const char *saslEncoded; unsigned int saslEncodedLength; unsigned int saslEncodedOffset; + + char saslTemporary[8192]; /* temorary holds data to be decoded */ #endif /* Buffer for incoming data packets @@ -10063,15 +10065,15 @@ remoteIOReadMessage(struct private_data *priv) { #if HAVE_SASL if (priv->saslconn) { if (priv->saslDecoded == NULL) { - char encoded[8192]; int ret, err; - ret = remoteIOReadBuffer(priv, encoded, sizeof(encoded)); + ret = remoteIOReadBuffer(priv, priv->saslTemporary, + sizeof(priv->saslTemporary)); if (ret < 0) return -1; if (ret == 0) return 0; - err = sasl_decode(priv->saslconn, encoded, ret, + err = sasl_decode(priv->saslconn, priv->saslTemporary, ret, &priv->saslDecoded, &priv->saslDecodedLength); if (err != SASL_OK) { remoteError(VIR_ERR_INTERNAL_ERROR, -- 1.7.0.4

--- src/xenxs/xen_xm.c | 14 ++++++-------- 1 files changed, 6 insertions(+), 8 deletions(-) diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c index 0acd120..22ad788 100644 --- a/src/xenxs/xen_xm.c +++ b/src/xenxs/xen_xm.c @@ -211,6 +211,7 @@ xenParseXM(virConfPtr conf, int xendConfigVersion, const char *defaultArch, *defaultMachine; int vmlocaltime = 0; unsigned long count; + char *script = NULL; if (VIR_ALLOC(def) < 0) { virReportOOMError(); @@ -556,7 +557,6 @@ xenParseXM(virConfPtr conf, int xendConfigVersion, if (list && list->type == VIR_CONF_LIST) { list = list->list; while (list) { - char script[PATH_MAX]; char model[10]; char type[10]; char ip[16]; @@ -567,7 +567,6 @@ xenParseXM(virConfPtr conf, int xendConfigVersion, bridge[0] = '\0'; mac[0] = '\0'; - script[0] = '\0'; ip[0] = '\0'; model[0] = '\0'; type[0] = '\0'; @@ -602,12 +601,10 @@ xenParseXM(virConfPtr conf, int xendConfigVersion, goto skipnic; } } else if (STRPREFIX(key, "script=")) { - int len = nextkey ? (nextkey - data) : sizeof(script) - 1; - if (virStrncpy(script, data, len, sizeof(script)) == NULL) { - XENXS_ERROR(VIR_ERR_INTERNAL_ERROR, - _("Script %s too big for destination"), - data); - goto skipnic; + int len = nextkey ? (nextkey - data) : strlen(data); + VIR_FREE(script); + if (!(script = strndup(data, len))) { + goto no_memory; } } else if (STRPREFIX(key, "model=")) { int len = nextkey ? (nextkey - data) : sizeof(model) - 1; @@ -1043,6 +1040,7 @@ cleanup: virDomainNetDefFree(net); virDomainDiskDefFree(disk); virDomainDefFree(def); + VIR_FREE(script); return NULL; } -- 1.7.0.4

Removes multiple 4kb stack allocations. --- src/libvirt.c | 113 +++++++++++++++++++++------------------------------------ 1 files changed, 42 insertions(+), 71 deletions(-) diff --git a/src/libvirt.c b/src/libvirt.c index 8be18d4..25f8d41 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -2238,7 +2238,6 @@ error: int virDomainSave(virDomainPtr domain, const char *to) { - char filepath[4096]; virConnectPtr conn; VIR_DOMAIN_DEBUG(domain, "to=%s", to); @@ -2260,29 +2259,24 @@ virDomainSave(virDomainPtr domain, const char *to) goto error; } - /* - * We must absolutize the file path as the save is done out of process - * TODO: check for URI when libxml2 is linked in. - */ - if (to[0] != '/') { - unsigned int len, t; + if (conn->driver->domainSave) { + int ret; + char *absolute_to; - t = strlen(to); - if (getcwd(filepath, sizeof(filepath) - (t + 3)) == NULL) - return -1; - len = strlen(filepath); - /* that should be covered by getcwd() semantic, but be 100% sure */ - if (len > sizeof(filepath) - (t + 3)) - return -1; - filepath[len] = '/'; - strcpy(&filepath[len + 1], to); - to = &filepath[0]; + /* + * We must absolutize the file path as the save is done out of process + * TODO: check for URI when libxml2 is linked in. + */ + if (virFileAbsPath(to, &absolute_to) < 0) { + virLibConnError(VIR_ERR_INTERNAL_ERROR, + _("could not build absolute output file path")); + goto error; + } - } + ret = conn->driver->domainSave(domain, absolute_to); + + VIR_FREE(absolute_to); - if (conn->driver->domainSave) { - int ret; - ret = conn->driver->domainSave (domain, to); if (ret < 0) goto error; return ret; @@ -2298,7 +2292,7 @@ error: /** * virDomainRestore: * @conn: pointer to the hypervisor connection - * @from: path to the + * @from: path to the input file * * This method will restore a domain saved to disk by virDomainSave(). * @@ -2307,7 +2301,6 @@ error: int virDomainRestore(virConnectPtr conn, const char *from) { - char filepath[4096]; VIR_DEBUG("conn=%p, from=%s", conn, from); virResetLastError(); @@ -2326,34 +2319,24 @@ virDomainRestore(virConnectPtr conn, const char *from) goto error; } - /* - * We must absolutize the file path as the restore is done out of process - * TODO: check for URI when libxml2 is linked in. - */ - if (from[0] != '/') { - unsigned int len, t; + if (conn->driver->domainRestore) { + int ret; + char *absolute_from; - t = strlen(from); - if (getcwd(filepath, sizeof(filepath) - (t + 3)) == NULL) { - virLibConnError(VIR_ERR_SYSTEM_ERROR, - _("cannot get working directory")); - goto error; - } - len = strlen(filepath); - /* that should be covered by getcwd() semantic, but be 100% sure */ - if (len > sizeof(filepath) - (t + 3)) { + /* + * We must absolutize the file path as the restore is done out of process + * TODO: check for URI when libxml2 is linked in. + */ + if (virFileAbsPath(from, &absolute_from) < 0) { virLibConnError(VIR_ERR_INTERNAL_ERROR, - _("path too long")); + _("could not build absolute input file path")); goto error; } - filepath[len] = '/'; - strcpy(&filepath[len + 1], from); - from = &filepath[0]; - } - if (conn->driver->domainRestore) { - int ret; - ret = conn->driver->domainRestore (conn, from); + ret = conn->driver->domainRestore(conn, absolute_from); + + VIR_FREE(absolute_from); + if (ret < 0) goto error; return ret; @@ -2381,7 +2364,6 @@ error: int virDomainCoreDump(virDomainPtr domain, const char *to, int flags) { - char filepath[4096]; virConnectPtr conn; VIR_DOMAIN_DEBUG(domain, "to=%s, flags=%d", to, flags); @@ -2403,35 +2385,24 @@ virDomainCoreDump(virDomainPtr domain, const char *to, int flags) goto error; } - /* - * We must absolutize the file path as the save is done out of process - * TODO: check for URI when libxml2 is linked in. - */ - if (to[0] != '/') { - unsigned int len, t; + if (conn->driver->domainCoreDump) { + int ret; + char *absolute_to; - t = strlen(to); - if (getcwd(filepath, sizeof(filepath) - (t + 3)) == NULL) { - virLibDomainError(VIR_ERR_SYSTEM_ERROR, - _("cannot get current directory")); - goto error; - } - len = strlen(filepath); - /* that should be covered by getcwd() semantic, but be 100% sure */ - if (len > sizeof(filepath) - (t + 3)) { - virLibDomainError(VIR_ERR_INTERNAL_ERROR, - _("path too long")); + /* + * We must absolutize the file path as the save is done out of process + * TODO: check for URI when libxml2 is linked in. + */ + if (virFileAbsPath(to, &absolute_to) < 0) { + virLibConnError(VIR_ERR_INTERNAL_ERROR, + _("could not build absolute core file path")); goto error; } - filepath[len] = '/'; - strcpy(&filepath[len + 1], to); - to = &filepath[0]; - } + ret = conn->driver->domainCoreDump(domain, absolute_to, flags); + + VIR_FREE(absolute_to); - if (conn->driver->domainCoreDump) { - int ret; - ret = conn->driver->domainCoreDump (domain, to, flags); if (ret < 0) goto error; return ret; -- 1.7.0.4

--- tools/virsh.c | 23 +++++++++++++++++------ 1 files changed, 17 insertions(+), 6 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 19e3449..99da054 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -2052,7 +2052,7 @@ cmdDominfo(vshControl *ctl, const vshCmd *cmd) virDomainInfo info; virDomainPtr dom; virSecurityModel secmodel; - virSecurityLabel seclabel; + virSecurityLabelPtr seclabel; int persistent = 0; int ret = TRUE, autostart; unsigned int id; @@ -2138,15 +2138,22 @@ cmdDominfo(vshControl *ctl, const vshCmd *cmd) vshPrint(ctl, "%-15s %s\n", _("Security DOI:"), secmodel.doi); /* Security labels are only valid for active domains */ - memset(&seclabel, 0, sizeof seclabel); - if (virDomainGetSecurityLabel(dom, &seclabel) == -1) { + if (VIR_ALLOC(seclabel) < 0) { virDomainFree(dom); return FALSE; + } + + if (virDomainGetSecurityLabel(dom, seclabel) == -1) { + virDomainFree(dom); + VIR_FREE(seclabel); + return FALSE; } else { - if (seclabel.label[0] != '\0') + if (seclabel->label[0] != '\0') vshPrint(ctl, "%-15s %s (%s)\n", _("Security label:"), - seclabel.label, seclabel.enforcing ? "enforcing" : "permissive"); + seclabel->label, seclabel->enforcing ? "enforcing" : "permissive"); } + + VIR_FREE(seclabel); } } virDomainFree(dom); @@ -12141,7 +12148,7 @@ vshOpenLogFile(vshControl *ctl) static void vshOutputLogFile(vshControl *ctl, int log_level, const char *msg_format, va_list ap) { - char msg_buf[MSG_BUFFER]; + char *msg_buf; const char *lvl = ""; struct timeval stTimeval; struct tm *stTm; @@ -12149,6 +12156,8 @@ vshOutputLogFile(vshControl *ctl, int log_level, const char *msg_format, va_list if (ctl->log_fd == -1) return; + msg_buf = vshMalloc(ctl, MSG_BUFFER); + /** * create log format * @@ -12199,6 +12208,8 @@ vshOutputLogFile(vshControl *ctl, int log_level, const char *msg_format, va_list vshCloseLogFile(ctl); vshError(ctl, "%s", _("failed to write the log file")); } + + VIR_FREE(msg_buf); } /** -- 1.7.0.4

--- src/phyp/phyp_driver.c | 13 +++++++++++-- 1 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index fe2e99d..76207c2 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -2362,13 +2362,22 @@ phypBuildVolume(virConnectPtr conn, const char *lvname, const char *spname, static virStorageVolPtr phypVolumeLookupByName(virStoragePoolPtr pool, const char *volname) { + char *key; + virStorageVolPtr vol; - char key[MAX_KEY_SIZE]; + if (VIR_ALLOC_N(key, MAX_KEY_SIZE) < 0) { + virReportOOMError(); + return NULL; + } if (phypVolumeGetKey(pool->conn, key, volname) == -1) return NULL; - return virGetStorageVol(pool->conn, pool->name, volname, key); + vol = virGetStorageVol(pool->conn, pool->name, volname, key); + + VIR_FREE(key); + + return vol; } static virStorageVolPtr -- 1.7.0.4

--- daemon/remote.c | 20 ++++++++++++++------ 1 files changed, 14 insertions(+), 6 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index 1700c2d..dd85ef1 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -1607,7 +1607,7 @@ remoteDispatchDomainGetSecurityLabel(struct qemud_server *server ATTRIBUTE_UNUSE remote_domain_get_security_label_ret *ret) { virDomainPtr dom; - virSecurityLabel seclabel; + virSecurityLabelPtr seclabel; dom = get_nonnull_domain(conn, args->dom); if (dom == NULL) { @@ -1615,22 +1615,30 @@ remoteDispatchDomainGetSecurityLabel(struct qemud_server *server ATTRIBUTE_UNUSE return -1; } - memset(&seclabel, 0, sizeof seclabel); - if (virDomainGetSecurityLabel(dom, &seclabel) == -1) { + if (VIR_ALLOC(seclabel) < 0) { + virDomainFree(dom); + remoteDispatchOOMError(rerr); + return -1; + } + + if (virDomainGetSecurityLabel(dom, seclabel) == -1) { remoteDispatchConnError(rerr, conn); virDomainFree(dom); + VIR_FREE(seclabel); return -1; } - ret->label.label_len = strlen(seclabel.label) + 1; + ret->label.label_len = strlen(seclabel->label) + 1; if (VIR_ALLOC_N(ret->label.label_val, ret->label.label_len) < 0) { virDomainFree(dom); + VIR_FREE(seclabel); remoteDispatchOOMError(rerr); return -1; } - strcpy(ret->label.label_val, seclabel.label); - ret->enforcing = seclabel.enforcing; + strcpy(ret->label.label_val, seclabel->label); + ret->enforcing = seclabel->enforcing; virDomainFree(dom); + VIR_FREE(seclabel); return 0; } -- 1.7.0.4

Replace openvz_readline with getline in several places to get rid of stack allocated buffers to hold lines. openvzReadConfigParam allocates memory for return values instead of expecting a preexisting buffer. --- src/openvz/openvz_conf.c | 141 +++++++++++++++++++++++++++----------------- src/openvz/openvz_conf.h | 2 +- src/openvz/openvz_driver.c | 45 +++++++++----- 3 files changed, 117 insertions(+), 71 deletions(-) diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c index 2fcca68..88cd4c8 100644 --- a/src/openvz/openvz_conf.c +++ b/src/openvz/openvz_conf.c @@ -187,7 +187,7 @@ openvzReadNetworkConf(virDomainDefPtr def, int veid) { int ret; virDomainNetDefPtr net = NULL; - char temp[4096]; + char *temp = NULL; char *token, *saveptr = NULL; /*parse routing network configuration* @@ -195,7 +195,7 @@ openvzReadNetworkConf(virDomainDefPtr def, * IP_ADDRESS="1.1.1.1 1.1.1.2" * splited IPs by space */ - ret = openvzReadVPSConfigParam(veid, "IP_ADDRESS", temp, sizeof(temp)); + ret = openvzReadVPSConfigParam(veid, "IP_ADDRESS", &temp); if (ret < 0) { openvzError(VIR_ERR_INTERNAL_ERROR, _("Could not read 'IP_ADDRESS' from config for container %d"), @@ -227,7 +227,7 @@ openvzReadNetworkConf(virDomainDefPtr def, *NETIF="ifname=eth10,mac=00:18:51:C1:05:EE,host_ifname=veth105.10,host_mac=00:18:51:8F:D9:F3" *devices splited by ';' */ - ret = openvzReadVPSConfigParam(veid, "NETIF", temp, sizeof(temp)); + ret = openvzReadVPSConfigParam(veid, "NETIF", &temp); if (ret < 0) { openvzError(VIR_ERR_INTERNAL_ERROR, _("Could not read 'NETIF' from config for container %d"), @@ -316,10 +316,13 @@ openvzReadNetworkConf(virDomainDefPtr def, } } + VIR_FREE(temp); + return 0; no_memory: virReportOOMError(); error: + VIR_FREE(temp); virDomainNetDefFree(net); return -1; } @@ -364,10 +367,10 @@ openvzReadFSConf(virDomainDefPtr def, int veid) { int ret; virDomainFSDefPtr fs = NULL; - char* veid_str = NULL; - char temp[100]; + char *veid_str = NULL; + char *temp = NULL; - ret = openvzReadVPSConfigParam(veid, "OSTEMPLATE", temp, sizeof(temp)); + ret = openvzReadVPSConfigParam(veid, "OSTEMPLATE", &temp); if (ret < 0) { openvzError(VIR_ERR_INTERNAL_ERROR, _("Could not read 'OSTEMPLATE' from config for container %d"), @@ -381,7 +384,7 @@ openvzReadFSConf(virDomainDefPtr def, fs->src = strdup(temp); } else { /* OSTEMPLATE was not found, VE was booted from a private dir directly */ - ret = openvzReadVPSConfigParam(veid, "VE_PRIVATE", temp, sizeof(temp)); + ret = openvzReadVPSConfigParam(veid, "VE_PRIVATE", &temp); if (ret <= 0) { openvzError(VIR_ERR_INTERNAL_ERROR, _("Could not read 'VE_PRIVATE' from config for container %d"), @@ -411,10 +414,13 @@ openvzReadFSConf(virDomainDefPtr def, def->fss[def->nfss++] = fs; fs = NULL; + VIR_FREE(temp); + return 0; no_memory: virReportOOMError(); error: + VIR_FREE(temp); virDomainFSDefFree(fs); return -1; } @@ -439,7 +445,7 @@ int openvzLoadDomains(struct openvz_driver *driver) { char *status; char uuidstr[VIR_UUID_STRING_BUFLEN]; virDomainObjPtr dom = NULL; - char temp[50]; + char *temp = NULL; char *outbuf = NULL; char *line; virCommandPtr cmd = NULL; @@ -506,7 +512,7 @@ int openvzLoadDomains(struct openvz_driver *driver) { if (!(dom->def->os.init = strdup("/sbin/init"))) goto no_memory; - ret = openvzReadVPSConfigParam(veid, "CPUS", temp, sizeof(temp)); + ret = openvzReadVPSConfigParam(veid, "CPUS", &temp); if (ret < 0) { openvzError(VIR_ERR_INTERNAL_ERROR, _("Could not read config for container %d"), @@ -534,6 +540,7 @@ int openvzLoadDomains(struct openvz_driver *driver) { } virCommandFree(cmd); + VIR_FREE(temp); VIR_FREE(outbuf); return 0; @@ -543,6 +550,7 @@ int openvzLoadDomains(struct openvz_driver *driver) { cleanup: virCommandFree(cmd); + VIR_FREE(temp); VIR_FREE(outbuf); /* dom hasn't been shared yet, so unref should return 0 */ if (dom) @@ -565,25 +573,26 @@ static int openvzWriteConfigParam(const char * conf_file, const char *param, const char *value) { char * temp_file = NULL; - int fd = -1, temp_fd = -1; - char line[PATH_MAX]; + int temp_fd = -1; + FILE *fp; + char *line = NULL; + size_t line_size = 0; if (virAsprintf(&temp_file, "%s.tmp", conf_file)<0) { virReportOOMError(); return -1; } - fd = open(conf_file, O_RDONLY); - if (fd == -1) + fp = fopen(conf_file, "r"); + if (fp == NULL) goto error; temp_fd = open(temp_file, O_WRONLY | O_CREAT | O_TRUNC, 0644); if (temp_fd == -1) { - VIR_FORCE_CLOSE(fd); goto error; } while (1) { - if (openvz_readline(fd, line, sizeof(line)) <= 0) + if (getline(&line, &line_size, fp) <= 0) break; if (!(STRPREFIX(line, param) && line[strlen(param)] == '=')) { @@ -599,7 +608,7 @@ openvzWriteConfigParam(const char * conf_file, const char *param, const char *va safewrite(temp_fd, "\"\n", 2) < 0) goto error; - if (VIR_CLOSE(fd) < 0) + if (VIR_FCLOSE(fp) < 0) goto error; if (VIR_CLOSE(temp_fd) < 0) goto error; @@ -607,10 +616,13 @@ openvzWriteConfigParam(const char * conf_file, const char *param, const char *va if (rename(temp_file, conf_file) < 0) goto error; + VIR_FREE(line); + return 0; error: - VIR_FORCE_CLOSE(fd); + VIR_FREE(line); + VIR_FORCE_FCLOSE(fp); VIR_FORCE_CLOSE(temp_fd); if (temp_file) unlink(temp_file); @@ -632,23 +644,29 @@ openvzWriteVPSConfigParam(int vpsid, const char *param, const char *value) return ret; } +/* + * value will be freed before a new value is assigned to it, the caller is + * responsible for freeing it afterwards. + */ static int -openvzReadConfigParam(const char * conf_file ,const char * param, char *value, int maxlen) +openvzReadConfigParam(const char *conf_file, const char *param, char **value) { - char line[PATH_MAX]; - int ret, found = 0; - int fd; - char * sf, * token; + char *line = NULL; + size_t line_size = 0; + ssize_t ret; + FILE *fp; + int found = 0; + char *sf, *token; char *saveptr = NULL; value[0] = 0; - fd = open(conf_file, O_RDONLY); - if (fd == -1) + fp = fopen(conf_file, "r"); + if (fp == NULL) return -1; while (1) { - ret = openvz_readline(fd, line, sizeof(line)); + ret = getline(&line, &line_size, fp); if (ret <= 0) break; saveptr = NULL; @@ -658,7 +676,9 @@ openvzReadConfigParam(const char * conf_file ,const char * param, char *value, i if (sf[0] == '=' && sf[1] != '\0' ) { sf++; if ((token = strtok_r(sf,"\"\t\n", &saveptr)) != NULL) { - if (virStrcpy(value, token, maxlen) == NULL) { + VIR_FREE(*value); + *value = strdup(token); + if (value == NULL) { ret = -1; break; } @@ -667,7 +687,8 @@ openvzReadConfigParam(const char * conf_file ,const char * param, char *value, i } } } - VIR_FORCE_CLOSE(fd); + VIR_FREE(line); + VIR_FORCE_FCLOSE(fp); if (ret == 0 && found) ret = 1; @@ -676,14 +697,18 @@ openvzReadConfigParam(const char * conf_file ,const char * param, char *value, i } /* -* Read parameter from container config -* sample: 133, "OSTEMPLATE", value, 1024 -* return: -1 - error -* 0 - don't found -* 1 - OK -*/ + * Read parameter from container config + * + * value will be freed before a new value is assined to it, the caller is + * responsible for freeing it afterwards. + * + * sample: 133, "OSTEMPLATE", &value + * return: -1 - error + * 0 - don't found + * 1 - OK + */ int -openvzReadVPSConfigParam(int vpsid ,const char * param, char *value, int maxlen) +openvzReadVPSConfigParam(int vpsid, const char *param, char **value) { char *conf_file; int ret; @@ -691,7 +716,7 @@ openvzReadVPSConfigParam(int vpsid ,const char * param, char *value, int maxlen) if (openvzLocateConfFile(vpsid, &conf_file, "conf") < 0) return -1; - ret = openvzReadConfigParam(conf_file, param, value, maxlen); + ret = openvzReadConfigParam(conf_file, param, value); VIR_FREE(conf_file); return ret; } @@ -699,21 +724,23 @@ openvzReadVPSConfigParam(int vpsid ,const char * param, char *value, int maxlen) static int openvz_copyfile(char* from_path, char* to_path) { - char line[PATH_MAX]; - int fd, copy_fd; + char *line = NULL; + size_t line_size = 0; + FILE *fp; + int copy_fd; int bytes_read; - fd = open(from_path, O_RDONLY); - if (fd == -1) + fp = fopen(from_path, "r"); + if (fp == NULL) return -1; copy_fd = open(to_path, O_WRONLY | O_CREAT | O_TRUNC, 0644); if (copy_fd == -1) { - VIR_FORCE_CLOSE(fd); + VIR_FORCE_FCLOSE(fp); return -1; } while (1) { - if (openvz_readline(fd, line, sizeof(line)) <= 0) + if (getline(&line, &line_size, fp) <= 0) break; bytes_read = strlen(line); @@ -721,15 +748,18 @@ openvz_copyfile(char* from_path, char* to_path) goto error; } - if (VIR_CLOSE(fd) < 0) + if (VIR_FCLOSE(fp) < 0) goto error; if (VIR_CLOSE(copy_fd) < 0) goto error; + VIR_FREE(line); + return 0; error: - VIR_FORCE_CLOSE(fd); + VIR_FREE(line); + VIR_FORCE_FCLOSE(fp); VIR_FORCE_CLOSE(copy_fd); return -1; } @@ -742,14 +772,13 @@ error: int openvzCopyDefaultConfig(int vpsid) { - char * confdir = NULL; - char * default_conf_file = NULL; - char configfile_value[PATH_MAX]; + char *confdir = NULL; + char *default_conf_file = NULL; + char *configfile_value = NULL; char *conf_file = NULL; int ret = -1; - if (openvzReadConfigParam(VZ_CONF_FILE, "CONFIGFILE", configfile_value, - PATH_MAX) < 0) + if (openvzReadConfigParam(VZ_CONF_FILE, "CONFIGFILE", &configfile_value) < 0) goto cleanup; confdir = openvzLocateConfDir(); @@ -772,6 +801,7 @@ openvzCopyDefaultConfig(int vpsid) cleanup: VIR_FREE(confdir); VIR_FREE(default_conf_file); + VIR_FREE(configfile_value); VIR_FREE(conf_file); return ret; } @@ -844,22 +874,24 @@ static int openvzGetVPSUUID(int vpsid, char *uuidstr, size_t len) { char *conf_file; - char line[1024]; + char *line = NULL; + size_t line_size = 0; + ssize_t ret; char *saveptr = NULL; char *uuidbuf; char *iden; - int fd, ret; + FILE *fp; int retval = -1; if (openvzLocateConfFile(vpsid, &conf_file, "conf") < 0) return -1; - fd = open(conf_file, O_RDONLY); - if (fd == -1) + fp = fopen(conf_file, "r"); + if (fp == NULL) goto cleanup; while (1) { - ret = openvz_readline(fd, line, sizeof(line)); + ret = getline(&line, &line_size, fp); if (ret == -1) goto cleanup; @@ -882,7 +914,8 @@ openvzGetVPSUUID(int vpsid, char *uuidstr, size_t len) } retval = 0; cleanup: - VIR_FORCE_CLOSE(fd); + VIR_FREE(line); + VIR_FORCE_FCLOSE(fp); VIR_FREE(conf_file); return retval; diff --git a/src/openvz/openvz_conf.h b/src/openvz/openvz_conf.h index 50bcb5e..4673fa6 100644 --- a/src/openvz/openvz_conf.h +++ b/src/openvz/openvz_conf.h @@ -55,7 +55,7 @@ struct openvz_driver { int openvz_readline(int fd, char *ptr, int maxlen); int openvzExtractVersion(struct openvz_driver *driver); -int openvzReadVPSConfigParam(int vpsid ,const char * param, char *value, int maxlen); +int openvzReadVPSConfigParam(int vpsid, const char *param, char **value); int openvzWriteVPSConfigParam(int vpsid, const char *param, const char *value); int openvzCopyDefaultConfig(int vpsid); virCapsPtr openvzCapsInit(void); diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index fb30c37..c6fee3b 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -631,15 +631,16 @@ openvzGenerateVethName(int veid, char *dev_name_ve) static char * openvzGenerateContainerVethName(int veid) { - char temp[1024]; + char *temp = NULL; + char *name = NULL; /* try to get line "^NETIF=..." from config */ - if (openvzReadVPSConfigParam(veid, "NETIF", temp, sizeof(temp)) <= 0) { - snprintf(temp, sizeof(temp), "eth0"); + if (openvzReadVPSConfigParam(veid, "NETIF", &temp) <= 0) { + name = strdup("eth0"); } else { char *saveptr; - char *s; - int max = 0; + char *s; + int max = 0; /* get maximum interface number (actually, it is the last one) */ for (s=strtok_r(temp, ";", &saveptr); s; s=strtok_r(NULL, ";", &saveptr)) { @@ -650,9 +651,16 @@ openvzGenerateContainerVethName(int veid) } /* set new name */ - snprintf(temp, sizeof(temp), "eth%d", max+1); + virAsprintf(&name, "eth%d", max + 1); } - return strdup(temp); + + VIR_FREE(temp); + + if (name == NULL) { + virReportOOMError(); + } + + return name; } static int @@ -1125,7 +1133,7 @@ openvzDomainGetAutostart(virDomainPtr dom, int *autostart) { struct openvz_driver *driver = dom->conn->privateData; virDomainObjPtr vm; - char value[1024]; + char *value = NULL; int ret = -1; openvzDriverLock(driver); @@ -1138,7 +1146,7 @@ openvzDomainGetAutostart(virDomainPtr dom, int *autostart) goto cleanup; } - if (openvzReadVPSConfigParam(strtoI(vm->def->name), "ONBOOT", value, sizeof(value)) < 0) { + if (openvzReadVPSConfigParam(strtoI(vm->def->name), "ONBOOT", &value) < 0) { openvzError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not read container config")); goto cleanup; @@ -1150,6 +1158,8 @@ openvzDomainGetAutostart(virDomainPtr dom, int *autostart) ret = 0; cleanup: + VIR_FREE(value); + if (vm) virDomainObjUnlock(vm); return ret; @@ -1464,12 +1474,14 @@ out: return -1; } -static int openvzGetProcessInfo(unsigned long long *cpuTime, int vpsid) { - int fd; - char line[1024] ; +static int openvzGetProcessInfo(unsigned long long *cpuTime, int vpsid) +{ + FILE *fp; + char *line = NULL; + size_t line_size = 0; unsigned long long usertime, systime, nicetime; int readvps = vpsid + 1; /* ensure readvps is initially different */ - int ret; + ssize_t ret; /* read statistic from /proc/vz/vestat. sample: @@ -1479,12 +1491,12 @@ Version: 2.2 55 178 0 5340 59424597 542650441835148 other.. */ - if ((fd = open("/proc/vz/vestat", O_RDONLY)) == -1) + if ((fp = fopen("/proc/vz/vestat", "r")) == NULL) return -1; /*search line with VEID=vpsid*/ while (1) { - ret = openvz_readline(fd, line, sizeof(line)); + ret = getline(&line, &line_size, fp); if (ret <= 0) break; @@ -1499,7 +1511,8 @@ Version: 2.2 } } - VIR_FORCE_CLOSE(fd); + VIR_FREE(line); + VIR_FORCE_FCLOSE(fp); if (ret < 0) return -1; -- 1.7.0.4

--- src/util/util.c | 9 ++++----- 1 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/util/util.c b/src/util/util.c index 31feecc..c0391ad 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -1901,14 +1901,13 @@ int virFileOpenTtyAt(const char *ptmx, } if (ttyName) { - char tempTtyName[PATH_MAX]; - if (ptsname_r(*ttymaster, tempTtyName, sizeof(tempTtyName)) < 0) - goto cleanup; - - if ((*ttyName = strdup(tempTtyName)) == NULL) { + if (VIR_ALLOC_N(*ttyName, PATH_MAX) < 0) { errno = ENOMEM; goto cleanup; } + + if (ptsname_r(*ttymaster, *ttyName, PATH_MAX) < 0) + goto cleanup; } rc = 0; -- 1.7.0.4

--- src/qemu/qemu_command.c | 43 ++++++++++++++++++++++++++++++------------- 1 files changed, 30 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 3138943..326a6b3 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3318,7 +3318,8 @@ qemuBuildCommandLine(virConnectPtr conn, } else { for (i = 0 ; i < def->ndisks ; i++) { char dev[NAME_MAX]; - char file[PATH_MAX]; + char *file; + const char *fmt; virDomainDiskDefPtr disk = def->disks[i]; int j; @@ -3368,9 +3369,13 @@ qemuBuildCommandLine(virConnectPtr conn, goto error; } if (disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) - snprintf(file, PATH_MAX, "fat:floppy:%s", disk->src); + fmt = "fat:floppy:%s"; else - snprintf(file, PATH_MAX, "fat:%s", disk->src); + fmt = "fat:%s"; + + if (virAsprintf(&file, fmt, disk->src) < 0) { + goto no_memory; + } } else if (disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK) { switch (disk->protocol) { case VIR_DOMAIN_DISK_PROTOCOL_NBD: @@ -3379,11 +3384,15 @@ qemuBuildCommandLine(virConnectPtr conn, _("NBD accepts only one host")); goto error; } - snprintf(file, PATH_MAX, "nbd:%s:%s,", - disk->hosts->name, disk->hosts->port); + if (virAsprintf(&file, "nbd:%s:%s,", disk->hosts->name, + disk->hosts->port) < 0) { + goto no_memory; + } break; case VIR_DOMAIN_DISK_PROTOCOL_RBD: - snprintf(file, PATH_MAX, "rbd:%s,", disk->src); + if (virAsprintf(&file, "rbd:%s,", disk->src) < 0) { + goto no_memory; + } for (j = 0 ; j < disk->nhosts ; j++) { if (!has_rbd_hosts) { virBufferAddLit(&rbd_hosts, "CEPH_ARGS=-m "); @@ -3403,20 +3412,28 @@ qemuBuildCommandLine(virConnectPtr conn, } break; case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG: - if (disk->nhosts == 0) - snprintf(file, PATH_MAX, "sheepdog:%s,", disk->src); - else + if (disk->nhosts == 0) { + if (virAsprintf(&file, "sheepdog:%s,", disk->src) < 0) { + goto no_memory; + } + } else { /* only one host is supported now */ - snprintf(file, PATH_MAX, "sheepdog:%s:%s:%s,", - disk->hosts->name, disk->hosts->port, - disk->src); + if (virAsprintf(&file, "sheepdog:%s:%s:%s,", + disk->hosts->name, disk->hosts->port, + disk->src) < 0) { + goto no_memory; + } + } break; } } else { - snprintf(file, PATH_MAX, "%s", disk->src); + if (!(file = strdup(disk->src))) { + goto no_memory; + } } virCommandAddArgList(cmd, dev, file, NULL); + VIR_FREE(file); } } -- 1.7.0.4

--- src/xen/block_stats.c | 52 +++++++++++++++++++++++++----------------------- 1 files changed, 27 insertions(+), 25 deletions(-) diff --git a/src/xen/block_stats.c b/src/xen/block_stats.c index e7c80c9..6b4171d 100644 --- a/src/xen/block_stats.c +++ b/src/xen/block_stats.c @@ -113,34 +113,36 @@ read_stat (const char *path) } static int64_t -read_bd_stat (int device, int domid, const char *str) +read_bd_stat(int device, int domid, const char *str) { - char path[PATH_MAX]; + static const char *paths[] = { + "/sys/bus/xen-backend/devices/vbd-%d-%d/statistics/%s", + "/sys/bus/xen-backend/devices/tap-%d-%d/statistics/%s", + "/sys/devices/xen-backend/vbd-%d-%d/statistics/%s", + "/sys/devices/xen-backend/tap-%d-%d/statistics/%s", + NULL + }; + + int i; + char *path; int64_t r; - snprintf (path, sizeof path, - "/sys/bus/xen-backend/devices/vbd-%d-%d/statistics/%s", - domid, device, str); - r = read_stat (path); - if (r >= 0) return r; - - snprintf (path, sizeof path, - "/sys/bus/xen-backend/devices/tap-%d-%d/statistics/%s", - domid, device, str); - r = read_stat (path); - if (r >= 0) return r; - - snprintf (path, sizeof path, - "/sys/devices/xen-backend/vbd-%d-%d/statistics/%s", - domid, device, str); - r = read_stat (path); - if (r >= 0) return r; - - snprintf (path, sizeof path, - "/sys/devices/xen-backend/tap-%d-%d/statistics/%s", - domid, device, str); - r = read_stat (path); - return r; + for (i = 0; paths[i] != NULL; ++i) { + if (virAsprintf(&path, paths[i], domid, device, str) < 0) { + virReportOOMError(); + return -1; + } + + r = read_stat(path); + + VIR_FREE(path); + + if (r >= 0) { + return r; + } + } + + return -1; } /* In Xenstore, /local/domain/0/backend/vbd/<domid>/<device>/state, -- 1.7.0.4

--- src/storage/storage_backend_iscsi.c | 11 ++++++++--- 1 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/storage/storage_backend_iscsi.c b/src/storage/storage_backend_iscsi.c index 6eff5f5..2e2e99e 100644 --- a/src/storage/storage_backend_iscsi.c +++ b/src/storage/storage_backend_iscsi.c @@ -409,12 +409,15 @@ static int virStorageBackendISCSIFindLUs(virStoragePoolObjPtr pool, const char *session) { - char sysfs_path[PATH_MAX]; + char *sysfs_path; int retval = 0; uint32_t host; - snprintf(sysfs_path, PATH_MAX, - "/sys/class/iscsi_session/session%s/device", session); + if (virAsprintf(&sysfs_path, + "/sys/class/iscsi_session/session%s/device", session) < 0) { + virReportOOMError(); + return -1; + } if (virStorageBackendSCSIGetHostNumber(sysfs_path, &host) < 0) { virReportSystemError(errno, @@ -430,6 +433,8 @@ virStorageBackendISCSIFindLUs(virStoragePoolObjPtr pool, retval = -1; } + VIR_FREE(sysfs_path); + return retval; } -- 1.7.0.4

--- src/uml/uml_driver.c | 10 +++++++--- 1 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index e2bd5f2..33849a0 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -1036,21 +1036,25 @@ static char *umlGetCapabilities(virConnectPtr conn) { -static int umlGetProcessInfo(unsigned long long *cpuTime, int pid) { - char proc[PATH_MAX]; +static int umlGetProcessInfo(unsigned long long *cpuTime, int pid) +{ + char *proc; FILE *pidinfo; unsigned long long usertime, systime; - if (snprintf(proc, sizeof(proc), "/proc/%d/stat", pid) >= (int)sizeof(proc)) { + if (virAsprintf(&proc, "/proc/%d/stat", pid) < 0) { return -1; } if (!(pidinfo = fopen(proc, "r"))) { /* VM probably shut down, so fake 0 */ *cpuTime = 0; + VIR_FREE(proc); return 0; } + VIR_FREE(proc); + if (fscanf(pidinfo, "%*d %*s %*c %*d %*d %*d %*d %*d %*u %*u %*u %*u %*u %llu %llu", &usertime, &systime) != 2) { umlDebug("not enough arg"); VIR_FORCE_FCLOSE(pidinfo); -- 1.7.0.4

--- src/xen/xend_internal.c | 12 ++++++++++-- 1 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index 8859373..8b07a8a 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -279,11 +279,17 @@ istartswith(const char *haystack, const char *needle) static int ATTRIBUTE_NONNULL (2) xend_req(int fd, char **content) { - char buffer[4096]; + char *buffer; + size_t buffer_size = 4096; int content_length = 0; int retcode = 0; - while (sreads(fd, buffer, sizeof(buffer)) > 0) { + if (VIR_ALLOC_N(buffer, buffer_size) < 0) { + virReportOOMError(); + return -1; + } + + while (sreads(fd, buffer, buffer_size) > 0) { if (STREQ(buffer, "\r\n")) break; @@ -293,6 +299,8 @@ xend_req(int fd, char **content) retcode = atoi(buffer + 9); } + VIR_FREE(buffer); + if (content_length > 0) { ssize_t ret; -- 1.7.0.4

Removes 4kb stack allocation in the XenD subdriver. --- src/util/sexpr.c | 142 +++++++++++++++++++++++------------------------ src/util/sexpr.h | 3 +- src/xen/xend_internal.c | 17 +++++- 3 files changed, 86 insertions(+), 76 deletions(-) diff --git a/src/util/sexpr.c b/src/util/sexpr.c index 7f14206..ae1692a 100644 --- a/src/util/sexpr.c +++ b/src/util/sexpr.c @@ -199,78 +199,56 @@ sexpr_append(struct sexpr *lst, const struct sexpr *value) * sexpr2string: * @sexpr: an S-Expression pointer * @buffer: the output buffer - * @n_buffer: the size of the buffer in bytes * * Serialize the S-Expression in the buffer. - * Note that the output may be truncated if @n_buffer is too small - * resulting in an unparseable value. * - * Returns the number of bytes used by the serialization in the buffer or - * 0 in case of error. + * Returns 0 on success, -1 on error. */ -size_t -sexpr2string(const struct sexpr * sexpr, char *buffer, size_t n_buffer) +int +sexpr2string(const struct sexpr *sexpr, virBufferPtr buffer) { - size_t ret = 0, tmp; - - if ((sexpr == NULL) || (buffer == NULL) || (n_buffer <= 0)) - return (0); + if ((sexpr == NULL) || (buffer == NULL)) + return -1; switch (sexpr->kind) { - case SEXPR_CONS: - tmp = snprintf(buffer + ret, n_buffer - ret, "("); - if (tmp == 0) - goto error; - ret += tmp; - tmp = sexpr2string(sexpr->u.s.car, buffer + ret, n_buffer - ret); - if (tmp == 0) - goto error; - ret += tmp; - while (sexpr->u.s.cdr->kind != SEXPR_NIL) { - sexpr = sexpr->u.s.cdr; - tmp = snprintf(buffer + ret, n_buffer - ret, " "); - if (tmp == 0) - goto error; - ret += tmp; - tmp = - sexpr2string(sexpr->u.s.car, buffer + ret, n_buffer - ret); - if (tmp == 0) - goto error; - ret += tmp; - } - tmp = snprintf(buffer + ret, n_buffer - ret, ")"); - if (tmp == 0) - goto error; - ret += tmp; - break; - case SEXPR_VALUE: - if (strchr(sexpr->u.value, ' ') || - strchr(sexpr->u.value, ')') || - strchr(sexpr->u.value, '(')) - tmp = snprintf(buffer + ret, n_buffer - ret, "'%s'", - sexpr->u.value); - else - tmp = snprintf(buffer + ret, n_buffer - ret, "%s", - sexpr->u.value); - if (tmp == 0) - goto error; - ret += tmp; - break; - case SEXPR_NIL: - tmp = snprintf(buffer + ret, n_buffer - ret, "()"); - if (tmp == 0) - goto error; - ret += tmp; - break; - default: + case SEXPR_CONS: + virBufferAddChar(buffer, '('); + + if (sexpr2string(sexpr->u.s.car, buffer) < 0) goto error; + + while (sexpr->u.s.cdr->kind != SEXPR_NIL) { + sexpr = sexpr->u.s.cdr; + + virBufferAddChar(buffer, ' '); + + if (sexpr2string(sexpr->u.s.car, buffer) < 0) + goto error; + } + + virBufferAddChar(buffer, ')'); + break; + case SEXPR_VALUE: + if (strchr(sexpr->u.value, ' ') || + strchr(sexpr->u.value, ')') || + strchr(sexpr->u.value, '(')) + virBufferVSprintf(buffer, "'%s'", sexpr->u.value); + else + virBufferVSprintf(buffer, "%s", sexpr->u.value); + + break; + case SEXPR_NIL: + virBufferAddLit(buffer, "()"); + break; + default: + goto error; } - return (ret); + return 0; + error: - buffer[n_buffer - 1] = 0; - virSexprError(VIR_ERR_SEXPR_SERIAL, "%s", buffer); - return (0); + virSexprError(VIR_ERR_SEXPR_SERIAL, NULL); + return -1; } #define IS_SPACE(c) ((c == 0x20) || (c == 0x9) || (c == 0xD) || (c == 0xA)) @@ -413,22 +391,28 @@ string2sexpr(const char *buffer) static struct sexpr * sexpr_lookup_key(const struct sexpr *sexpr, const char *node) { - char buffer[4096], *ptr, *token; + struct sexpr *result = NULL; + char *buffer, *ptr, *token; if ((node == NULL) || (sexpr == NULL)) - return (NULL); + return NULL; - snprintf(buffer, sizeof(buffer), "%s", node); + buffer = strdup(node); + + if (buffer == NULL) { + virReportOOMError(); + return NULL; + } ptr = buffer; token = strsep(&ptr, "/"); if (sexpr->kind != SEXPR_CONS || sexpr->u.s.car->kind != SEXPR_VALUE) { - return NULL; + goto cleanup; } if (STRNEQ(sexpr->u.s.car->u.value, token)) { - return NULL; + goto cleanup; } for (token = strsep(&ptr, "/"); token; token = strsep(&ptr, "/")) { @@ -454,10 +438,15 @@ sexpr_lookup_key(const struct sexpr *sexpr, const char *node) } if (token != NULL) { - return NULL; + goto cleanup; } - return (struct sexpr *) sexpr; + result = (struct sexpr *) sexpr; + +cleanup: + VIR_FREE(buffer); + + return result; } /** @@ -550,21 +539,30 @@ int sexpr_node_copy(const struct sexpr *sexpr, const char *node, char **dst) * @... extra data to build the path * * Search a node value in the S-Expression based on its path - * NOTE: path are limited to 4096 bytes. * * Returns the value of the node or NULL if not found. */ const char * sexpr_fmt_node(const struct sexpr *sexpr, const char *fmt, ...) { + int result; va_list ap; - char node[4096]; + char *node; + const char *value; va_start(ap, fmt); - vsnprintf(node, sizeof(node), fmt, ap); + result = virVasprintf(&node, fmt, ap); va_end(ap); - return sexpr_node(sexpr, node); + if (result < 0) { + return NULL; + } + + value = sexpr_node(sexpr, node); + + VIR_FREE(node); + + return value; } /** diff --git a/src/util/sexpr.h b/src/util/sexpr.h index dc6687d..8dfd89a 100644 --- a/src/util/sexpr.h +++ b/src/util/sexpr.h @@ -14,6 +14,7 @@ # define _LIBVIR_SEXPR_H_ # include "internal.h" +# include "buf.h" # include <sys/types.h> # include <stdint.h> @@ -36,7 +37,7 @@ struct sexpr { }; /* conversion to/from strings */ -size_t sexpr2string(const struct sexpr *sexpr, char *buffer, size_t n_buffer); +int sexpr2string(const struct sexpr *sexpr, virBufferPtr buffer); struct sexpr *string2sexpr(const char *buffer); /* constructors and destructors */ diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index 8b07a8a..04122ba 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -3023,7 +3023,8 @@ xenDaemonDomainSetAutostart(virDomainPtr domain, int autostart) { struct sexpr *root, *autonode; - char buf[4096]; + virBuffer buffer = VIR_BUFFER_INITIALIZER; + char *content = NULL; int ret = -1; xenUnifiedPrivatePtr priv; @@ -3065,12 +3066,20 @@ xenDaemonDomainSetAutostart(virDomainPtr domain, goto error; } - if (sexpr2string(root, buf, sizeof(buf)) == 0) { + if (sexpr2string(root, &buffer) < 0) { virXendError(VIR_ERR_INTERNAL_ERROR, "%s", _("sexpr2string failed")); goto error; } - if (xend_op(domain->conn, "", "op", "new", "config", buf, NULL) != 0) { + + if (virBufferError(&buffer)) { + virReportOOMError(); + goto error; + } + + content = virBufferContentAndReset(&buffer); + + if (xend_op(domain->conn, "", "op", "new", "config", content, NULL) != 0) { virXendError(VIR_ERR_XEN_CALL, "%s", _("Failed to redefine sexpr")); goto error; @@ -3083,6 +3092,8 @@ xenDaemonDomainSetAutostart(virDomainPtr domain, ret = 0; error: + virBufferFreeAndReset(&buffer); + VIR_FREE(content); sexpr_free(root); return ret; } -- 1.7.0.4