Fix memory leaks reported by Coverity Scan

plugins/experimental/jax_fingerprint/plugin.cc

@@ -373,23 +373,34 @@ TSPluginInit(int argc, char const **argv)
     return;
   }
 
-  PluginConfig *config = new PluginConfig();
-  config->plugin_type  = PluginType::GLOBAL;
+  auto owned_config         = std::make_unique<PluginConfig>();
+  owned_config->plugin_type = PluginType::GLOBAL;
 
-  if (!read_config_option(argc, argv, *config)) {
+  if (!read_config_option(argc, argv, *owned_config)) {
     TSError("[%s] Failed to parse options.", PLUGIN_NAME);
     return;
   }
 
-  if (!config->log_filename.empty()) {
-    if (!create_log_file(config->log_filename, config->log_handle)) {
+  if (!owned_config->log_filename.empty()) {
+    if (!create_log_file(owned_config->log_filename, owned_config->log_handle)) {
       TSError("[%s] Failed to create log.", PLUGIN_NAME);
       return;
     } else {
       Dbg(dbg_ctl, "Created log file.");
     }
   }
 
+  // Reserve the index before registering the log field, so that every failure exit happens while the
+  // configuration is still owned here and nothing has taken a reference to it yet.
+  if (reserve_user_arg(*owned_config) == TS_ERROR) {
+    TSError("[%s] Failed to reserve user arg index.", PLUGIN_NAME);
+    return;
+  }
+
+  // A global plugin's configuration lives for the life of the process: the log field callback and the
+  // continuation below both keep a reference to it, so release it from the unique_ptr here.
+  PluginConfig *config = owned_config.release();
+
   if (!config->log_symbol.empty()) {
     std::string name  = "jax_fingerprint-";
     name             += config->method.name;
@@ -412,11 +423,6 @@ TSPluginInit(int argc, char const **argv)
       TSLogIntUnmarshal);
   }
 
-  if (reserve_user_arg(*config) == TS_ERROR) {
-    TSError("[%s] Failed to reserve user arg index.", PLUGIN_NAME);
-    return;
-  }
-
   TSCont cont = TSContCreate(main_handler, nullptr);
   TSContDataSet(cont, config);
   if (config->method.on_client_hello) {
@@ -445,37 +451,38 @@ TSReturnCode
 TSRemapNewInstance(int argc, char *argv[], void **ih, char * /* errbuf ATS_UNUSED */, int /* errbuf_size ATS_UNUSED */)
 {
   Dbg(dbg_ctl, "New instance for client matching %s to %s", argv[0], argv[1]);
-  auto config         = new PluginConfig();
-  config->plugin_type = PluginType::REMAP;
+  auto owned_config         = std::make_unique<PluginConfig>();
+  owned_config->plugin_type = PluginType::REMAP;
 
   // Parse parameters
-  if (!read_config_option(argc - 1, const_cast<const char **>(argv + 1), *config)) {
-    delete config;
+  if (!read_config_option(argc - 1, const_cast<const char **>(argv + 1), *owned_config)) {
     Dbg(dbg_ctl, "Bad arguments");
     return TS_ERROR;
   }
 
-  if (!config->log_symbol.empty()) {
+  if (!owned_config->log_symbol.empty()) {
     TSError("[%s] --log-field is not supported in remap.config. Use it in plugin.config instead.", PLUGIN_NAME);
-    delete config;
     return TS_ERROR;
   }
 
   // Create a log file
-  if (!config->log_filename.empty()) {
-    if (!create_log_file(config->log_filename, config->log_handle)) {
+  if (!owned_config->log_filename.empty()) {
+    if (!create_log_file(owned_config->log_filename, owned_config->log_handle)) {
       TSError("[%s] Failed to create log.", PLUGIN_NAME);
       return TS_ERROR;
     } else {
       Dbg(dbg_ctl, "Created log file.");
     }
   }
 
-  if (reserve_user_arg(*config) == TS_ERROR) {
+  if (reserve_user_arg(*owned_config) == TS_ERROR) {
     TSError("[%s] Failed to reserve user arg index.", PLUGIN_NAME);
     return TS_ERROR;
   }
 
+  // Past here the instance handle owns the configuration and TSRemapDeleteInstance releases it.
+  PluginConfig *config = owned_config.release();
+
   // Create continuation
   if (config->standalone) {
     Dbg(dbg_ctl, "Standalone mode. Adding hooks.");

plugins/experimental/maxmind_acl/mmdb.cc

@@ -79,17 +79,20 @@ Acl::init(char const *filename)
   }
 
   // Associate our config file with remap.config or .yaml if possible to be able to initiate reloads
-  TSMgmtString result;
+  TSMgmtString result   = nullptr;
   const char  *var_name = "proxy.config.url_remap_yaml.filename";
   if (TS_SUCCESS != TSMgmtStringGet(var_name, &result) || TS_SUCCESS != TSMgmtConfigFileAdd(result, configloc.c_str())) {
     // Fall back to remap.config
+    TSfree(result);
+    result   = nullptr;
     var_name = "proxy.config.url_remap.filename";
     if (TS_SUCCESS != TSMgmtStringGet(var_name, &result)) {
       TSWarning("[%s] Could not retrieve remap filename", PLUGIN_NAME);
     } else if (TS_SUCCESS != TSMgmtConfigFileAdd(result, configloc.c_str())) {
       TSWarning("[%s] Error adding mgmt config file", PLUGIN_NAME);
     }
   }
+  TSfree(result);
 
   // Find our database name and convert to full path as needed
   status = loaddb(maxmind["database"]);

plugins/experimental/stale_response/stale_response.cc

@@ -1070,7 +1070,8 @@ parse_args(int argc, char const *argv[])
       plugin_config->log_info.stale_if_error = true;
       break;
     case 'd':
-      plugin_config->log_info.filename = strdup(optarg);
+      // Assigning replaces any name from an earlier occurrence of this option.
+      plugin_config->log_info.filename_override = optarg;
       break;
 
     case 'e':
@@ -1108,8 +1109,10 @@ parse_args(int argc, char const *argv[])
   }
 
   if (plugin_config->log_info.all || plugin_config->log_info.stale_while_revalidate || plugin_config->log_info.stale_if_error) {
-    SRDBG(TAG, "[%s] Logging to %s", __FUNCTION__, plugin_config->log_info.filename);
-    TSTextLogObjectCreate(plugin_config->log_info.filename, TS_LOG_MODE_ADD_TIMESTAMP, &(plugin_config->log_info.object));
+    char const *const log_filename =
+      plugin_config->log_info.filename_override.empty() ? PLUGIN_TAG : plugin_config->log_info.filename_override.c_str();
+    SRDBG(TAG, "[%s] Logging to %s", __FUNCTION__, log_filename);
+    TSTextLogObjectCreate(log_filename, TS_LOG_MODE_ADD_TIMESTAMP, &(plugin_config->log_info.object));
   }
 
   SRDBG(TAG, "[%s] global stale if error override = %" PRIdMAX, __FUNCTION__,

plugins/experimental/stale_response/stale_response.h

@@ -31,6 +31,7 @@
 #include "BodyData.h"
 
 #include <cstdint>
+#include <string>
 #include <map>
 
 struct BodyData;
@@ -49,7 +50,8 @@ struct LogInfo {
   bool            all                    = false;
   bool            stale_if_error         = false;
   bool            stale_while_revalidate = false;
-  char const     *filename               = PLUGIN_TAG;
+  // Empty means log to PLUGIN_TAG; see the effective name computed in parse_args().
+  std::string filename_override;
 };
 
 struct ConfigInfo {
@@ -65,9 +67,6 @@ struct ConfigInfo {
     if (this->body_data_mutex) {
       TSMutexDestroy(this->body_data_mutex);
     }
-    if (this->log_info.filename != PLUGIN_TAG) {
-      free(const_cast<char *>(this->log_info.filename));
-    }
   }
   UintBodyMap *body_data = nullptr;
   TSMutex      body_data_mutex;

plugins/experimental/uri_signing/config.cc

@@ -282,8 +282,9 @@ read_config_from_json(json_t *const issuer_json)
       if (id_json) {
         id = json_string_value(id_json);
         if (id) {
-          cfg->id = static_cast<char *>(malloc(strlen(id) + 1));
-          strcpy(cfg->id, id);
+          /* An earlier issuer may have set an id; free it so it is not leaked. Last issuer wins. */
+          free(cfg->id);
+          cfg->id = strdup(id);
           PluginDebug("Found Id in the config: %s", cfg->id);
         }
       }

plugins/regex_revalidate/regex_revalidate.cc

@@ -778,6 +778,7 @@ TSPluginInit(int argc, const char *argv[])
   while ((c = getopt_long(argc, (char *const *)argv, "c:l:f:m:", longopts, nullptr)) != -1) {
     switch (c) {
     case 'c':
+      TSfree(pstate->config_path); // An option can be repeated, so the earlier value is not leaked
       pstate->config_path = TSstrdup(optarg);
       break;
     case 'l':
@@ -790,9 +791,11 @@ TSPluginInit(int argc, const char *argv[])
       disable_timed_reload = true;
       break;
     case 'f':
+      TSfree(pstate->state_path);
       pstate->state_path = make_state_path(optarg);
       break;
     case 'm':
+      TSfree(pstate->match_header);
       pstate->match_header = TSstrdup(optarg);
       break;
     default:

plugins/remap_purge/remap_purge.cc

@@ -287,17 +287,21 @@ TSRemapNewInstance(int argc, char *argv[], void **ih, char * /* errbuf ATS_UNUSE
       purge->allow_get = true;
       break;
     case 'h':
+      TSfree(purge->header); // An option can be repeated, so the earlier value is not leaked
       purge->header     = TSstrdup(optarg);
       purge->header_len = strlen(purge->header);
       break;
     case 'i':
+      TSfree(purge->id);
       purge->id = TSstrdup(optarg);
       break;
     case 's':
+      TSfree(purge->secret);
       purge->secret     = TSstrdup(optarg);
       purge->secret_len = strlen(purge->secret);
       break;
     case 'f':
+      TSfree(purge->state_file);
       purge->state_file = make_state_path(optarg);
       break;
     }

plugins/xdebug/xdebug.cc

@@ -55,8 +55,8 @@ namespace
 atscppapi::TxnAuxMgrData mgrData;
 
 static struct {
-  const char *str;
-  int         len;
+  char *str;
+  int   len;
 } xDebugHeader = {nullptr, 0};
 
 enum {
@@ -947,6 +947,7 @@ TSPluginInit(int argc, const char *argv[])
     switch (opt) {
     case 'h':
       Dbg(dbg_ctl, "Setting header: %s", optarg);
+      TSfree(xDebugHeader.str); // The option can be repeated, so the earlier value is not leaked
       xDebugHeader.str = TSstrdup(optarg);
       break;
     case 'e':
@@ -974,7 +975,7 @@ TSPluginInit(int argc, const char *argv[])
   auto ret = TSUserArgIndexReserve(TS_USER_ARGS_GLB, "XDebugHeader", "XDebug header name", &idx);
   TSReleaseAssert(ret == TS_SUCCESS);
   TSReleaseAssert(idx >= 0);
-  TSUserArgSet(nullptr, idx, const_cast<char *>(xDebugHeader.str));
+  TSUserArgSet(nullptr, idx, xDebugHeader.str);
 
   AuxDataMgr::init("xdebug");
 

src/api/InkAPITest.cc

@@ -6656,6 +6656,9 @@ REGRESSION_TEST(SDK_API_TSMgmtGet)(RegressionTest *test, int /* atype ATS_UNUSED
     SDK_RPRINT(test, "TSMgmtStringGet", "TestCase1.4", TC_PASS, "ok");
   }
 
+  // TSMgmtStringGet() hands back a copy the caller owns.
+  TSfree(svalue);
+
   {
     TSRecordDataType result;
     auto             ret = TSMgmtDataTypeGet(CONFIG_PARAM_STRING_NAME, &result);

src/proxy/http/remap/RemapYamlConfig.cc

@@ -35,6 +35,7 @@
 #include <vector>
 
 #include "tscore/Diags.h"
+#include "tscore/ink_memory.h"
 #include "tscore/ink_string.h"
 #include "tsutil/ts_errata.h"
 #include "tsutil/PostScript.h"
@@ -388,7 +389,11 @@ parse_map_referer(const YAML::Node &node, url_mapping *url_mapping)
       !strcasecmp(url.c_str(), "<default_redirect_url>") || !strcasecmp(url.c_str(), "default_redirect_url")) {
     url_mapping->default_redirect_url = true;
   }
-  url_mapping->redir_chunk_list = redirect_tag_str::parse_format_redirect_url(ats_strdup(url.c_str()));
+  // parse_format_redirect_url() null-terminates each chunk in place before copying it out, so it
+  // needs a mutable buffer. It keeps no pointer into that buffer, only ats_strdup copies, so the
+  // duplicate can be released as soon as it returns. Previously nothing owned it and it leaked.
+  ats_scoped_str redirect_url(ats_strdup(url.c_str()));
+  url_mapping->redir_chunk_list = redirect_tag_str::parse_format_redirect_url(redirect_url.get());
 
   if (!node["regex"] || !node["regex"].IsSequence()) {
     return swoc::Errata("'regex' field must be sequence");

src/traffic_cache_tool/CacheTool.cc

@@ -208,7 +208,7 @@ struct Cache {
   std::map<int, Volume>              _volumes;
   std::vector<StripeSM *>            globalVec_stripe;
   std::unordered_set<ts::CacheURL *> URLset;
-  unsigned short                    *stripes_hash_table;
+  ats_scoped_mem<unsigned short>     stripes_hash_table;
 };
 
 Errata
@@ -685,7 +685,13 @@ Cache::calcTotalSpanPhysicalSize()
 }
 #endif
 
-Cache::~Cache() {}
+Cache::~Cache()
+{
+  // The URL set is owned solely by this instance; the stripe hash table owns itself.
+  for (auto *url : URLset) {
+    delete url;
+  }
+}
 
 Errata
 Span::load()
@@ -1007,6 +1013,7 @@ Cache::build_stripe_hash_table()
   for (int i = 0; i < num_stripes; i++) {
     printf("build_vol_hash_table index %d mapped to %d requested %d got %d\n", i, i, forvol[i], gotvol[i]);
   }
+  // Assigning releases any table a previous call installed.
   stripes_hash_table = ttable;
 
   ats_free(forvol);