From: Evan Hunt Date: Wed, 29 Oct 2025 01:28:35 +0000 (-0700) Subject: fix a "max-cache-size" configuration bug X-Git-Tag: v9.21.15~24^2~2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=cd921cc7ef97404de941c9ac2f9fc14d63b75ef1;p=thirdparty%2Fbind9.git 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. --- diff --git a/bin/named/config.c b/bin/named/config.c index 00006a8522f..98706984022 100644 --- a/bin/named/config.c +++ b/bin/named/config.c @@ -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\ diff --git a/bin/named/server.c b/bin/named/server.c index 5d7f74cb193..c3dca714b21 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -3742,42 +3742,6 @@ named_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 }; /* @@ -4040,42 +4004,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); - } else { - uint64_t value = cfg_obj_asuint64(obj); - if (value > SIZE_MAX) { - cfg_obj_log(obj, 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 = named_g_maxcachesize; + } else { + 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, 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) { @@ -4879,11 +4861,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); diff --git a/lib/isccfg/namedconf.c b/lib/isccfg/namedconf.c index 5d0117ac5ac..4b7a203ddb4 100644 --- a/lib/isccfg/namedconf.c +++ b/lib/isccfg/namedconf.c @@ -111,6 +111,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; @@ -138,7 +139,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; @@ -2091,7 +2091,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 }, @@ -2932,14 +2932,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); @@ -2948,10 +2948,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 }; /*%