From 4edd26c96e08b9094ae18d1342cf9eb066c20e24 Mon Sep 17 00:00:00 2001 From: Stanislav Levin Date: May 25 2020 06:45:49 +0000 Subject: bind-9.16: isc_mem_get cannot fail gracefully now Since [0] malloc is handled directly in default_memalloc and it aborts on NULL pointer result. Further check is no longer needed. Thus, isc_mem_get based macro can be safely removed [1]. [0]: https://gitlab.isc.org/isc-projects/bind9/-/commit/8de2451756 [1]: https://gitlab.isc.org/isc-projects/bind9/-/commit/f63e696967 https://gitlab.isc.org/isc-projects/bind9/-/commit/ae83801e2b --- diff --git a/src/acl.c b/src/acl.c index 0fead9a..e484403 100644 --- a/src/acl.c +++ b/src/acl.c @@ -200,9 +200,9 @@ get_types(isc_mem_t *mctx, const cfg_obj_t *obj, dns_rdatatype_t **typesp, obj = cfg_tuple_get(obj, "types"); n = count_list_elements(obj); - if (n > 0) - CHECKED_MEM_GET(mctx, types, n * sizeof(dns_rdatatype_t)); - + if (n > 0) { + types = isc_mem_get(mctx, n * sizeof(dns_rdatatype_t)); + } i = 0; for (el = cfg_list_first(obj); el != NULL; el = cfg_list_next(el)) { const cfg_obj_t *typeobj; diff --git a/src/fwd.c b/src/fwd.c index f2aa426..5f03e26 100644 --- a/src/fwd.c +++ b/src/fwd.c @@ -49,7 +49,6 @@ buffer_append_str(void *closure, const char *text, int textlen) { isc_buffer_region(out_buf, &old_space); new_space.length = isc_buffer_length(out_buf) + textlen + 1; new_space.base = isc_mem_get(out_buf->mctx, new_space.length); - RUNTIME_CHECK(new_space.base != NULL); isc_buffer_reinit(out_buf, new_space.base, new_space.length); if (old_space.base != NULL) isc_mem_put(out_buf->mctx, old_space.base, old_space.length); @@ -272,7 +271,7 @@ fwd_parse_str(const char *fwdrs_str, isc_mem_t *mctx, addr = *cfg_obj_assockaddr(fwdr_cfg); if (isc_sockaddr_getport(&addr) == 0) isc_sockaddr_setport(&addr, port); - CHECKED_MEM_GET_PTR(mctx, fwdr); + fwdr = isc_mem_get(mctx, sizeof(*(fwdr))); fwdr->addr = addr; fwdr->dscp = cfg_obj_getdscp(fwdr_cfg); ISC_LINK_INIT(fwdr, link); diff --git a/src/fwd_register.c b/src/fwd_register.c index 06a4d92..cc9764b 100644 --- a/src/fwd_register.c +++ b/src/fwd_register.c @@ -31,7 +31,7 @@ fwdr_create(isc_mem_t *mctx, fwd_register_t **fwdrp) REQUIRE(fwdrp != NULL && *fwdrp == NULL); - CHECKED_MEM_GET_PTR(mctx, fwdr); + fwdr = isc_mem_get(mctx, sizeof(*(fwdr))); ZERO_PTR(fwdr); isc_mem_attach(mctx, &fwdr->mctx); CHECK(dns_rbt_create(mctx, NULL, NULL, &fwdr->rbt)); @@ -41,11 +41,10 @@ fwdr_create(isc_mem_t *mctx, fwd_register_t **fwdrp) return ISC_R_SUCCESS; cleanup: - if (fwdr != NULL) { - if (fwdr->rbt != NULL) - dns_rbt_destroy(&fwdr->rbt); - MEM_PUT_AND_DETACH(fwdr); + if (fwdr->rbt != NULL) { + dns_rbt_destroy(&fwdr->rbt); } + MEM_PUT_AND_DETACH(fwdr); return result; } diff --git a/src/ldap_driver.c b/src/ldap_driver.c index 8c4ff62..471b047 100644 --- a/src/ldap_driver.c +++ b/src/ldap_driver.c @@ -990,7 +990,7 @@ ldapdb_create(isc_mem_t *mctx, dns_name_t *name, dns_dbtype_t type, REQUIRE(driverarg != NULL); REQUIRE(dbp != NULL && *dbp == NULL); - CHECKED_MEM_GET_PTR(mctx, ldapdb); + ldapdb = isc_mem_get(mctx, sizeof(*(ldapdb))); ZERO_PTR(ldapdb); isc_mem_attach(mctx, &ldapdb->common.mctx); diff --git a/src/ldap_entry.c b/src/ldap_entry.c index d0f83d4..cdf26d8 100644 --- a/src/ldap_entry.c +++ b/src/ldap_entry.c @@ -68,7 +68,6 @@ static isc_result_t ATTR_NONNULLS ATTR_CHECKRESULT ldap_attr_create(isc_mem_t *mctx, LDAP *ld, LDAPMessage *ldap_entry, ldap_attribute_t *attr) { - isc_result_t result; char **values; ldap_value_t *val; @@ -84,7 +83,7 @@ ldap_attr_create(isc_mem_t *mctx, LDAP *ld, LDAPMessage *ldap_entry, attr->ldap_values = values; for (unsigned int i = 0; values[i] != NULL; i++) { - CHECKED_MEM_GET_PTR(mctx, val); + val = isc_mem_get(mctx, sizeof(*(val))); val->value = values[i]; INIT_LINK(val, link); @@ -92,12 +91,6 @@ ldap_attr_create(isc_mem_t *mctx, LDAP *ld, LDAPMessage *ldap_entry, } return ISC_R_SUCCESS; - -cleanup: - ldap_valuelist_destroy(mctx, &attr->values); - ldap_value_free(values); - - return result; } /** @@ -112,7 +105,7 @@ ldap_entry_init(isc_mem_t *mctx, ldap_entry_t **entryp) { REQUIRE(entryp != NULL); REQUIRE(*entryp == NULL); - CHECKED_MEM_GET_PTR(mctx, entry); + entry = isc_mem_get(mctx, sizeof(*(entry))); ZERO_PTR(entry); isc_mem_attach(mctx, &entry->mctx); INIT_LIST(entry->attrs); @@ -120,7 +113,7 @@ ldap_entry_init(isc_mem_t *mctx, ldap_entry_t **entryp) { INIT_BUFFERED_NAME(entry->fqdn); INIT_BUFFERED_NAME(entry->zone_name); - CHECKED_MEM_GET(mctx, entry->rdata_target_mem, DNS_RDATA_MAXLENGTH); + entry->rdata_target_mem = isc_mem_get(mctx, DNS_RDATA_MAXLENGTH); CHECK(isc_lex_create(mctx, TOKENSIZ, &entry->lex)); *entryp = entry; @@ -203,7 +196,7 @@ ldap_entry_parse(isc_mem_t *mctx, LDAP *ld, LDAPMessage *ldap_entry, for (attribute = ldap_first_attribute(ld, ldap_entry, &ber); attribute != NULL; attribute = ldap_next_attribute(ld, ldap_entry, ber)) { - CHECKED_MEM_GET_PTR(mctx, attr); + attr = isc_mem_get(mctx, sizeof(*(attr))); ZERO_PTR(attr); attr->name = attribute; diff --git a/src/ldap_helper.c b/src/ldap_helper.c index 8b15a84..113d86f 100644 --- a/src/ldap_helper.c +++ b/src/ldap_helper.c @@ -551,7 +551,7 @@ new_ldap_instance(isc_mem_t *mctx, const char *db_name, const char *parameters, REQUIRE(ldap_instp != NULL && *ldap_instp == NULL); - CHECKED_MEM_GET_PTR(mctx, ldap_inst); + ldap_inst = isc_mem_get(mctx, sizeof(*(ldap_inst))); ZERO_PTR(ldap_inst); isc_refcount_init(&ldap_inst->errors, 0); isc_mem_attach(mctx, &ldap_inst->mctx); @@ -790,13 +790,12 @@ destroy_ldap_instance(ldap_instance_t **ldap_instp) static isc_result_t ATTR_NONNULLS ATTR_CHECKRESULT new_ldap_connection(ldap_pool_t *pool, ldap_connection_t **ldap_connp) { - isc_result_t result; ldap_connection_t *ldap_conn; REQUIRE(pool != NULL); REQUIRE(ldap_connp != NULL && *ldap_connp == NULL); - CHECKED_MEM_GET_PTR(pool->mctx, ldap_conn); + ldap_conn = isc_mem_get(pool->mctx, sizeof(*(ldap_conn))); ZERO_PTR(ldap_conn); /* isc_mutex_init and isc_condition_init failures are now fatal */ @@ -813,11 +812,6 @@ new_ldap_connection(ldap_pool_t *pool, ldap_connection_t **ldap_connp) *ldap_connp = ldap_conn; return ISC_R_SUCCESS; - -cleanup: - destroy_ldap_connection(&ldap_conn); - - return result; } static void @@ -2283,7 +2277,7 @@ findrdatatype_or_create(isc_mem_t *mctx, ldapdb_rdatalist_t *rdatalist, result = ldapdb_rdatalist_findrdatatype(rdatalist, rdtype, &rdlist); if (result != ISC_R_SUCCESS) { - CHECKED_MEM_GET_PTR(mctx, rdlist); + rdlist = isc_mem_get(mctx, sizeof(*(rdlist))); dns_rdatalist_init(rdlist); rdlist->rdclass = rdclass; @@ -2692,11 +2686,11 @@ parse_rdata(isc_mem_t *mctx, ldap_entry_t *entry, CHECK(dns_rdata_fromtext(NULL, rdclass, rdtype, entry->lex, origin, 0, mctx, &entry->rdata_target, NULL)); - CHECKED_MEM_GET_PTR(mctx, rdata); + rdata = isc_mem_get(mctx, sizeof(*(rdata))); dns_rdata_init(rdata); rdatamem.length = isc_buffer_usedlength(&entry->rdata_target); - CHECKED_MEM_GET(mctx, rdatamem.base, rdatamem.length); + rdatamem.base = isc_mem_get(mctx, rdatamem.length); memcpy(rdatamem.base, isc_buffer_base(&entry->rdata_target), rdatamem.length); @@ -3177,22 +3171,15 @@ isc_result_t ATTR_NONNULLS ATTR_CHECKRESULT ldap_mod_create(isc_mem_t *mctx, LDAPMod **changep) { LDAPMod *change = NULL; - isc_result_t result; REQUIRE(changep != NULL && *changep == NULL); - CHECKED_MEM_GET_PTR(mctx, change); + change = isc_mem_get(mctx, sizeof(*(change))); ZERO_PTR(change); - CHECKED_MEM_GET(mctx, change->mod_type, LDAP_ATTR_FORMATSIZE); + change->mod_type = isc_mem_get(mctx, LDAP_ATTR_FORMATSIZE); *changep = change; return ISC_R_SUCCESS; - -cleanup: - if (change != NULL) - SAFE_MEM_PUT_PTR(mctx, change); - - return result; } /** @@ -3604,13 +3591,13 @@ ldap_pool_create(isc_mem_t *mctx, unsigned int connections, ldap_pool_t **poolp) REQUIRE(poolp != NULL && *poolp == NULL); - CHECKED_MEM_GET(mctx, pool, sizeof(*pool)); + pool = isc_mem_get(mctx, sizeof(*pool)); ZERO_PTR(pool); isc_mem_attach(mctx, &pool->mctx); CHECK(semaphore_init(&pool->conn_semaphore, connections)); - CHECKED_MEM_GET(mctx, pool->conns, - connections * sizeof(ldap_connection_t *)); + pool->conns = isc_mem_get(mctx, + connections * sizeof(ldap_connection_t *)); memset(pool->conns, 0, connections * sizeof(ldap_connection_t *)); pool->connections = connections; diff --git a/src/metadb.c b/src/metadb.c index 1f3c0ef..f469a30 100644 --- a/src/metadb.c +++ b/src/metadb.c @@ -40,7 +40,7 @@ metadb_new(isc_mem_t *mctx, metadb_t **mdbp) { REQUIRE(mdbp != NULL && *mdbp == NULL); - CHECKED_MEM_GET_PTR(mctx, mdb); + mdb = isc_mem_get(mctx, sizeof(*(mdb))); ZERO_PTR(mdb); isc_mem_attach(mctx, &mdb->mctx); @@ -55,13 +55,11 @@ metadb_new(isc_mem_t *mctx, metadb_t **mdbp) { return result; cleanup: - if (mdb != NULL) { - if (lock_ready == true) { - /* isc_mutex_destroy errors are now fatal */ - isc_mutex_destroy(&mdb->newversion_lock); - } - MEM_PUT_AND_DETACH(mdb); + if (lock_ready == true) { + /* isc_mutex_destroy errors are now fatal */ + isc_mutex_destroy(&mdb->newversion_lock); } + MEM_PUT_AND_DETACH(mdb); return result; } @@ -163,7 +161,7 @@ metadb_iterator_create(metadb_t *mdb, metadb_iter_t **miterp) { REQUIRE(mdb != NULL); REQUIRE(miterp != NULL && *miterp == NULL); - CHECKED_MEM_GET_PTR(mdb->mctx, miter); + miter = isc_mem_get(mdb->mctx, sizeof(*(miter))); ZERO_PTR(miter); isc_mem_attach(mdb->mctx, &miter->mctx); @@ -261,7 +259,7 @@ metadb_node_init(metadb_t *mdb, dns_dbversion_t *ver, dns_name_t *mname, REQUIRE(nodep != NULL && *nodep == NULL); - CHECKED_MEM_GET_PTR(mdb->mctx, node); + node = isc_mem_get(mdb->mctx, sizeof(*(node))); ZERO_PTR(node); isc_mem_attach(mdb->mctx, &node->mctx); diff --git a/src/mldap.c b/src/mldap.c index fbab108..ced1777 100644 --- a/src/mldap.c +++ b/src/mldap.c @@ -59,7 +59,7 @@ mldap_new(isc_mem_t *mctx, mldapdb_t **mldapp) { REQUIRE(mldapp != NULL && *mldapp == NULL); - CHECKED_MEM_GET_PTR(mctx, mldap); + mldap = isc_mem_get(mctx, sizeof(*(mldap))); ZERO_PTR(mldap); isc_mem_attach(mctx, &mldap->mctx); @@ -425,7 +425,7 @@ mldap_iter_deadnodes_start(mldapdb_t *mldap, metadb_iter_t **iterp, REQUIRE(iterp != NULL && *iterp == NULL); CHECK(metadb_iterator_create(mldap->mdb, &iter)); - CHECKED_MEM_GET(mldap->mctx, iter->state, sizeof(uint32_t)); + iter->state = isc_mem_get(mldap->mctx, sizeof(uint32_t)); result = dns_dbiterator_seek(iter->iter, &uuid_rootname); if (result == ISC_R_NOTFOUND) /* metaLDAP is empty */ CLEANUP_WITH(ISC_R_NOMORE); diff --git a/src/rbt_helper.c b/src/rbt_helper.c index 2333d96..2d30d3c 100644 --- a/src/rbt_helper.c +++ b/src/rbt_helper.c @@ -87,7 +87,7 @@ rbt_iter_first(isc_mem_t *mctx, dns_rbt_t *rbt, isc_rwlock_t *rwlock, REQUIRE(rwlock != NULL); REQUIRE(iterp != NULL && *iterp == NULL); - CHECKED_MEM_GET_PTR(mctx, iter); + iter = isc_mem_get(mctx, sizeof(*(iter))); ZERO_PTR(iter); isc_mem_attach(mctx, &iter->mctx); diff --git a/src/settings.c b/src/settings.c index 6f1f30c..9c86dce 100644 --- a/src/settings.c +++ b/src/settings.c @@ -495,7 +495,7 @@ settings_set_create(isc_mem_t *mctx, const setting_t default_settings[], ZERO_PTR(new_set); isc_mem_attach(mctx, &new_set->mctx); - CHECKED_MEM_GET_PTR(mctx, new_set->lock); + new_set->lock = isc_mem_get(mctx, sizeof(*(new_set->lock))); /* isc_mutex_init failures are now fatal */ isc_mutex_init(new_set->lock); diff --git a/src/syncrepl.c b/src/syncrepl.c index 2352319..e0b6dea 100644 --- a/src/syncrepl.c +++ b/src/syncrepl.c @@ -269,7 +269,7 @@ sync_ctx_init(isc_mem_t *mctx, ldap_instance_t *inst, sync_ctx_t **sctxp) { REQUIRE(sctxp != NULL && *sctxp == NULL); - CHECKED_MEM_GET_PTR(mctx, sctx); + sctx = isc_mem_get(mctx, sizeof(*(sctx))); ZERO_PTR(sctx); isc_mem_attach(mctx, &sctx->mctx); @@ -447,13 +447,12 @@ sync_state_reset(sync_ctx_t *sctx) { */ isc_result_t sync_task_add(sync_ctx_t *sctx, isc_task_t *task) { - isc_result_t result = ISC_R_SUCCESS; task_element_t *newel = NULL; REQUIRE(sctx != NULL); REQUIRE(ISCAPI_TASK_VALID(task)); - CHECKED_MEM_GET_PTR(sctx->mctx, newel); + newel = isc_mem_get(sctx->mctx, sizeof(*(newel))); ZERO_PTR(newel); ISC_LINK_INIT(newel, link); newel->task = NULL; @@ -468,8 +467,7 @@ sync_task_add(sync_ctx_t *sctx, isc_task_t *task) { log_debug(2, "adding task %p to syncrepl list; %lu tasks in list", task, isc_refcount_current(&sctx->task_cnt)); -cleanup: - return result; + return ISC_R_SUCCESS; } /** diff --git a/src/util.h b/src/util.h index 63ec058..18b6eb9 100644 --- a/src/util.h +++ b/src/util.h @@ -45,19 +45,6 @@ extern bool verbose_checks; /* from settings.c */ } \ } while (0) -#define CHECKED_MEM_GET(m, target_ptr, s) \ - do { \ - (target_ptr) = isc_mem_get((m), (s)); \ - if ((target_ptr) == NULL) { \ - result = ISC_R_NOMEMORY; \ - log_error_position("Memory allocation failed"); \ - goto cleanup; \ - } \ - } while (0) - -#define CHECKED_MEM_GET_PTR(m, target_ptr) \ - CHECKED_MEM_GET(m, target_ptr, sizeof(*(target_ptr))) - #define CHECKED_MEM_STRDUP(m, source, target) \ do { \ (target) = isc_mem_strdup((m), (source)); \ diff --git a/src/zone_register.c b/src/zone_register.c index 8fc158e..c9f5b5d 100644 --- a/src/zone_register.c +++ b/src/zone_register.c @@ -111,7 +111,7 @@ zr_create(isc_mem_t *mctx, ldap_instance_t *ldap_inst, REQUIRE(glob_settings != NULL); REQUIRE(zrp != NULL && *zrp == NULL); - CHECKED_MEM_GET_PTR(mctx, zr); + zr = isc_mem_get(mctx, sizeof(*(zr))); ZERO_PTR(zr); isc_mem_attach(mctx, &zr->mctx); CHECK(dns_rbt_create(mctx, delete_zone_info, mctx, &zr->rbt)); @@ -123,11 +123,10 @@ zr_create(isc_mem_t *mctx, ldap_instance_t *ldap_inst, return ISC_R_SUCCESS; cleanup: - if (zr != NULL) { - if (zr->rbt != NULL) - dns_rbt_destroy(&zr->rbt); - MEM_PUT_AND_DETACH(zr); + if (zr->rbt != NULL) { + dns_rbt_destroy(&zr->rbt); } + MEM_PUT_AND_DETACH(zr); return result; } @@ -272,7 +271,7 @@ create_zone_info(isc_mem_t * const mctx, dns_zone_t * const raw, REQUIRE(dn != NULL); REQUIRE(zinfop != NULL && *zinfop == NULL); - CHECKED_MEM_GET_PTR(mctx, zinfo); + zinfo = isc_mem_get(mctx, sizeof(*(zinfo))); ZERO_PTR(zinfo); CHECKED_MEM_STRDUP(mctx, dn, zinfo->dn); dns_zone_attach(raw, &zinfo->raw);