From 7cced1732d5b6de1efe65862fe049cf2a136ef59 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Wed, 7 Feb 2024 12:56:39 +1100 Subject: [PATCH 1/4] cleanup isc_symtab_undefine callers isc_symtab_undefine now only return ISC_R_SUCCESS and ISC_R_EXISTS. Cleanup callers looking for other values. --- lib/isccfg/check.c | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/lib/isccfg/check.c b/lib/isccfg/check.c index 66d24f5ed7..2dcead8ec7 100644 --- a/lib/isccfg/check.c +++ b/lib/isccfg/check.c @@ -478,8 +478,6 @@ exists(const cfg_obj_t *obj, const char *name, int value, isc_symtab_t *symtab, cfg_obj_log(obj, logctx, ISC_LOG_ERROR, fmt, key, file, line); isc_mem_free(mctx, key); result = ISC_R_EXISTS; - } else if (result != ISC_R_SUCCESS) { - isc_mem_free(mctx, key); } return (result); } @@ -2146,10 +2144,6 @@ check_remoteserverlist(const cfg_obj_t *cctx, const char *list, isc_mem_free(mctx, tmp); result = tresult; break; - } else if (tresult != ISC_R_SUCCESS) { - isc_mem_free(mctx, tmp); - result = tresult; - break; } elt = cfg_list_next(elt); @@ -3305,9 +3299,6 @@ check_zoneconf(const cfg_obj_t *zconfig, const cfg_obj_t *voptions, tresult = isc_symtab_define( inview, tmp, 1, symvalue, isc_symexists_replace); - if (tresult == ISC_R_NOMEMORY) { - isc_mem_free(mctx, tmp); - } if (result == ISC_R_SUCCESS && tresult != ISC_R_SUCCESS) { @@ -4984,10 +4975,6 @@ record_ds_keys(isc_symtab_t *symtab, isc_mem_t *mctx, isc_symexists_reject); if (result == ISC_R_EXISTS) { isc_mem_free(mctx, p); - } else if (result != ISC_R_SUCCESS) { - isc_mem_free(mctx, p); - ret = result; - continue; } } @@ -6259,8 +6246,6 @@ isccfg_check_namedconf(const cfg_obj_t *config, unsigned int flags, "previous definition: %s:%u", key, file, line); result = tresult; - } else if (tresult != ISC_R_SUCCESS) { - result = tresult; } else if ((strcasecmp(key, "_bind") == 0 && vclass == dns_rdataclass_ch) || (strcasecmp(key, "_default") == 0 && From 95de7f829cde5f3cbb66ce65a2af8f48ee2a9259 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Fri, 5 Jan 2024 17:36:00 +1100 Subject: [PATCH 2/4] Ensure keyname buffer is big enough Use a temporary string rather than a fixed buffer to construct the keyname. --- lib/isccfg/check.c | 49 ++++++++++++++++++++++++---------------------- 1 file changed, 26 insertions(+), 23 deletions(-) diff --git a/lib/isccfg/check.c b/lib/isccfg/check.c index 2dcead8ec7..05fcddee6c 100644 --- a/lib/isccfg/check.c +++ b/lib/isccfg/check.c @@ -3236,12 +3236,17 @@ check_zoneconf(const cfg_obj_t *zconfig, const cfg_obj_t *voptions, "zone '%s': is not a valid name", znamestr); result = ISC_R_FAILURE; } else { - char namebuf[DNS_NAME_FORMATSIZE + 128]; - char *tmp = namebuf; - size_t len = sizeof(namebuf); + char namebuf[DNS_NAME_FORMATSIZE]; + char classbuf[DNS_RDATACLASS_FORMATSIZE]; + char *key = NULL; + const char *vname = NULL; + size_t len = 0; + int n; zname = dns_fixedname_name(&fixedname); dns_name_format(zname, namebuf, sizeof(namebuf)); + dns_rdataclass_format(zclass, classbuf, sizeof(classbuf)); + tresult = exists( zconfig, namebuf, ztype == CFG_ZONE_HINT ? 1 @@ -3260,15 +3265,16 @@ check_zoneconf(const cfg_obj_t *zconfig, const cfg_obj_t *voptions, } else if (dns_name_isula(zname)) { ula = true; } - len -= strlen(tmp); - tmp += strlen(tmp); - (void)snprintf(tmp, len, "%u/%s", zclass, - (ztype == CFG_ZONE_INVIEW) ? target - : (viewname != NULL) ? viewname - : "_default"); + vname = (ztype == CFG_ZONE_INVIEW) ? target + : (viewname != NULL) ? viewname + : "_default"; + len = strlen(namebuf) + strlen(classbuf) + strlen(vname) + 3; + key = isc_mem_get(mctx, len); + n = snprintf(key, len, "%s/%s/%s", namebuf, classbuf, vname); + RUNTIME_CHECK(n > 0 && (size_t)n < len); switch (ztype) { case CFG_ZONE_INVIEW: - tresult = isc_symtab_lookup(inview, namebuf, 0, NULL); + tresult = isc_symtab_lookup(inview, key, 0, NULL); if (tresult != ISC_R_SUCCESS) { cfg_obj_log(inviewobj, logctx, ISC_LOG_ERROR, "'in-view' zone '%s' " @@ -3291,25 +3297,22 @@ check_zoneconf(const cfg_obj_t *zconfig, const cfg_obj_t *voptions, case CFG_ZONE_MIRROR: case CFG_ZONE_HINT: case CFG_ZONE_STUB: - case CFG_ZONE_STATICSTUB: - tmp = isc_mem_strdup(mctx, namebuf); + case CFG_ZONE_STATICSTUB: { + char *tmp = isc_mem_strdup(mctx, key); + isc_symvalue_t symvalue; + symvalue.as_cpointer = NULL; + tresult = isc_symtab_define(inview, tmp, 1, symvalue, + isc_symexists_replace); + if (result == ISC_R_SUCCESS && tresult != ISC_R_SUCCESS) { - isc_symvalue_t symvalue; - symvalue.as_cpointer = NULL; - tresult = isc_symtab_define( - inview, tmp, 1, symvalue, - isc_symexists_replace); - if (result == ISC_R_SUCCESS && - tresult != ISC_R_SUCCESS) - { - result = tresult; - } + result = tresult; } - break; + } break; default: UNREACHABLE(); } + isc_mem_put(mctx, key, len); } if (ztype == CFG_ZONE_INVIEW) { From 1fb61494a80912e3663567dfcef7bee597cba7a4 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Wed, 7 Feb 2024 13:34:39 +1100 Subject: [PATCH 3/4] Add RUNTIME_CHECK --- lib/isccfg/check.c | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/isccfg/check.c b/lib/isccfg/check.c index 05fcddee6c..90961048b1 100644 --- a/lib/isccfg/check.c +++ b/lib/isccfg/check.c @@ -4237,6 +4237,7 @@ keydirexist(const cfg_obj_t *zcfg, const char *optname, dns_name_t *zname, symvalue.as_cpointer = zcfg; result = isc_symtab_define(symtab, symkey, 2, symvalue, isc_symexists_reject); + RUNTIME_CHECK(result == ISC_R_SUCCESS); return (result); } From 2f87c429a29084d7d2e3d855f99c75ddf28692c3 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Wed, 7 Feb 2024 13:52:10 +1100 Subject: [PATCH 4/4] cleanup isc_symtab_define with isc_symexists_replace --- lib/isccfg/check.c | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/lib/isccfg/check.c b/lib/isccfg/check.c index 90961048b1..93ce7448fa 100644 --- a/lib/isccfg/check.c +++ b/lib/isccfg/check.c @@ -3303,10 +3303,7 @@ check_zoneconf(const cfg_obj_t *zconfig, const cfg_obj_t *voptions, symvalue.as_cpointer = NULL; tresult = isc_symtab_define(inview, tmp, 1, symvalue, isc_symexists_replace); - if (result == ISC_R_SUCCESS && tresult != ISC_R_SUCCESS) - { - result = tresult; - } + RUNTIME_CHECK(tresult == ISC_R_SUCCESS); } break; default: @@ -5879,9 +5876,7 @@ check_logging(const cfg_obj_t *config, isc_log_t *logctx, isc_mem_t *mctx) { for (i = 0; default_channels[i] != NULL; i++) { tresult = isc_symtab_define(symtab, default_channels[i], 1, symvalue, isc_symexists_replace); - if (tresult != ISC_R_SUCCESS) { - result = tresult; - } + RUNTIME_CHECK(tresult == ISC_R_SUCCESS); } cfg_map_get(logobj, "channel", &channels); @@ -5919,9 +5914,7 @@ check_logging(const cfg_obj_t *config, isc_log_t *logctx, isc_mem_t *mctx) { } tresult = isc_symtab_define(symtab, channelname, 1, symvalue, isc_symexists_replace); - if (tresult != ISC_R_SUCCESS) { - result = tresult; - } + RUNTIME_CHECK(tresult == ISC_R_SUCCESS); } cfg_map_get(logobj, "category", &categories);