From ee820c73dbe8beef6d44df3e32ffb11438017460 Mon Sep 17 00:00:00 2001 From: Brian Neradt Date: Tue, 31 Aug 2021 11:43:22 -0500 Subject: [PATCH] Fix leaks in ConfigManager::configName (#8269) This fixes an ASan reported leak of ConfigManager::configName. It used to be strdup'd but not freed in the destructor. This simply changes it to a std::string. ASan also reported a leak in AddConfigFilesHere which is fixed with an ats_free as well. --- mgmt/ConfigManager.cc | 19 +++++++------------ mgmt/ConfigManager.h | 12 +++++++----- src/traffic_manager/AddConfigFilesHere.cc | 7 ++++--- 3 files changed, 18 insertions(+), 20 deletions(-) diff --git a/mgmt/ConfigManager.cc b/mgmt/ConfigManager.cc index a34d6d912e0..2f2a8306327 100644 --- a/mgmt/ConfigManager.cc +++ b/mgmt/ConfigManager.cc @@ -56,29 +56,24 @@ ConfigManager::ConfigManager(const char *fileName_, const char *configName_, boo } // Copy the file name. - fileName = ats_strdup(fileName_); - configName = ats_strdup(configName_); + fileName = std::string{fileName_}; + configName = std::string{configName_}; ink_mutex_init(&fileAccessLock); // Check to make sure that our configuration file exists // if (statFile(&fileInfo) < 0) { - mgmt_log("[ConfigManager::ConfigManager] %s Unable to load: %s", fileName, strerror(errno)); + mgmt_log("[ConfigManager::ConfigManager] %s Unable to load: %s", fileName.c_str(), strerror(errno)); if (isRequired) { - mgmt_fatal(0, "[ConfigManager::ConfigManager] Unable to open required configuration file %s.\n\tStat failed : %s\n", fileName, - strerror(errno)); + mgmt_fatal(0, "[ConfigManager::ConfigManager] Unable to open required configuration file %s.\n\tStat failed : %s\n", + fileName.c_str(), strerror(errno)); } } else { fileLastModified = TS_ARCHIVE_STAT_MTIME(fileInfo); } } -ConfigManager::~ConfigManager() -{ - ats_free(fileName); -} - // // // int ConfigManager::statFile() @@ -118,9 +113,9 @@ ConfigManager::checkForUserUpdate(RollBackCheckType how) if (how == ROLLBACK_CHECK_AND_UPDATE) { fileLastModified = TS_ARCHIVE_STAT_MTIME(fileInfo); if (!this->isChildManaged()) { - configFiles->fileChanged(fileName, configName); + configFiles->fileChanged(fileName.c_str(), configName.c_str()); } - mgmt_log("User has changed config file %s\n", fileName); + mgmt_log("User has changed config file %s\n", fileName.c_str()); } result = true; } else { diff --git a/mgmt/ConfigManager.h b/mgmt/ConfigManager.h index b10a9ae3cfe..f9210616a1d 100644 --- a/mgmt/ConfigManager.h +++ b/mgmt/ConfigManager.h @@ -26,6 +26,8 @@ #include "tscore/ink_mutex.h" #include "tscore/List.h" +#include + class FileManager; class TextBuffer; @@ -56,7 +58,7 @@ class ConfigManager // fileName_ should be rooted or a base file name. ConfigManager(const char *fileName_, const char *configName_, bool root_access_needed, bool isRequired_, ConfigManager *parentConfig_); - ~ConfigManager(); + ~ConfigManager() = default; // Manual take out of lock required void @@ -78,13 +80,13 @@ class ConfigManager const char * getFileName() const { - return fileName; + return fileName.c_str(); } const char * getConfigName() const { - return configName; + return configName.c_str(); } bool @@ -121,8 +123,8 @@ class ConfigManager int statFile(struct stat *buf); ink_mutex fileAccessLock; - char *fileName; - char *configName; + std::string fileName; + std::string configName; bool root_access_needed; bool isRequired; ConfigManager *parentConfig; diff --git a/src/traffic_manager/AddConfigFilesHere.cc b/src/traffic_manager/AddConfigFilesHere.cc index 4e49476d8de..ec2a0f208cc 100644 --- a/src/traffic_manager/AddConfigFilesHere.cc +++ b/src/traffic_manager/AddConfigFilesHere.cc @@ -47,12 +47,13 @@ testcall(char *foo, char * /*configName */) void registerFile(const char *configName, const char *defaultName, bool isRequired, bool isElevateNeeded = false) { - bool found = false; - const char *fname = REC_readString(configName, &found); + bool found = false; + char *fname = REC_readString(configName, &found); if (!found) { - fname = defaultName; + fname = ats_strdup(defaultName); } configFiles->addFile(fname, configName, isElevateNeeded, isRequired); + ats_free(fname); } //