From 7ebef930d700ec52dbdbce4255e12d4c96894186 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Wed, 13 Sep 2017 09:50:51 +1000 Subject: [PATCH] 4719. [bug] Address PVS static analyzer warnings. [RT #45946] (cherry picked from commit 34130ee25a904d34062ea1197bf5ac6c2e53c132) --- CHANGES | 2 ++ bin/dig/nslookup.c | 37 ++++++++++++++++++++---------------- bin/dnssec/dnssectool.c | 2 +- bin/named/xfrout.c | 2 -- lib/dns/adb.c | 21 +++++++++++--------- lib/dns/tsig.c | 12 ++++++------ lib/isc/unix/interfaceiter.c | 2 +- 7 files changed, 43 insertions(+), 35 deletions(-) diff --git a/CHANGES b/CHANGES index 72eca539d7..efa5c9a51e 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,5 @@ +4719. [bug] Address PVS static analyzer warnings. [RT #45946] + 4717. [bug] Treat replies with QCOUNT=0 as truncated if TC=1, FORMERR if TC=0, and log the error correctly. [RT #45836] diff --git a/bin/dig/nslookup.c b/bin/dig/nslookup.c index f893cbdc40..be1df73090 100644 --- a/bin/dig/nslookup.c +++ b/bin/dig/nslookup.c @@ -594,7 +594,12 @@ version(void) { static void setoption(char *opt) { - if (strncasecmp(opt, "all", 3) == 0) { + size_t l = strlen(opt); + +#define CHECKOPT(A, N) \ + ((l >= N) && (l < sizeof(A)) && (strncasecmp(opt, A, l) == 0)) + + if (CHECKOPT("all", 3) == 0) { show_settings(ISC_TRUE, ISC_FALSE); } else if (strncasecmp(opt, "class=", 6) == 0) { if (testclass(&opt[6])) @@ -636,41 +641,41 @@ setoption(char *opt) { set_timeout(&opt[8]); } else if (strncasecmp(opt, "t=", 2) == 0) { set_timeout(&opt[2]); - } else if (strncasecmp(opt, "rec", 3) == 0) { + } else if (CHECKOPT("recurse", 3)) { recurse = ISC_TRUE; - } else if (strncasecmp(opt, "norec", 5) == 0) { + } else if (CHECKOPT("norecurse", 5)) { recurse = ISC_FALSE; } else if (strncasecmp(opt, "retry=", 6) == 0) { set_tries(&opt[6]); } else if (strncasecmp(opt, "ret=", 4) == 0) { set_tries(&opt[4]); - } else if (strncasecmp(opt, "def", 3) == 0) { + } else if (CHECKOPT("defname", 3)) { usesearch = ISC_TRUE; - } else if (strncasecmp(opt, "nodef", 5) == 0) { + } else if (CHECKOPT("nodefname", 5)) { usesearch = ISC_FALSE; - } else if (strncasecmp(opt, "vc", 3) == 0) { + } else if (CHECKOPT("vc", 2) == 0) { tcpmode = ISC_TRUE; - } else if (strncasecmp(opt, "novc", 5) == 0) { + } else if (CHECKOPT("novc", 4) == 0) { tcpmode = ISC_FALSE; - } else if (strncasecmp(opt, "deb", 3) == 0) { + } else if (CHECKOPT("debug", 3) == 0) { short_form = ISC_FALSE; showsearch = ISC_TRUE; - } else if (strncasecmp(opt, "nodeb", 5) == 0) { + } else if (CHECKOPT("nodebug", 5) == 0) { short_form = ISC_TRUE; showsearch = ISC_FALSE; - } else if (strncasecmp(opt, "d2", 2) == 0) { + } else if (CHECKOPT("d2", 2) == 0) { debugging = ISC_TRUE; - } else if (strncasecmp(opt, "nod2", 4) == 0) { + } else if (CHECKOPT("nod2", 4) == 0) { debugging = ISC_FALSE; - } else if (strncasecmp(opt, "search", 3) == 0) { + } else if (CHECKOPT("search", 3) == 0) { usesearch = ISC_TRUE; - } else if (strncasecmp(opt, "nosearch", 5) == 0) { + } else if (CHECKOPT("nosearch", 5) == 0) { usesearch = ISC_FALSE; - } else if (strncasecmp(opt, "sil", 3) == 0) { + } else if (CHECKOPT("sil", 3) == 0) { /* deprecation_msg = ISC_FALSE; */ - } else if (strncasecmp(opt, "fail", 3) == 0) { + } else if (CHECKOPT("fail", 3) == 0) { nofail=ISC_FALSE; - } else if (strncasecmp(opt, "nofail", 3) == 0) { + } else if (CHECKOPT("nofail", 5) == 0) { nofail=ISC_TRUE; } else if (strncasecmp(opt, "ndots=", 6) == 0) { set_ndots(&opt[6]); diff --git a/bin/dnssec/dnssectool.c b/bin/dnssec/dnssectool.c index 75c0c3e56f..8d5e3bd5e2 100644 --- a/bin/dnssec/dnssectool.c +++ b/bin/dnssec/dnssectool.c @@ -1839,7 +1839,7 @@ verifyzone(dns_db_t *db, dns_dbversion_t *ver, for (i = 0; i < 256; i++) { if ((ksk_algorithms[i] != 0) || (standby_ksk[i] != 0) || - (revoked_zsk[i] != 0) || + (revoked_ksk[i] != 0) || (zsk_algorithms[i] != 0) || (standby_zsk[i] != 0) || (revoked_zsk[i] != 0)) { diff --git a/bin/named/xfrout.c b/bin/named/xfrout.c index 91dd20b4ad..5c9713ac30 100644 --- a/bin/named/xfrout.c +++ b/bin/named/xfrout.c @@ -1156,8 +1156,6 @@ xfrout_ctx_create(isc_mem_t *mctx, ns_client_t *client, unsigned int id, xfr->tsigkey = tsigkey; xfr->lasttsig = lasttsig; xfr->verified_tsig = verified_tsig; - xfr->txmem = NULL; - xfr->txmemlen = 0; xfr->nmsg = 0; xfr->many_answers = many_answers; xfr->sends = 0; diff --git a/lib/dns/adb.c b/lib/dns/adb.c index f49ace18d9..64bdf6f1ed 100644 --- a/lib/dns/adb.c +++ b/lib/dns/adb.c @@ -2128,7 +2128,7 @@ log_quota(dns_adbentry_t *entry, const char *fmt, ...) { isc_netaddr_format(&netaddr, addrbuf, sizeof(addrbuf)); isc_log_write(dns_lctx, DNS_LOGCATEGORY_DATABASE, DNS_LOGMODULE_ADB, - ISC_LOG_INFO, "adb: quota %s (%d/%d): %s", + ISC_LOG_INFO, "adb: quota %s (%u/%u): %s", addrbuf, entry->active, entry->quota, msgbuf); } #endif /* ENABLE_FETCHLIMIT */ @@ -2803,7 +2803,7 @@ void dns_adb_whenshutdown(dns_adb_t *adb, isc_task_t *task, isc_event_t **eventp) { isc_task_t *tclone; isc_event_t *event; - isc_boolean_t zeroirefcnt = ISC_FALSE; + isc_boolean_t zeroirefcnt; /* * Send '*eventp' to 'task' when 'adb' has shutdown. @@ -2816,8 +2816,8 @@ dns_adb_whenshutdown(dns_adb_t *adb, isc_task_t *task, isc_event_t **eventp) { *eventp = NULL; LOCK(&adb->lock); - LOCK(&adb->reflock); + zeroirefcnt = ISC_TF(adb->irefcnt == 0); if (adb->shutting_down && zeroirefcnt && @@ -3446,10 +3446,10 @@ dump_adb(dns_adb_t *adb, FILE *f, isc_boolean_t debug, isc_stdtime_t now) { print_namehook_list(f, "v6", adb, &name->v6, debug, now); - if (debug) + if (debug) { print_fetch_list(f, name); - if (debug) print_find_list(f, name); + } } } @@ -3499,7 +3499,7 @@ dump_entry(FILE *f, dns_adb_t *adb, dns_adbentry_t *entry, #ifdef ENABLE_FETCHLIMIT if (adb != NULL && adb->quota != 0 && adb->atr_freq != 0) { - fprintf(f, " [atr %0.2f] [quota %d]", + fprintf(f, " [atr %0.2f] [quota %u]", entry->atr, entry->quota); } #endif /* ENABLE_FETCHLIMIT */ @@ -4176,6 +4176,8 @@ static int quota_adj[] = { 312, 307, 303, 298, 294, 290, 286, 282, 278 }; +#define QUOTA_ADJ_SIZE (sizeof(quota_adj)/sizeof(quota_adj[0])) + /* * Caller must hold adbentry lock */ @@ -4214,12 +4216,13 @@ maybe_adjust_quota(dns_adb_t *adb, dns_adbaddrinfo_t *addr, if (addr->entry->atr < adb->atr_low && addr->entry->mode > 0) { addr->entry->quota = adb->quota * quota_adj[--addr->entry->mode] / 10000; - log_quota(addr->entry, "atr %0.2f, quota increased to %d", + log_quota(addr->entry, "atr %0.2f, quota increased to %u", addr->entry->atr, addr->entry->quota); - } else if (addr->entry->atr > adb->atr_high && addr->entry->mode < 99) { + } else if (addr->entry->atr > adb->atr_high && + addr->entry->mode < (QUOTA_ADJ_SIZE - 1)) { addr->entry->quota = adb->quota * quota_adj[++addr->entry->mode] / 10000; - log_quota(addr->entry, "atr %0.2f, quota decreased to %d", + log_quota(addr->entry, "atr %0.2f, quota decreased to %u", addr->entry->atr, addr->entry->quota); } diff --git a/lib/dns/tsig.c b/lib/dns/tsig.c index 49caa2f501..d5c5df76ad 100644 --- a/lib/dns/tsig.c +++ b/lib/dns/tsig.c @@ -1692,13 +1692,13 @@ tsig_verify_tcp(isc_buffer_t *source, dns_message_t *msg) { addcount_n = ntohs(addcount); addcount = htons((isc_uint16_t)(addcount_n - 1)); memmove(&header[DNS_MESSAGE_HEADERLEN - 2], &addcount, 2); - } - /* - * Put in the original id. - */ - /* XXX Can TCP transfers be forwarded? How would that work? */ - if (has_tsig) { + /* + * Put in the original id. + * + * XXX Can TCP transfers be forwarded? How would that + * work? + */ id = htons(tsig.originalid); memmove(&header[0], &id, 2); } diff --git a/lib/isc/unix/interfaceiter.c b/lib/isc/unix/interfaceiter.c index 7272f71823..792a00c230 100644 --- a/lib/isc/unix/interfaceiter.c +++ b/lib/isc/unix/interfaceiter.c @@ -186,7 +186,7 @@ linux_if_inet6_current(isc_interfaceiter_t *iter) { char address[33]; char name[IF_NAMESIZE+1]; struct in6_addr addr6; - int ifindex, prefix, flag3, flag4; + unsigned int ifindex, prefix, flag3, flag4; int res; unsigned int i;