From f5074c0c8ec72feb598ca122cb1f7df3f26eb056 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Tue, 4 Jan 2022 11:48:25 -0800 Subject: [PATCH 1/2] Ensure that cache pointer is set to NULL by isc_tlsctx_cache_detach() If the reference count was higher than 1, detaching a tlsctx cache didn't clear the pointer, which could trigger an assertion later. --- lib/isc/tls.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/lib/isc/tls.c b/lib/isc/tls.c index cf781a2302..2cc237997b 100644 --- a/lib/isc/tls.c +++ b/lib/isc/tls.c @@ -963,17 +963,23 @@ tlsctx_cache_entry_destroy(isc_mem_t *mctx, isc_tlsctx_cache_entry_t *entry) { void isc_tlsctx_cache_detach(isc_tlsctx_cache_t **pcache) { - isc_tlsctx_cache_t *cache; + isc_tlsctx_cache_t *cache = NULL; isc_ht_iter_t *it = NULL; isc_result_t result; + REQUIRE(pcache != NULL); + cache = *pcache; + *pcache = NULL; + REQUIRE(VALID_TLSCTX_CACHE(cache)); if (isc_refcount_decrement(&cache->references) > 1) { return; } + cache->magic = 0; + RUNTIME_CHECK(isc_ht_iter_create(cache->data, &it) == ISC_R_SUCCESS); for (result = isc_ht_iter_first(it); result == ISC_R_SUCCESS; result = isc_ht_iter_delcurrent_next(it)) @@ -982,13 +988,12 @@ isc_tlsctx_cache_detach(isc_tlsctx_cache_t **pcache) { isc_ht_iter_current(it, (void **)&entry); tlsctx_cache_entry_destroy(cache->mctx, entry); } + isc_ht_iter_destroy(&it); isc_ht_destroy(&cache->data); isc_rwlock_destroy(&cache->rwlock); - cache->magic = 0; isc_mem_putanddetach(&cache->mctx, cache, sizeof(*cache)); - *pcache = NULL; } isc_result_t From 61c160c4a5f89a1782ac76d785d13e197478d162 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Tue, 4 Jan 2022 13:02:44 -0800 Subject: [PATCH 2/2] Clean up isc_tlsctx_cache_detach() For consistency with similar functions, rename `pcache` to `cachep`, call a separate destroy function when references reach 0, and add a missing call to isc_refcount_destroy(). --- lib/isc/include/isc/tls.h | 6 +++--- lib/isc/tls.c | 35 ++++++++++++++++++++--------------- 2 files changed, 23 insertions(+), 18 deletions(-) diff --git a/lib/isc/include/isc/tls.h b/lib/isc/include/isc/tls.h index ec7382901d..e79e40e12d 100644 --- a/lib/isc/include/isc/tls.h +++ b/lib/isc/include/isc/tls.h @@ -240,13 +240,13 @@ isc_tlsctx_cache_attach(isc_tlsctx_cache_t *source, */ void -isc_tlsctx_cache_detach(isc_tlsctx_cache_t **pcache); +isc_tlsctx_cache_detach(isc_tlsctx_cache_t **cachep); /*%< * Remove a reference to the TLS context cache object. * * Requires: - *\li 'pcache' is a valid pointer to a pointer which must point to a - * valid TLS context cache object. + *\li 'cachep' is a pointer to a pointer to a valid TLS + * context cache object. */ isc_result_t diff --git a/lib/isc/tls.c b/lib/isc/tls.c index 2cc237997b..cc2882de95 100644 --- a/lib/isc/tls.c +++ b/lib/isc/tls.c @@ -961,25 +961,15 @@ tlsctx_cache_entry_destroy(isc_mem_t *mctx, isc_tlsctx_cache_entry_t *entry) { isc_mem_put(mctx, entry, sizeof(*entry)); } -void -isc_tlsctx_cache_detach(isc_tlsctx_cache_t **pcache) { - isc_tlsctx_cache_t *cache = NULL; +static void +tlsctx_cache_destroy(isc_tlsctx_cache_t *cache) { isc_ht_iter_t *it = NULL; isc_result_t result; - REQUIRE(pcache != NULL); - - cache = *pcache; - *pcache = NULL; - - REQUIRE(VALID_TLSCTX_CACHE(cache)); - - if (isc_refcount_decrement(&cache->references) > 1) { - return; - } - cache->magic = 0; + isc_refcount_destroy(&cache->references); + RUNTIME_CHECK(isc_ht_iter_create(cache->data, &it) == ISC_R_SUCCESS); for (result = isc_ht_iter_first(it); result == ISC_R_SUCCESS; result = isc_ht_iter_delcurrent_next(it)) @@ -991,11 +981,26 @@ isc_tlsctx_cache_detach(isc_tlsctx_cache_t **pcache) { isc_ht_iter_destroy(&it); isc_ht_destroy(&cache->data); - isc_rwlock_destroy(&cache->rwlock); isc_mem_putanddetach(&cache->mctx, cache, sizeof(*cache)); } +void +isc_tlsctx_cache_detach(isc_tlsctx_cache_t **cachep) { + isc_tlsctx_cache_t *cache = NULL; + + REQUIRE(cachep != NULL); + + cache = *cachep; + *cachep = NULL; + + REQUIRE(VALID_TLSCTX_CACHE(cache)); + + if (isc_refcount_decrement(&cache->references) == 1) { + tlsctx_cache_destroy(cache); + } +} + isc_result_t isc_tlsctx_cache_add(isc_tlsctx_cache_t *cache, const char *name, const isc_tlsctx_cache_transport_t transport,