From e53e202ef3b51c58fb9abe0d15e3829a970b00e7 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Thu, 28 May 2015 13:17:07 +1000 Subject: [PATCH] 4128. [bug] Address issues raised by Coverity 7.6. [RT #39537] --- CHANGES | 2 ++ bin/dnssec/dnssec-signzone.c | 5 ++++- bin/named/interfacemgr.c | 2 +- bin/tests/rbt/t_rbt.c | 3 ++- bin/tests/tasks/t_tasks.c | 6 ++++++ lib/dns/dlz.c | 28 ++++++++++++---------------- lib/dns/dst_api.c | 5 ----- lib/dns/journal.c | 2 ++ lib/dns/master.c | 2 +- lib/dns/openssldh_link.c | 8 ++++---- lib/dns/opensslrsa_link.c | 28 +++++++++++++++++----------- lib/dns/rbt.c | 6 ++---- lib/dns/rrl.c | 11 +++-------- lib/dns/view.c | 2 +- lib/dns/zone.c | 6 +++--- lib/isc/unix/app.c | 2 +- 16 files changed, 61 insertions(+), 57 deletions(-) diff --git a/CHANGES b/CHANGES index 6d84ca3328..fa215da4c4 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,5 @@ +4128. [bug] Address issues raised by Coverity 7.6. [RT #39537] + 4127. [protocol] CDS and CDNSKEY need to be signed by the key signing key as per RFC 7344, Section 4.1. [RT #37215] diff --git a/bin/dnssec/dnssec-signzone.c b/bin/dnssec/dnssec-signzone.c index f60719c5d3..8190fc081a 100644 --- a/bin/dnssec/dnssec-signzone.c +++ b/bin/dnssec/dnssec-signzone.c @@ -3592,7 +3592,10 @@ main(int argc, char *argv[]) { * of keys rather early. */ ISC_LIST_INIT(keylist); - isc_rwlock_init(&keylist_lock, 0, 0); + result = isc_rwlock_init(&keylist_lock, 0, 0); + if (result != ISC_R_SUCCESS) + fatal("could not initialize keylist_lock: %s", + isc_result_totext(result)); /* * Fill keylist with: diff --git a/bin/named/interfacemgr.c b/bin/named/interfacemgr.c index d116447d4b..ad80464212 100644 --- a/bin/named/interfacemgr.c +++ b/bin/named/interfacemgr.c @@ -471,7 +471,7 @@ ns_interface_listenudp(ns_interface_t *ifp) { return (ISC_R_SUCCESS); addtodispatch_failure: - for (i = disp - 1; i <= 0; i--) { + for (i = disp - 1; i >= 0; i--) { dns_dispatch_changeattributes(ifp->udpdispatch[i], 0, DNS_DISPATCHATTR_NOLISTEN); dns_dispatch_detach(&(ifp->udpdispatch[i])); diff --git a/bin/tests/rbt/t_rbt.c b/bin/tests/rbt/t_rbt.c index d99fd7301f..26004994f8 100644 --- a/bin/tests/rbt/t_rbt.c +++ b/bin/tests/rbt/t_rbt.c @@ -297,6 +297,7 @@ rbt_init(char *filename, dns_rbt_t **rbt, isc_mem_t *mctx) { if ((rval != 0) || (dns_result != ISC_R_SUCCESS)) { t_info("add of %s failed\n", p); dns_rbt_destroy(rbt); + (void) free(p); fclose(fp); return(1); } @@ -704,7 +705,7 @@ t9_walkchain(dns_rbtnodechain_t *chain, dns_rbt_t *rbt) { if (order >= 0) { t_info("unexpected order %s %s %s\n", dnsname_totext(dns_fixedname_name(&fullname1)), - order == -1 ? "<" : (order == 0 ? "==" : ">"), + order == 0 ? "==" : ">", dnsname_totext(dns_fixedname_name(&fullname2))); ++nprobs; } diff --git a/bin/tests/tasks/t_tasks.c b/bin/tests/tasks/t_tasks.c index 3da3a89174..e11acd36b3 100644 --- a/bin/tests/tasks/t_tasks.c +++ b/bin/tests/tasks/t_tasks.c @@ -999,6 +999,12 @@ t_tasks4(void) { NULL, sizeof(*event)); if (event == NULL) { t_info("isc_event_allocate failed\n"); + isc_result = isc_mutex_unlock(&T4_mx); + if (isc_result != ISC_R_SUCCESS) { + t_info("isc_mutex_unlock failed %s\n", + isc_result_totext(isc_result)); + ++T4_nprobs; + } DESTROYLOCK(&T4_mx); isc_task_destroy(&task); (void) isc_condition_destroy(&T4_cv); diff --git a/lib/dns/dlz.c b/lib/dns/dlz.c index 1a90d0929a..cf4dfff48e 100644 --- a/lib/dns/dlz.c +++ b/lib/dns/dlz.c @@ -240,8 +240,8 @@ dns_dlzcreate(isc_mem_t *mctx, const char *dlzname, const char *drivername, void dns_dlzdestroy(dns_dlzdb_t **dbp) { - isc_mem_t *mctx; dns_dlzdestroy_t destroy; + dns_dlzdb_t *db; /* Write debugging message to log */ isc_log_write(dns_lctx, DNS_LOGCATEGORY_DATABASE, @@ -253,23 +253,19 @@ dns_dlzdestroy(dns_dlzdb_t **dbp) { */ REQUIRE(dbp != NULL && DNS_DLZ_VALID(*dbp)); - if ((*dbp)->ssutable != NULL) { - dns_ssutable_detach(&(*dbp)->ssutable); - } + db = *dbp; + *dbp = NULL; + + if (db->ssutable != NULL) + dns_ssutable_detach(&db->ssutable); /* call the drivers destroy method */ - if ((*dbp) != NULL) { - mctx = (*dbp)->mctx; - if ((*dbp)->dlzname != NULL) - isc_mem_free(mctx, (*dbp)->dlzname); - destroy = (*dbp)->implementation->methods->destroy; - (*destroy)((*dbp)->implementation->driverarg,(*dbp)->dbdata); - /* return memory */ - isc_mem_put(mctx, (*dbp), sizeof(dns_dlzdb_t)); - isc_mem_detach(&mctx); - } - - *dbp = NULL; + if (db->dlzname != NULL) + isc_mem_free(db->mctx, db->dlzname); + destroy = db->implementation->methods->destroy; + (*destroy)(db->implementation->driverarg, db->dbdata); + /* return memory and detach */ + isc_mem_putanddetach(&db->mctx, db, sizeof(dns_dlzdb_t)); } /*% diff --git a/lib/dns/dst_api.c b/lib/dns/dst_api.c index 4fd70e2289..e429caed9e 100644 --- a/lib/dns/dst_api.c +++ b/lib/dns/dst_api.c @@ -1083,9 +1083,6 @@ comparekeys(const dst_key_t *key1, const dst_key_t *key2, if (key1 == key2) return (ISC_TRUE); - if (key1 == NULL || key2 == NULL) - return (ISC_FALSE); - if (key1->key_alg != key2->key_alg) return (ISC_FALSE); @@ -1175,8 +1172,6 @@ dst_key_paramcompare(const dst_key_t *key1, const dst_key_t *key2) { if (key1 == key2) return (ISC_TRUE); - if (key1 == NULL || key2 == NULL) - return (ISC_FALSE); if (key1->key_alg == key2->key_alg && key1->func->paramcompare != NULL && key1->func->paramcompare(key1, key2) == ISC_TRUE) diff --git a/lib/dns/journal.c b/lib/dns/journal.c index fc0abe1ecd..9ce21e4f26 100644 --- a/lib/dns/journal.c +++ b/lib/dns/journal.c @@ -2107,6 +2107,8 @@ dns_journal_compact(isc_mem_t *mctx, char *filename, isc_uint32_t serial, char backup[1024]; isc_boolean_t is_backup = ISC_FALSE; + REQUIRE(filename != NULL); + namelen = strlen(filename); if (namelen > 4U && strcmp(filename + namelen - 4, ".jnl") == 0) namelen -= 4; diff --git a/lib/dns/master.c b/lib/dns/master.c index f5dccc94fd..cb51991a5a 100644 --- a/lib/dns/master.c +++ b/lib/dns/master.c @@ -292,7 +292,7 @@ loadctx_destroy(dns_loadctx_t *lctx); SETRESULT(lctx, result); \ LOGIT(result); \ read_till_eol = ISC_TRUE; \ - continue; \ + break; \ } else if (result != ISC_R_SUCCESS) \ goto log_and_cleanup; \ } \ diff --git a/lib/dns/openssldh_link.c b/lib/dns/openssldh_link.c index 2056394e03..ec74d8a581 100644 --- a/lib/dns/openssldh_link.c +++ b/lib/dns/openssldh_link.c @@ -224,13 +224,13 @@ openssldh_generate(dst_key_t *key, int generator, void (*callback)(int)) { #else dh = DH_generate_parameters(key->key_size, generator, NULL, NULL); + if (dh == NULL) + return (dst__openssl_toresult2( + "DH_generate_parameters", + DST_R_OPENSSLFAILURE)); #endif } - if (dh == NULL) - return (dst__openssl_toresult2("DH_generate_parameters", - DST_R_OPENSSLFAILURE)); - if (DH_generate_key(dh) == 0) { DH_free(dh); return (dst__openssl_toresult2("DH_generate_key", diff --git a/lib/dns/opensslrsa_link.c b/lib/dns/opensslrsa_link.c index 7740103ca0..43d5df9e5e 100644 --- a/lib/dns/opensslrsa_link.c +++ b/lib/dns/opensslrsa_link.c @@ -1382,12 +1382,20 @@ opensslrsa_fromlabel(dst_key_t *key, const char *engine, const char *label, isc_result_t ret; EVP_PKEY *pkey = NULL; RSA *rsa = NULL, *pubrsa = NULL; - char *colon; + char *colon, *tmpengine = NULL; UNUSED(pin); - if (engine == NULL) - DST_RET(DST_R_NOENGINE); + if (engine == NULL) { + colon = strchr(label, ':'); + if (colon == NULL) + DST_RET(DST_R_NOENGINE); + tmpengine = isc_mem_strdup(key->mctx, label); + if (tmpengine == NULL) + DST_RET(ISC_R_NOMEMORY); + colon = strchr(tmpengine, ':'); + *colon = '\0'; + } e = dst__openssl_getengine(engine); if (e == NULL) DST_RET(DST_R_NOENGINE); @@ -1402,17 +1410,13 @@ opensslrsa_fromlabel(dst_key_t *key, const char *engine, const char *label, if (pkey == NULL) DST_RET(dst__openssl_toresult2("ENGINE_load_private_key", ISC_R_NOTFOUND)); - if (engine != NULL) { + if (tmpengine != NULL) { + key->engine = tmpengine; + tmpengine = NULL; + } else { key->engine = isc_mem_strdup(key->mctx, engine); if (key->engine == NULL) DST_RET(ISC_R_NOMEMORY); - } else { - key->engine = isc_mem_strdup(key->mctx, label); - if (key->engine == NULL) - DST_RET(ISC_R_NOMEMORY); - colon = strchr(key->engine, ':'); - if (colon != NULL) - *colon = '\0'; } key->label = isc_mem_strdup(key->mctx, label); if (key->label == NULL) @@ -1437,6 +1441,8 @@ opensslrsa_fromlabel(dst_key_t *key, const char *engine, const char *label, return (ISC_R_SUCCESS); err: + if (tmpengine != NULL) + isc_mem_free(key->mctx, tmpengine); if (rsa != NULL) RSA_free(rsa); if (pubrsa != NULL) diff --git a/lib/dns/rbt.c b/lib/dns/rbt.c index b8ca65445c..10b8c498b4 100644 --- a/lib/dns/rbt.c +++ b/lib/dns/rbt.c @@ -2353,8 +2353,7 @@ rotate_left(dns_rbtnode_t *node, dns_rbtnode_t **rootp) { PARENT(LEFT(child)) = node; LEFT(child) = node; - if (child != NULL) - PARENT(child) = PARENT(node); + PARENT(child) = PARENT(node); if (IS_ROOT(node)) { *rootp = child; @@ -2386,8 +2385,7 @@ rotate_right(dns_rbtnode_t *node, dns_rbtnode_t **rootp) { PARENT(RIGHT(child)) = node; RIGHT(child) = node; - if (child != NULL) - PARENT(child) = PARENT(node); + PARENT(child) = PARENT(node); if (IS_ROOT(node)) { *rootp = child; diff --git a/lib/dns/rrl.c b/lib/dns/rrl.c index 766d007514..75b45e49d8 100644 --- a/lib/dns/rrl.c +++ b/lib/dns/rrl.c @@ -1161,22 +1161,17 @@ dns_rrl(dns_view_t *view, client_addr, now, log_buf, log_buf_len); if (rrl_all_result != DNS_RRL_RESULT_OK) { - int level; - e = e_all; rrl_result = rrl_all_result; - if (rrl_result == DNS_RRL_RESULT_OK) - level = DNS_RRL_LOG_DEBUG2; - else - level = DNS_RRL_LOG_DEBUG1; - if (isc_log_wouldlog(dns_lctx, level)) { + if (isc_log_wouldlog(dns_lctx, DNS_RRL_LOG_DEBUG1)) { make_log_buf(rrl, e, "prefer all-per-second limiting ", NULL, ISC_TRUE, qname, ISC_FALSE, DNS_RRL_RESULT_OK, resp_result, log_buf, log_buf_len); isc_log_write(dns_lctx, DNS_LOGCATEGORY_RRL, - DNS_LOGMODULE_REQUEST, level, + DNS_LOGMODULE_REQUEST, + DNS_RRL_LOG_DEBUG1, "%s", log_buf); } } diff --git a/lib/dns/view.c b/lib/dns/view.c index fb6368615a..6cbbd36856 100644 --- a/lib/dns/view.c +++ b/lib/dns/view.c @@ -1332,7 +1332,7 @@ dns_view_findzonecut2(dns_view_t *view, dns_name_t *name, dns_name_t *fname, if (result == ISC_R_SUCCESS) { if (zfname != NULL && (!dns_name_issubdomain(fname, zfname) || - (dns_zone_staticstub && + (dns_zone_gettype(zone) == dns_zone_staticstub && dns_name_equal(fname, zfname)))) { /* * We found a zonecut in the cache, but our diff --git a/lib/dns/zone.c b/lib/dns/zone.c index 1d6784064d..6c2b05c3b9 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -12782,10 +12782,10 @@ dns_zone_notifyreceive(dns_zone_t *zone, isc_sockaddr_t *from, dns_zone_log(zone, ISC_LOG_INFO, "notify from %s: no serial", fromtext); zone->notifyfrom = *from; - local = zone->masteraddr; - remote = zone->sourceaddr; + remote = zone->masteraddr; + local = zone->sourceaddr; UNLOCK_ZONE(zone); - dns_zonemgr_unreachabledel(zone->zmgr, &local, &remote); + dns_zonemgr_unreachabledel(zone->zmgr, &remote, &local); dns_zone_refresh(zone); return (ISC_R_SUCCESS); } diff --git a/lib/isc/unix/app.c b/lib/isc/unix/app.c index 48da3810b3..edd0745db5 100644 --- a/lib/isc/unix/app.c +++ b/lib/isc/unix/app.c @@ -723,7 +723,7 @@ isc__app_ctxrun(isc_appctx_t *ctx0) { return (ISC_R_UNEXPECTED); } #endif - result = sigsuspend(&sset); + (void)sigsuspend(&sset); } else { /* * External, or BIND9 using multiple contexts: