[trafficserver] branch 9.1.x updated: Fix leaks in ConfigManager::configName (#8269) (#8843)

[bc...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

bcall pushed a commit to branch 9.1.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git


The following commit(s) were added to refs/heads/9.1.x by this push:
     new 307fcdc9e Fix leaks in ConfigManager::configName (#8269) (#8843)
307fcdc9e is described below

commit 307fcdc9efb6e3ad2bafe6a25a9619a88cb0c5fd
Author: Randall Meyer <rr...@apache.org>
AuthorDate: Mon Jun 13 10:55:17 2022 -0700

    Fix leaks in ConfigManager::configName (#8269) (#8843)
    
    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.
    
    (cherry picked from commit ee820c73dbe8beef6d44df3e32ffb11438017460)
    
    Conflicts:
        src/traffic_manager/AddConfigFilesHere.cc
---
 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 a34d6d912..2f2a83063 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 b10a9ae3c..f9210616a 100644
--- a/mgmt/ConfigManager.h
+++ b/mgmt/ConfigManager.h
@@ -26,6 +26,8 @@
 #include "tscore/ink_mutex.h"
 #include "tscore/List.h"
 
+#include <string>
+
 class FileManager;
 class TextBuffer;
 
@@ -56,7 +58,7 @@ public:
   // 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 @@ public:
   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 @@ private:
   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 3fdf57207..aec765b6c 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 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, false, isRequired);
+  ats_free(fname);
 }
 
 //