From 538f4a22b1cbde59a9e53fef51583904a7f7393f Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Fri, 30 Nov 2012 11:39:37 +1100 Subject: [PATCH 1/7] 3429. [bug] dns_zone_getserial2 could a return success without returning a valid serial. [RT #32007] Squashed commit of the following: commit 0057f4b6e843c3998b987dbc7f32ceeee8afc150 Author: Mark Andrews Date: Fri Nov 30 08:13:15 2012 +1100 zone_get_from_db could return success without setting return valuses; serial is only valid if soacount is none zero --- CHANGES | 3 ++ lib/dns/zone.c | 74 ++++++++++++++++++++++++++++++++++++-------------- 2 files changed, 56 insertions(+), 21 deletions(-) diff --git a/CHANGES b/CHANGES index 233e26c23f..ea87ecbe07 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,6 @@ +3429. [bug] dns_zone_getserial2 could a return success without + returning a valid serial. [RT #32007] + 3428. [cleanup] dig: Add timezone to date output. [RT #2269] 3427. [bug] dig +trace incorrectly displayed name server diff --git a/lib/dns/zone.c b/lib/dns/zone.c index c59501346c..58fa171a14 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -1155,6 +1155,7 @@ dns_zone_setnotifytype(dns_zone_t *zone, dns_notifytype_t notifytype) { isc_result_t dns_zone_getserial2(dns_zone_t *zone, isc_uint32_t *serialp) { isc_result_t result; + unsigned int soacount; REQUIRE(DNS_ZONE_VALID(zone)); REQUIRE(serialp != NULL); @@ -1162,8 +1163,11 @@ dns_zone_getserial2(dns_zone_t *zone, isc_uint32_t *serialp) { LOCK_ZONE(zone); ZONEDB_LOCK(&zone->dblock, isc_rwlocktype_read); if (zone->db != NULL) { - result = zone_get_from_db(zone, zone->db, NULL, NULL, serialp, - NULL, NULL, NULL, NULL, NULL); + result = zone_get_from_db(zone, zone->db, NULL, &soacount, + serialp, NULL, NULL, NULL, NULL, + NULL); + if (result == ISC_R_SUCCESS && soacount == 0) + result = ISC_R_FAILURE; } else result = DNS_R_NOTLOADED; ZONEDB_UNLOCK(&zone->dblock, isc_rwlocktype_read); @@ -1915,14 +1919,15 @@ zone_gotreadhandle(isc_task_t *task, isc_event_t *event) { static void get_raw_serial(dns_zone_t *raw, dns_masterrawheader_t *rawdata) { isc_result_t result; + unsigned int soacount; LOCK(&raw->lock); if (raw->db != NULL) { - result = zone_get_from_db(raw, raw->db, NULL, NULL, + result = zone_get_from_db(raw, raw->db, NULL, &soacount, &rawdata->sourceserial, NULL, NULL, NULL, NULL, NULL); - if (result == ISC_R_SUCCESS) + if (result == ISC_R_SUCCESS && soacount > 0U) rawdata->flags |= DNS_MASTERRAW_SOURCESERIALSET; } UNLOCK(&raw->lock); @@ -3669,10 +3674,12 @@ maybe_send_secure(dns_zone_t *zone) { if (zone->raw->db != NULL) { if (zone->db != NULL) { isc_uint32_t serial; + unsigned int soacount; + result = zone_get_from_db(zone->raw, zone->raw->db, - NULL, NULL, &serial, NULL, + NULL, &soacount, &serial, NULL, NULL, NULL, NULL, NULL); - if (result == ISC_R_SUCCESS) + if (result == ISC_R_SUCCESS && soacount > 0U) zone_send_secureserial(zone->raw, ISC_TRUE, serial); } else zone_send_securedb(zone->raw, ISC_TRUE, zone->raw->db); @@ -3914,14 +3921,18 @@ zone_postload(dns_zone_t *zone, dns_db_t *db, isc_time_t loadtime, } if (zone->db != NULL) { + unsigned int oldsoacount; + /* * This is checked in zone_replacedb() for slave zones * as they don't reload from disk. */ - result = zone_get_from_db(zone, zone->db, NULL, NULL, - &oldserial, NULL, NULL, NULL, - NULL, NULL); + result = zone_get_from_db(zone, zone->db, NULL, + &oldsoacount, &oldserial, + NULL, NULL, NULL, NULL, + NULL); RUNTIME_CHECK(result == ISC_R_SUCCESS); + RUNTIME_CHECK(soacount > 0U); if (DNS_ZONE_OPTION(zone, DNS_ZONEOPT_IXFRFROMDIFFS) && !isc_serial_gt(serial, oldserial)) { isc_uint32_t serialmin, serialmax; @@ -4364,6 +4375,19 @@ zone_load_soa_rr(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version, *expire = soa.expire; if (minimum != NULL) *minimum = soa.minimum; + } else { + if (soacount != NULL) + *soacount = 0; + if (serial != NULL) + *serial = 0; + if (refresh != NULL) + *refresh = 0; + if (retry != NULL) + *retry = 0; + if (expire != NULL) + *expire = 0; + if (minimum != NULL) + *minimum = 0; } result = ISC_R_SUCCESS; @@ -8727,16 +8751,18 @@ dns_zone_markdirty(dns_zone_t *zone) { LOCK_ZONE(zone); if (zone->type == dns_zone_master) { if (inline_raw(zone)) { + unsigned int soacount; + ZONEDB_LOCK(&zone->dblock, isc_rwlocktype_read); if (zone->db != NULL) { result = zone_get_from_db(zone, zone->db, NULL, - NULL, &serial, NULL, + &soacount, &serial, NULL, NULL, NULL, - NULL); + NULL, NULL); } else result = DNS_R_NOTLOADED; ZONEDB_UNLOCK(&zone->dblock, isc_rwlocktype_read); - if (result == ISC_R_SUCCESS) + if (result == ISC_R_SUCCESS && soacount > 0U) zone_send_secureserial(zone, ISC_FALSE, serial); } @@ -9950,7 +9976,7 @@ stub_callback(isc_task_t *task, isc_event_t *event) { isc_time_t now; isc_boolean_t exiting = ISC_FALSE; isc_interval_t i; - unsigned int j; + unsigned int j, soacount; stub = revent->ev_arg; INSIST(DNS_STUB_VALID(stub)); @@ -10093,9 +10119,9 @@ stub_callback(isc_task_t *task, isc_event_t *event) { ZONEDB_LOCK(&zone->dblock, isc_rwlocktype_write); if (zone->db == NULL) zone_attachdb(zone, stub->db); - result = zone_get_from_db(zone, zone->db, NULL, NULL, NULL, &refresh, - &retry, &expire, NULL, NULL); - if (result == ISC_R_SUCCESS) { + result = zone_get_from_db(zone, zone->db, NULL, &soacount, NULL, + &refresh, &retry, &expire, NULL, NULL); + if (result == ISC_R_SUCCESS && soacount > 0U) { zone->refresh = RANGE(refresh, zone->minrefresh, zone->maxrefresh); zone->retry = RANGE(retry, zone->minretry, zone->maxretry); @@ -10434,10 +10460,12 @@ refresh_callback(isc_task_t *task, isc_event_t *event) { serial = soa.serial; if (DNS_ZONE_FLAG(zone, DNS_ZONEFLG_LOADED)) { - result = zone_get_from_db(zone, zone->db, NULL, NULL, + unsigned int soacount; + result = zone_get_from_db(zone, zone->db, NULL, &soacount, &oldserial, NULL, NULL, NULL, NULL, NULL); RUNTIME_CHECK(result == ISC_R_SUCCESS); + RUNTIME_CHECK(soacount > 0U); zone_debuglog(zone, me, 1, "serial: new %u, old %u", serial, oldserial); } else @@ -11690,6 +11718,7 @@ dns_zone_notifyreceive(dns_zone_t *zone, isc_sockaddr_t *from, result = dns_rdataset_first(rdataset); if (result == ISC_R_SUCCESS) { isc_uint32_t serial = 0, oldserial; + unsigned int soacount; dns_rdataset_current(rdataset, &rdata); result = dns_rdata_tostruct(&rdata, &soa, NULL); @@ -11699,10 +11728,11 @@ dns_zone_notifyreceive(dns_zone_t *zone, isc_sockaddr_t *from, * The following should safely be performed without DB * lock and succeed in this context. */ - result = zone_get_from_db(zone, zone->db, NULL, NULL, - &oldserial, NULL, NULL, NULL, - NULL, NULL); + result = zone_get_from_db(zone, zone->db, NULL, + &soacount, &oldserial, NULL, + NULL, NULL, NULL, NULL); RUNTIME_CHECK(result == ISC_R_SUCCESS); + RUNTIME_CHECK(soacount > 0U); if (isc_serial_le(serial, oldserial)) { dns_zone_log(zone, ISC_LOG_INFO, @@ -12964,6 +12994,7 @@ zone_replacedb(dns_zone_t *zone, dns_db_t *db, isc_boolean_t dump) { !DNS_ZONE_FLAG(zone, DNS_ZONEFLG_FORCEXFER)) { isc_uint32_t serial, oldserial; + unsigned int soacount; dns_zone_log(zone, ISC_LOG_DEBUG(3), "generating diffs"); @@ -12978,10 +13009,11 @@ zone_replacedb(dns_zone_t *zone, dns_db_t *db, isc_boolean_t dump) { /* * This is checked in zone_postload() for master zones. */ - result = zone_get_from_db(zone, zone->db, NULL, NULL, + result = zone_get_from_db(zone, zone->db, NULL, &soacount, &oldserial, NULL, NULL, NULL, NULL, NULL); RUNTIME_CHECK(result == ISC_R_SUCCESS); + RUNTIME_CHECK(soacount > 0U); if ((zone->type == dns_zone_slave || (zone->type == dns_zone_redirect && zone->masters != NULL)) From 9f0a04a8294f2e8defd697f94b22a4263e9b90f5 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Fri, 30 Nov 2012 12:20:59 +1100 Subject: [PATCH 2/7] add /opt/local/share/xsl/docbook-xsl to list of locations to look for docbook files --- configure.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.in b/configure.in index d974671e3d..0019f3cced 100644 --- a/configure.in +++ b/configure.in @@ -3086,7 +3086,7 @@ AC_ARG_WITH(docbook-xsl, case "$docbook_path" in auto) AC_MSG_RESULT(auto) - docbook_xsl_trees="/usr/pkg/share/xsl/docbook /usr/local/share/xsl/docbook /usr/share/xsl/docbook /opt/local/share/xsl/docbook-xsl/" + docbook_xsl_trees="/usr/pkg/share/xsl/docbook /usr/local/share/xsl/docbook /usr/share/xsl/docbook /opt/local/share/xsl/docbook-xsl" ;; *) docbook_xsl_trees="$withval" From ab95e7c3a237112427d2eeb9ee253f53de41de3a Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Fri, 30 Nov 2012 12:32:00 +1100 Subject: [PATCH 3/7] regen --- configure | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure b/configure index 246a2d4695..320f3b88f1 100755 --- a/configure +++ b/configure @@ -18992,7 +18992,7 @@ case "$docbook_path" in auto) { $as_echo "$as_me:${as_lineno-$LINENO}: result: auto" >&5 $as_echo "auto" >&6; } - docbook_xsl_trees="/usr/pkg/share/xsl/docbook /usr/local/share/xsl/docbook /usr/share/xsl/docbook /opt/local/share/xsl/docbook-xsl/" + docbook_xsl_trees="/usr/pkg/share/xsl/docbook /usr/local/share/xsl/docbook /usr/share/xsl/docbook /opt/local/share/xsl/docbook-xsl" ;; *) docbook_xsl_trees="$withval" From 85a873f00020e088418527a98f38ac684c937d46 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Fri, 30 Nov 2012 16:19:00 +1100 Subject: [PATCH 4/7] conditionally silence false positives from clang --analyze --- lib/dns/tsig.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/lib/dns/tsig.c b/lib/dns/tsig.c index 31b5cc3b8a..c8f00adbcc 100644 --- a/lib/dns/tsig.c +++ b/lib/dns/tsig.c @@ -974,6 +974,11 @@ dns_tsig_sign(dns_message_t *msg) { if (ret != ISC_R_SUCCESS) goto cleanup_context; } +#if defined(__clang__) && \ + ( __clang_major__ < 4 || (__clang_major__ == 4 && __clang_minor__ < 2)) + /* false positive: http://llvm.org/bugs/show_bug.cgi?id=14461 */ + else memset(&querytsig, 0, sizeof(querytsig)); +#endif /* * Digest the header. @@ -1229,6 +1234,11 @@ dns_tsig_verify(isc_buffer_t *source, dns_message_t *msg, if (ret != ISC_R_SUCCESS) return (ret); } +#if defined(__clang__) && \ + ( __clang_major__ < 4 || (__clang_major__ == 4 && __clang_minor__ < 2)) + /* false positive: http://llvm.org/bugs/show_bug.cgi?id=14461 */ + else memset(&querytsig, 0, sizeof(querytsig)); +#endif /* * Do the key name and algorithm match that of the query? From 4151109b94d197eb568b279b73022901c20e21b6 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Fri, 30 Nov 2012 18:50:38 +1100 Subject: [PATCH 5/7] silence clang --analyze warnings --- bin/named/client.c | 4 ++-- lib/dns/dispatch.c | 2 +- lib/dns/rbt.c | 4 ++++ lib/dns/rbtdb.c | 1 + lib/isc/include/isc/queue.h | 4 ++-- 5 files changed, 10 insertions(+), 5 deletions(-) diff --git a/bin/named/client.c b/bin/named/client.c index 4eb6d3e542..0bca10a2d0 100644 --- a/bin/named/client.c +++ b/bin/named/client.c @@ -2594,10 +2594,10 @@ ns_clientmgr_create(isc_mem_t *mctx, isc_taskmgr_t *taskmgr, return (ISC_R_SUCCESS); cleanup_listlock: - isc_mutex_destroy(&manager->listlock); + (void) isc_mutex_destroy(&manager->listlock); cleanup_lock: - isc_mutex_destroy(&manager->lock); + (void) isc_mutex_destroy(&manager->lock); cleanup_manager: isc_mem_put(manager->mctx, manager, sizeof(*manager)); diff --git a/lib/dns/dispatch.c b/lib/dns/dispatch.c index e6786954af..b9086b737b 100644 --- a/lib/dns/dispatch.c +++ b/lib/dns/dispatch.c @@ -739,7 +739,7 @@ destroy_disp(isc_task_t *task, isc_event_t *event) { if (disp->sepool != NULL) { isc_mempool_destroy(&disp->sepool); - isc_mutex_destroy(&disp->sepool_lock); + (void)isc_mutex_destroy(&disp->sepool_lock); } if (disp->socket != NULL) diff --git a/lib/dns/rbt.c b/lib/dns/rbt.c index 53d5410432..059822f67b 100644 --- a/lib/dns/rbt.c +++ b/lib/dns/rbt.c @@ -2065,6 +2065,8 @@ rehash(dns_rbt_t *rbt) { return; } + INSIST(rbt->hashsize > 0); + for (i = 0; i < rbt->hashsize; i++) rbt->hashtable[i] = NULL; @@ -2475,6 +2477,7 @@ deletefromlevel(dns_rbtnode_t *delete, dns_rbtnode_t **rootp) { COLOR(sibling) = COLOR(parent); MAKE_BLACK(parent); + INSIST(RIGHT(sibling) != NULL); MAKE_BLACK(RIGHT(sibling)); rotate_left(parent, rootp); child = *rootp; @@ -2512,6 +2515,7 @@ deletefromlevel(dns_rbtnode_t *delete, dns_rbtnode_t **rootp) { COLOR(sibling) = COLOR(parent); MAKE_BLACK(parent); + INSIST(LEFT(sibling) != NULL); MAKE_BLACK(LEFT(sibling)); rotate_right(parent, rootp); child = *rootp; diff --git a/lib/dns/rbtdb.c b/lib/dns/rbtdb.c index a2521f4a39..b75372e4ca 100644 --- a/lib/dns/rbtdb.c +++ b/lib/dns/rbtdb.c @@ -6345,6 +6345,7 @@ add(dns_rbtdb_t *rbtdb, dns_rbtnode_t *rbtnode, rbtdb_version_t *rbtversion, * will do it on the LRU side, so memory * will not leak... for long. */ + INSIST(rbtdb->heaps != NULL); isc_heap_insert(rbtdb->heaps[idx], newheader); } else if (RESIGN(newheader)) resign_insert(rbtdb, idx, newheader); diff --git a/lib/isc/include/isc/queue.h b/lib/isc/include/isc/queue.h index fc421beaa8..d71bce7986 100644 --- a/lib/isc/include/isc/queue.h +++ b/lib/isc/include/isc/queue.h @@ -61,8 +61,8 @@ #define ISC_QUEUE_DESTROY(queue) \ do { \ ISC_QLINK_INSIST(ISC_QUEUE_EMPTY(queue)); \ - isc_mutex_destroy(&(queue).taillock); \ - isc_mutex_destroy(&(queue).headlock); \ + (void) isc_mutex_destroy(&(queue).taillock); \ + (void) isc_mutex_destroy(&(queue).headlock); \ } while (0) /* From 0524248a3b86ecc4ef944cade9c9341fd5668e09 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Fri, 30 Nov 2012 23:35:34 +1100 Subject: [PATCH 6/7] signed/unsigned comparision --- bin/dig/dig.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/dig/dig.c b/bin/dig/dig.c index cbe3705963..203ed4eb94 100644 --- a/bin/dig/dig.c +++ b/bin/dig/dig.c @@ -259,7 +259,7 @@ received(int bytes, isc_sockaddr_t *from, dig_query_t *query) { time(&tnow); tmnow = *localtime(&tnow); if (strftime(time_str, sizeof(time_str), - "%a %b %d %T %Z %Y", &tmnow) > 0) + "%a %b %d %T %Z %Y", &tmnow) > 0U) printf(";; WHEN: %s\n", time_str); if (query->lookup->doing_xfr) { printf(";; XFR size: %u records (messages %u, " From 68ba0155abd6be12dc211527e5420a7f124e0571 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Sat, 1 Dec 2012 09:19:29 +1100 Subject: [PATCH 7/7] silence clang --analyze warning --- bin/named/query.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/bin/named/query.c b/bin/named/query.c index 25b70d82a5..ffb519dfb7 100644 --- a/bin/named/query.c +++ b/bin/named/query.c @@ -4243,6 +4243,8 @@ rpz_find(ns_client_t *client, dns_rdatatype_t qtype, dns_name_t *qnamef, dns_clientinfomethods_t cm; dns_clientinfo_t ci; + REQUIRE(nodep != NULL); + dns_clientinfomethods_init(&cm, ns_client_sourceip); dns_clientinfo_init(&ci, client);