diff --git a/plugins/experimental/jax_fingerprint/plugin.cc b/plugins/experimental/jax_fingerprint/plugin.cc index 374d66faecb..1980c2fca11 100644 --- a/plugins/experimental/jax_fingerprint/plugin.cc +++ b/plugins/experimental/jax_fingerprint/plugin.cc @@ -373,16 +373,16 @@ TSPluginInit(int argc, char const **argv) return; } - PluginConfig *config = new PluginConfig(); - config->plugin_type = PluginType::GLOBAL; + auto owned_config = std::make_unique(); + 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 { @@ -390,6 +390,17 @@ TSPluginInit(int argc, char const **argv) } } + // 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,25 +451,23 @@ 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(); + owned_config->plugin_type = PluginType::REMAP; // Parse parameters - if (!read_config_option(argc - 1, const_cast(argv + 1), *config)) { - delete config; + if (!read_config_option(argc - 1, const_cast(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 { @@ -471,11 +475,14 @@ TSRemapNewInstance(int argc, char *argv[], void **ih, char * /* errbuf ATS_UNUSE } } - 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."); diff --git a/plugins/experimental/maxmind_acl/mmdb.cc b/plugins/experimental/maxmind_acl/mmdb.cc index 26aa8070472..4b41c459caa 100644 --- a/plugins/experimental/maxmind_acl/mmdb.cc +++ b/plugins/experimental/maxmind_acl/mmdb.cc @@ -79,10 +79,12 @@ 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); @@ -90,6 +92,7 @@ Acl::init(char const *filename) 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"]); diff --git a/plugins/experimental/stale_response/stale_response.cc b/plugins/experimental/stale_response/stale_response.cc index 80b27082e61..9611cd671e2 100644 --- a/plugins/experimental/stale_response/stale_response.cc +++ b/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__, diff --git a/plugins/experimental/stale_response/stale_response.h b/plugins/experimental/stale_response/stale_response.h index 8d9f9be8fff..704ed5fcbde 100644 --- a/plugins/experimental/stale_response/stale_response.h +++ b/plugins/experimental/stale_response/stale_response.h @@ -31,6 +31,7 @@ #include "BodyData.h" #include +#include #include 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(this->log_info.filename)); - } } UintBodyMap *body_data = nullptr; TSMutex body_data_mutex; diff --git a/plugins/experimental/uri_signing/config.cc b/plugins/experimental/uri_signing/config.cc index 36435b9db5f..f489ff12832 100644 --- a/plugins/experimental/uri_signing/config.cc +++ b/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(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); } } diff --git a/plugins/regex_revalidate/regex_revalidate.cc b/plugins/regex_revalidate/regex_revalidate.cc index 5e8d8cf2f7b..e3605b70005 100644 --- a/plugins/regex_revalidate/regex_revalidate.cc +++ b/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: diff --git a/plugins/remap_purge/remap_purge.cc b/plugins/remap_purge/remap_purge.cc index fd0b8198fbe..eba0fce5ece 100644 --- a/plugins/remap_purge/remap_purge.cc +++ b/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; } diff --git a/plugins/xdebug/xdebug.cc b/plugins/xdebug/xdebug.cc index 32cd8631372..c131d3317eb 100644 --- a/plugins/xdebug/xdebug.cc +++ b/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(xDebugHeader.str)); + TSUserArgSet(nullptr, idx, xDebugHeader.str); AuxDataMgr::init("xdebug"); diff --git a/src/api/InkAPITest.cc b/src/api/InkAPITest.cc index eac60ccd3fc..881f0d91ebb 100644 --- a/src/api/InkAPITest.cc +++ b/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); diff --git a/src/proxy/http/remap/RemapYamlConfig.cc b/src/proxy/http/remap/RemapYamlConfig.cc index 7a441434463..1016c468c10 100644 --- a/src/proxy/http/remap/RemapYamlConfig.cc +++ b/src/proxy/http/remap/RemapYamlConfig.cc @@ -35,6 +35,7 @@ #include #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(), "") || !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"); diff --git a/src/traffic_cache_tool/CacheTool.cc b/src/traffic_cache_tool/CacheTool.cc index c28c2111c42..2d1e9187108 100644 --- a/src/traffic_cache_tool/CacheTool.cc +++ b/src/traffic_cache_tool/CacheTool.cc @@ -208,7 +208,7 @@ struct Cache { std::map _volumes; std::vector globalVec_stripe; std::unordered_set URLset; - unsigned short *stripes_hash_table; + ats_scoped_mem 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);