fix a "max-cache-size" configuration bug

"max-cache-size default;" is allowed, according to the documentation
and the parser, but when it's configured, named crashes due to an
INSIST that the only legal string value is "unlimited". this has
been fied.

the configuration has also been simplified. previously, we checked for
max-cache-size in view and options, then determined whether to look in
the global default options based on whether the view had recursion set.
the default value set there was only applicable to views with recursion.
now, the default is an explicit "default", which affects views with
and without recursion in different ways.

the cfg type for "max-cache-size" has been changed from
cfg_type_sizeorpercent to cfg_type_maxcachesize.

(cherry picked from commit cd921cc7ef)
This commit is contained in:
Evan Hunt 2025-10-28 21:30:21 -07:00
parent 895d35d4db
commit 5ed3bebbf4
3 changed files with 58 additions and 81 deletions

View file

@ -168,7 +168,7 @@ options {\n\
#ifdef HAVE_LMDB
" lmdb-mapsize 32M;\n"
#endif /* ifdef HAVE_LMDB */
" max-cache-size 90%;\n\
" max-cache-size default;\n\
max-cache-ttl 604800; /* 1 week */\n\
max-clients-per-query 100;\n\
max-ncache-ttl 10800; /* 3 hours */\n\

View file

@ -4148,42 +4148,6 @@ register_one_plugin(const cfg_obj_t *config, const cfg_obj_t *obj,
return result;
}
/*
* Determine if a minimal-sized cache can be used for a given view, according
* to 'maps' (implicit defaults, global options, view options) and 'optionmaps'
* (global options, view options). This is only allowed for views which have
* recursion disabled and do not have "max-cache-size" set explicitly. Using
* minimal-sized caches prevents a situation in which all explicitly configured
* and built-in views inherit the default "max-cache-size 90%;" setting, which
* could lead to memory exhaustion with multiple views configured.
*/
static bool
minimal_cache_allowed(const cfg_obj_t *maps[4],
const cfg_obj_t *optionmaps[3]) {
const cfg_obj_t *obj;
/*
* Do not use a minimal-sized cache for a view with recursion enabled.
*/
obj = NULL;
(void)named_config_get(maps, "recursion", &obj);
INSIST(obj != NULL);
if (cfg_obj_asboolean(obj)) {
return false;
}
/*
* Do not use a minimal-sized cache if a specific size was requested.
*/
obj = NULL;
(void)named_config_get(optionmaps, "max-cache-size", &obj);
if (obj != NULL) {
return false;
}
return true;
}
static const char *const response_synonyms[] = { "response", NULL };
/*
@ -4458,42 +4422,60 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config,
* we can reuse/share an existing cache.
*/
obj = NULL;
result = named_config_get(maps, "max-cache-size", &obj);
result = named_config_get(maps, "recursion", &obj);
INSIST(result == ISC_R_SUCCESS);
/*
* If "-T maxcachesize=..." is in effect, it overrides any other
* "max-cache-size" setting found in configuration, either implicit or
* explicit. For simplicity, the value passed to that command line
* option is always treated as the number of bytes to set
* "max-cache-size" to.
*/
view->recursion = cfg_obj_asboolean(obj);
if (named_g_maxcachesize != 0) {
max_cache_size = named_g_maxcachesize;
} else if (minimal_cache_allowed(maps, optionmaps)) {
/*
* dns_cache_setcachesize() will adjust this to the smallest
* allowed value.
* If "-T maxcachesize=..." is in effect, it overrides any
* other "max-cache-size" setting found in configuration,
* either implicit or explicit. For simplicity, the value
* passed to that command line option is always treated as
* the number of bytes to set "max-cache-size" to.
*/
max_cache_size = 1;
} else if (cfg_obj_isstring(obj)) {
str = cfg_obj_asstring(obj);
INSIST(strcasecmp(str, "unlimited") == 0);
max_cache_size = 0;
} else if (cfg_obj_ispercentage(obj)) {
max_cache_size = SIZE_AS_PERCENT;
max_cache_size_percent = cfg_obj_aspercentage(obj);
max_cache_size = named_g_maxcachesize;
} else {
uint64_t value = cfg_obj_asuint64(obj);
if (value > SIZE_MAX) {
cfg_obj_log(obj, named_g_lctx, ISC_LOG_WARNING,
"'max-cache-size "
"%" PRIu64 "' "
"is too large for this "
"system; reducing to %lu",
value, (unsigned long)SIZE_MAX);
value = SIZE_MAX;
obj = NULL;
result = named_config_get(maps, "max-cache-size", &obj);
INSIST(result == ISC_R_SUCCESS);
if (cfg_obj_isstring(obj) &&
strcasecmp(cfg_obj_asstring(obj), "default") == 0)
{
/*
* The default for a view with recursion
* is 90% of memory. With no recursion,
* it's the minimum cache size allowed by
* dns_cache_setcachesize().
*/
if (view->recursion) {
max_cache_size = SIZE_AS_PERCENT;
max_cache_size_percent = 90;
} else {
max_cache_size = 1;
}
} else if (cfg_obj_isstring(obj)) {
str = cfg_obj_asstring(obj);
INSIST(strcasecmp(str, "unlimited") == 0);
max_cache_size = 0;
} else if (cfg_obj_ispercentage(obj)) {
max_cache_size = SIZE_AS_PERCENT;
max_cache_size_percent = cfg_obj_aspercentage(obj);
} else if (cfg_obj_isuint64(obj)) {
uint64_t value = cfg_obj_asuint64(obj);
if (value > SIZE_MAX) {
cfg_obj_log(obj, named_g_lctx, ISC_LOG_WARNING,
"'max-cache-size "
"%" PRIu64 "' "
"is too large for this "
"system; reducing to %lu",
value, (unsigned long)SIZE_MAX);
value = SIZE_MAX;
}
max_cache_size = (size_t)value;
} else {
UNREACHABLE();
}
max_cache_size = (size_t)value;
}
if (max_cache_size == SIZE_AS_PERCENT) {
@ -5332,11 +5314,6 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config,
/*
* Configure other configurable data.
*/
obj = NULL;
result = named_config_get(maps, "recursion", &obj);
INSIST(result == ISC_R_SUCCESS);
view->recursion = cfg_obj_asboolean(obj);
obj = NULL;
result = named_config_get(maps, "qname-minimization", &obj);
INSIST(result == ISC_R_SUCCESS);

View file

@ -112,6 +112,7 @@ static cfg_type_t cfg_type_logseverity;
static cfg_type_t cfg_type_logsuffix;
static cfg_type_t cfg_type_logversions;
static cfg_type_t cfg_type_remoteselement;
static cfg_type_t cfg_type_maxcachesize;
static cfg_type_t cfg_type_maxduration;
static cfg_type_t cfg_type_minimal;
static cfg_type_t cfg_type_nameportiplist;
@ -139,7 +140,6 @@ static cfg_type_t cfg_type_server;
static cfg_type_t cfg_type_server_key_kludge;
static cfg_type_t cfg_type_size;
static cfg_type_t cfg_type_sizenodefault;
static cfg_type_t cfg_type_sizeorpercent;
static cfg_type_t cfg_type_sizeval;
static cfg_type_t cfg_type_sockaddr4wild;
static cfg_type_t cfg_type_sockaddr6wild;
@ -2221,7 +2221,7 @@ static cfg_clausedef_t view_clauses[] = {
{ "lmdb-mapsize", &cfg_type_sizeval, CFG_CLAUSEFLAG_NOTCONFIGURED },
#endif /* ifdef HAVE_LMDB */
{ "max-acache-size", NULL, CFG_CLAUSEFLAG_ANCIENT },
{ "max-cache-size", &cfg_type_sizeorpercent, 0 },
{ "max-cache-size", &cfg_type_maxcachesize, 0 },
{ "max-cache-ttl", &cfg_type_duration, 0 },
{ "max-clients-per-query", &cfg_type_uint32, 0 },
{ "max-ncache-ttl", &cfg_type_duration, 0 },
@ -3027,14 +3027,14 @@ static cfg_type_t cfg_type_sizeval_percent = {
*/
static isc_result_t
parse_size_or_percent(cfg_parser_t *pctx, const cfg_type_t *type,
cfg_obj_t **ret) {
parse_maxcachesize(cfg_parser_t *pctx, const cfg_type_t *type,
cfg_obj_t **ret) {
return cfg_parse_enum_or_other(pctx, type, &cfg_type_sizeval_percent,
ret);
}
static void
doc_parse_size_or_percent(cfg_printer_t *pctx, const cfg_type_t *type) {
doc_maxcachesize(cfg_printer_t *pctx, const cfg_type_t *type) {
UNUSED(type);
cfg_print_cstr(pctx, "( default | unlimited | ");
cfg_doc_terminal(pctx, &cfg_type_sizeval);
@ -3043,10 +3043,10 @@ doc_parse_size_or_percent(cfg_printer_t *pctx, const cfg_type_t *type) {
cfg_print_cstr(pctx, " )");
}
static const char *sizeorpercent_enums[] = { "default", "unlimited", NULL };
static cfg_type_t cfg_type_sizeorpercent = {
"size_or_percent", parse_size_or_percent, cfg_print_ustring,
doc_parse_size_or_percent, &cfg_rep_string, sizeorpercent_enums
static const char *maxcachesize_enums[] = { "default", "unlimited", NULL };
static cfg_type_t cfg_type_maxcachesize = {
"maxcachesize", parse_maxcachesize, cfg_print_ustring,
doc_maxcachesize, &cfg_rep_string, maxcachesize_enums
};
/*%