Skip to content

Commit

Permalink
Fix leaks in ConfigManager::configName (#8269)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
bneradt authored Aug 31, 2021
1 parent 947d78b commit ee820c7
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 20 deletions.
19 changes: 7 additions & 12 deletions mgmt/ConfigManager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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 {
Expand Down
12 changes: 7 additions & 5 deletions mgmt/ConfigManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
#include "tscore/ink_mutex.h"
#include "tscore/List.h"

#include <string>

class FileManager;
class TextBuffer;

Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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;
Expand Down
7 changes: 4 additions & 3 deletions src/traffic_manager/AddConfigFilesHere.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

//
Expand Down

0 comments on commit ee820c7

Please sign in to comment.