From: Mark Andrews Date: Mon, 3 Aug 2026 05:48:59 +0000 (+1000) Subject: Grow arrays in signset if key->index is too big X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=d5e2019b270754110bcaa1600b225d01f68c74ce;p=thirdparty%2Fbind9.git Grow arrays in signset if key->index is too big The arrays in signset are sized to allow for one additional key per RRSIG however the list of known keys is updated in parallel which means that the index of the returned key can be too big for the arrays. Grow the arrays if this happens. Additionally ensure that the keylist and keycount are locked when they are being read. --- diff --git a/bin/dnssec/dnssec-signzone.c b/bin/dnssec/dnssec-signzone.c index 5197feda639..75f85f31cae 100644 --- a/bin/dnssec/dnssec-signzone.c +++ b/bin/dnssec/dnssec-signzone.c @@ -465,6 +465,30 @@ setverifies(dns_name_t *name, dns_rdataset_t *set, dst_key_t *key, } } +static void +grow_arrays(unsigned int newarraysize, unsigned int *arraysize, + bool **wassignedby, bool **nowsignedby) { + bool *nwsb = isc_mem_cget(isc_g_mctx, newarraysize, sizeof(bool)); + bool *nnsb = isc_mem_cget(isc_g_mctx, newarraysize, sizeof(bool)); + unsigned int i; + + INSIST(newarraysize > *arraysize); + + for (i = 0; i < *arraysize; i++) { + nwsb[i] = (*wassignedby)[i]; + nnsb[i] = (*nowsignedby)[i]; + } + for (; i < newarraysize; i++) { + nwsb[i] = nnsb[i] = false; + } + + isc_mem_cput(isc_g_mctx, *wassignedby, *arraysize, sizeof(bool)); + isc_mem_cput(isc_g_mctx, *nowsignedby, *arraysize, sizeof(bool)); + *wassignedby = nwsb; + *nowsignedby = nnsb; + *arraysize = newarraysize; +} + /*% * Signs a set. Goes through contortions to decide if each RRSIG should * be dropped or retained, and then determines if any new SIGs need to @@ -478,10 +502,10 @@ signset(dns_diff_t *del, dns_diff_t *add, dns_dbnode_t *node, dns_name_t *name, isc_result_t result; bool nosigs = false; bool *wassignedby, *nowsignedby; - int arraysize; + unsigned int arraysize; dns_difftuple_t *tuple; dns_ttl_t ttl; - int i; + unsigned int i; char namestr[DNS_NAME_FORMATSIZE]; char typestr[DNS_RDATATYPE_FORMATSIZE]; char sigstr[SIG_FORMATSIZE]; @@ -507,7 +531,9 @@ signset(dns_diff_t *del, dns_diff_t *add, dns_dbnode_t *node, dns_name_t *name, vbprintf(1, "%s/%s:\n", namestr, typestr); + RWLOCK(&keylist_lock, isc_rwlocktype_read); arraysize = keycount; + RWUNLOCK(&keylist_lock, isc_rwlocktype_read); if (!nosigs) { arraysize += dns_rdataset_count(&sigset); } @@ -533,6 +559,13 @@ signset(dns_diff_t *del, dns_diff_t *add, dns_dbnode_t *node, dns_name_t *name, future = isc_serial_lt(now, rrsig.timesigned); key = keythatsigned(&rrsig); + /* + * Grow arrays if needed. + */ + if (key != NULL && key->index >= arraysize) { + grow_arrays(key->index + 1, &arraysize, + &wassignedby, &nowsignedby); + } offline = (key != NULL) ? key->pubkey : false; sig_format(&rrsig, sigstr, sizeof(sigstr)); expired = isc_serial_gt(now, rrsig.timeexpire); @@ -679,16 +712,29 @@ signset(dns_diff_t *del, dns_diff_t *add, dns_dbnode_t *node, dns_name_t *name, check_result(result, "dns_rdataset_first/next"); dns_rdataset_cleanup(&sigset); + RWLOCK(&keylist_lock, isc_rwlocktype_read); ISC_LIST_FOREACH(keylist, key, link) { + RWUNLOCK(&keylist_lock, isc_rwlocktype_read); if (REVOKE(key->key) && set->type != dns_rdatatype_dnskey) { + RWLOCK(&keylist_lock, isc_rwlocktype_read); continue; } + /* + * Grow arrays if needed. + */ + if (key->index >= arraysize) { + grow_arrays(key->index + 1, &arraysize, &wassignedby, + &nowsignedby); + } + if (nowsignedby[key->index]) { + RWLOCK(&keylist_lock, isc_rwlocktype_read); continue; } if (!issigningkey(key)) { + RWLOCK(&keylist_lock, isc_rwlocktype_read); continue; } @@ -699,19 +745,27 @@ signset(dns_diff_t *del, dns_diff_t *add, dns_dbnode_t *node, dns_name_t *name, { bool have_ksk = isksk(key); + RWLOCK(&keylist_lock, isc_rwlocktype_read); ISC_LIST_FOREACH(keylist, curr, link) { + RWUNLOCK(&keylist_lock, isc_rwlocktype_read); if (dst_key_alg(key->key) != dst_key_alg(curr->key)) { + RWLOCK(&keylist_lock, + isc_rwlocktype_read); continue; } if (REVOKE(curr->key)) { + RWLOCK(&keylist_lock, + isc_rwlocktype_read); continue; } if (isksk(curr)) { have_ksk = true; } + RWLOCK(&keylist_lock, isc_rwlocktype_read); } + RWUNLOCK(&keylist_lock, isc_rwlocktype_read); if (isksk(key) || !have_ksk || (iszsk(key) && !keyset_kskonly)) { @@ -737,14 +791,19 @@ signset(dns_diff_t *del, dns_diff_t *add, dns_dbnode_t *node, dns_name_t *name, * - Have key ID equal to the predecessor id. * - Have a successor that matches 'key' id. */ + RWLOCK(&keylist_lock, isc_rwlocktype_read); ISC_LIST_FOREACH(keylist, curr, link) { uint32_t suc; + RWUNLOCK(&keylist_lock, + isc_rwlocktype_read); if (dst_key_alg(key->key) != dst_key_alg(curr->key) || !iszsk(curr) || dst_key_id(curr->key) != pre) { + RWLOCK(&keylist_lock, + isc_rwlocktype_read); continue; } result = dst_key_getnum( @@ -753,8 +812,16 @@ signset(dns_diff_t *del, dns_diff_t *add, dns_dbnode_t *node, dns_name_t *name, if (result != ISC_R_SUCCESS || dst_key_id(key->key) != suc) { + RWLOCK(&keylist_lock, + isc_rwlocktype_read); continue; } + if (curr->index >= arraysize) { + grow_arrays(curr->index + 1, + &arraysize, + &wassignedby, + &nowsignedby); + } /* * curr is the predecessor we were @@ -764,7 +831,10 @@ signset(dns_diff_t *del, dns_diff_t *add, dns_dbnode_t *node, dns_name_t *name, if (nowsignedby[curr->index]) { have_pre_sig = true; } + RWLOCK(&keylist_lock, + isc_rwlocktype_read); } + RWUNLOCK(&keylist_lock, isc_rwlocktype_read); } /* @@ -776,7 +846,9 @@ signset(dns_diff_t *del, dns_diff_t *add, dns_dbnode_t *node, dns_name_t *name, "signing with dnskey"); } } + RWLOCK(&keylist_lock, isc_rwlocktype_read); } + RWUNLOCK(&keylist_lock, isc_rwlocktype_read); isc_mem_cput(isc_g_mctx, wassignedby, arraysize, sizeof(bool)); isc_mem_cput(isc_g_mctx, nowsignedby, arraysize, sizeof(bool));