From 8b154d0f9fd7b902f79c7ed3d4c8f047ec7c1bae Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Mon, 18 May 2020 18:09:08 -0700 Subject: [PATCH] pass the nodename to add32() instead of calling dns_rbt_fullnamefromnode() in addition to being more efficient, this prevents a possible crash by looking up the node name before the tree sructure can be changed when cleaning up dead nodes in addrdataset(). (cherry picked from commit db9d10e3c1761d39600defb031934068d74ccd1b) --- lib/dns/rbtdb.c | 41 +++++++++++++++++++++-------------------- 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/lib/dns/rbtdb.c b/lib/dns/rbtdb.c index fa56ce5290..a64f47852a 100644 --- a/lib/dns/rbtdb.c +++ b/lib/dns/rbtdb.c @@ -5959,14 +5959,14 @@ update_recordsandbytes(bool add, rbtdb_version_t *rbtversion, } static isc_result_t -add32(dns_rbtdb_t *rbtdb, dns_rbtnode_t *rbtnode, rbtdb_version_t *rbtversion, - rdatasetheader_t *newheader, unsigned int options, bool loading, - dns_rdataset_t *addedrdataset, isc_stdtime_t now) { +add32(dns_rbtdb_t *rbtdb, dns_rbtnode_t *rbtnode, const dns_name_t *nodename, + rbtdb_version_t *rbtversion, rdatasetheader_t *newheader, + unsigned int options, bool loading, dns_rdataset_t *addedrdataset, + isc_stdtime_t now) { rbtdb_changed_t *changed = NULL; - rdatasetheader_t *topheader, *topheader_prev, *header, *sigheader; - dns_fixedname_t fname; - dns_name_t *nodename = dns_fixedname_initname(&fname); - unsigned char *merged; + rdatasetheader_t *topheader = NULL, *topheader_prev = NULL; + rdatasetheader_t *header = NULL, *sigheader = NULL; + unsigned char *merged = NULL; isc_result_t result; bool header_nx; bool newheader_nx; @@ -5984,8 +5984,6 @@ add32(dns_rbtdb_t *rbtdb, dns_rbtnode_t *rbtnode, rbtdb_version_t *rbtversion, * Caller must be holding the node lock. */ - dns_rbt_fullnamefromnode(rbtnode, nodename); - if ((options & DNS_DBADD_MERGE) != 0) { REQUIRE(rbtversion != NULL); merge = true; @@ -6669,7 +6667,7 @@ addrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version, name = dns_fixedname_initname(&fixed); RWLOCK(&rbtdb->tree_lock, isc_rwlocktype_read); - dns_rbt_fullnamefromnode(node, name); + dns_rbt_fullnamefromnode(rbtnode, name); RWUNLOCK(&rbtdb->tree_lock, isc_rwlocktype_read); dns_rdataset_getownercase(rdataset, name); @@ -6762,10 +6760,11 @@ addrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version, RWUNLOCK(&rbtdb->tree_lock, isc_rwlocktype_read); /* - * If we're adding a delegation type, adding to the auxiliary NSEC tree, - * or the DB is a cache in an overmem state, hold an exclusive lock on - * the tree. In the latter case the lock does not necessarily have to - * be acquired but it will help purge ancient entries more effectively. + * If we're adding a delegation type, adding to the auxiliary NSEC + * tree, or the DB is a cache in an overmem state, hold an + * exclusive lock on the tree. In the latter case the lock does + * not necessarily have to be acquired but it will help purge + * ancient entries more effectively. */ if (IS_CACHE(rbtdb) && isc_mem_isovermem(rbtdb->common.mctx)) { cache_is_overmem = true; @@ -6812,7 +6811,6 @@ addrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version, if (newnsec) { dns_rbtnode_t *nsecnode; - dns_rbt_fullnamefromnode(rbtnode, name); nsecnode = NULL; result = dns_rbt_addnode(rbtdb->nsec, name, &nsecnode); if (result == ISC_R_SUCCESS) { @@ -6825,8 +6823,8 @@ addrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version, } if (result == ISC_R_SUCCESS) { - result = add32(rbtdb, rbtnode, rbtversion, newheader, options, - false, addedrdataset, now); + result = add32(rbtdb, rbtnode, name, rbtversion, newheader, + options, false, addedrdataset, now); } if (result == ISC_R_SUCCESS && delegating) { rbtnode->find_callback = 1; @@ -7081,6 +7079,8 @@ deleterdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version, dns_rbtdb_t *rbtdb = (dns_rbtdb_t *)db; dns_rbtnode_t *rbtnode = (dns_rbtnode_t *)node; rbtdb_version_t *rbtversion = version; + dns_fixedname_t fname; + dns_name_t *nodename = dns_fixedname_initname(&fname); isc_result_t result; rdatasetheader_t *newheader; @@ -7117,8 +7117,9 @@ deleterdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version, NODE_LOCK(&rbtdb->node_locks[rbtnode->locknum].lock, isc_rwlocktype_write); - result = add32(rbtdb, rbtnode, rbtversion, newheader, DNS_DBADD_FORCE, - false, NULL, 0); + dns_rbt_fullnamefromnode(rbtnode, nodename); + result = add32(rbtdb, rbtnode, nodename, rbtversion, newheader, + DNS_DBADD_FORCE, false, NULL, 0); NODE_UNLOCK(&rbtdb->node_locks[rbtnode->locknum].lock, isc_rwlocktype_write); @@ -7325,7 +7326,7 @@ loading_addrdataset(void *arg, const dns_name_t *name, NODE_LOCK(&rbtdb->node_locks[node->locknum].lock, isc_rwlocktype_write); - result = add32(rbtdb, node, rbtdb->current_version, newheader, + result = add32(rbtdb, node, name, rbtdb->current_version, newheader, DNS_DBADD_MERGE, true, NULL, 0); NODE_UNLOCK(&rbtdb->node_locks[node->locknum].lock,