From: Wouter Wijngaards Date: Mon, 18 Jun 2012 14:22:29 +0000 (+0000) Subject: code review. X-Git-Tag: release-1.4.18rc1~35 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=15aacbe89b640cfa337eab28f131a74d7ae3229e;p=thirdparty%2Funbound.git code review. git-svn-id: file:///svn/unbound/trunk@2688 be551aaa-1e26-0410-a405-d3ace91eadb9 --- diff --git a/daemon/remote.c b/daemon/remote.c index 38ca15c85..f2e8f3ff9 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -1393,6 +1393,7 @@ parse_delegpt(SSL* ssl, char* args, uint8_t* nm, int allow_names) } if(!delegpt_add_ns_mlc(dp, n, 0)) { (void)ssl_printf(ssl, "error out of memory\n"); + free(n); delegpt_free_mlc(dp); return NULL; } @@ -1442,7 +1443,6 @@ do_forward(SSL* ssl, struct worker* worker, char* args) return; if(!forwards_add_zone(fwd, LDNS_RR_CLASS_IN, dp)) { (void)ssl_printf(ssl, "error out of memory\n"); - delegpt_free_mlc(dp); return; } } @@ -1514,7 +1514,6 @@ do_forward_add(SSL* ssl, struct worker* worker, char* args) } if(!forwards_add_zone(fwd, LDNS_RR_CLASS_IN, dp)) { (void)ssl_printf(ssl, "error out of memory\n"); - delegpt_free_mlc(dp); free(nm); return; } @@ -1571,7 +1570,6 @@ do_stub_add(SSL* ssl, struct worker* worker, char* args) forwards_delete_stub_hole(fwd, LDNS_RR_CLASS_IN, nm); if(insecure) anchors_delete_insecure(worker->env.anchors, LDNS_RR_CLASS_IN, nm); - delegpt_free_mlc(dp); free(nm); return; } diff --git a/doc/Changelog b/doc/Changelog index 77511edca..71409d0b7 100644 --- a/doc/Changelog +++ b/doc/Changelog @@ -1,3 +1,6 @@ +18 June 2012: Wouter + - code review. + 15 June 2012: Wouter - implement log-time-ascii on windows. - The key-cache bad key ttl is now 60 seconds. diff --git a/iterator/iter_fwd.c b/iterator/iter_fwd.c index f12531f47..17ca56674 100644 --- a/iterator/iter_fwd.c +++ b/iterator/iter_fwd.c @@ -250,10 +250,12 @@ read_forwards(struct iter_forwards* fwd, struct config_file* cfg) struct config_stub* s; for(s = cfg->forwards; s; s = s->next) { struct delegpt* dp; - if(!(dp=read_fwds_name(s)) || - !read_fwds_host(s, dp) || - !read_fwds_addr(s, dp)) + if(!(dp=read_fwds_name(s))) return 0; + if(!read_fwds_host(s, dp) || !read_fwds_addr(s, dp)) { + delegpt_free_mlc(dp); + return 0; + } /* set flag that parent side NS information is included. * Asking a (higher up) server on the internet is not useful */ /* the flag is turned off for 'forward-first' so that the diff --git a/iterator/iter_hints.c b/iterator/iter_hints.c index cfb9db7ab..09e473113 100644 --- a/iterator/iter_hints.c +++ b/iterator/iter_hints.c @@ -126,32 +126,35 @@ compile_time_root_prime(int do_ip4, int do_ip6) return NULL; dp->has_parent_side_NS = 1; if(do_ip4) { - if(!ah(dp, "A.ROOT-SERVERS.NET.", "198.41.0.4")) return 0; - if(!ah(dp, "B.ROOT-SERVERS.NET.", "192.228.79.201")) return 0; - if(!ah(dp, "C.ROOT-SERVERS.NET.", "192.33.4.12")) return 0; - if(!ah(dp, "D.ROOT-SERVERS.NET.", "128.8.10.90")) return 0; - if(!ah(dp, "E.ROOT-SERVERS.NET.", "192.203.230.10")) return 0; - if(!ah(dp, "F.ROOT-SERVERS.NET.", "192.5.5.241")) return 0; - if(!ah(dp, "G.ROOT-SERVERS.NET.", "192.112.36.4")) return 0; - if(!ah(dp, "H.ROOT-SERVERS.NET.", "128.63.2.53")) return 0; - if(!ah(dp, "I.ROOT-SERVERS.NET.", "192.36.148.17")) return 0; - if(!ah(dp, "J.ROOT-SERVERS.NET.", "192.58.128.30")) return 0; - if(!ah(dp, "K.ROOT-SERVERS.NET.", "193.0.14.129")) return 0; - if(!ah(dp, "L.ROOT-SERVERS.NET.", "199.7.83.42")) return 0; - if(!ah(dp, "M.ROOT-SERVERS.NET.", "202.12.27.33")) return 0; + if(!ah(dp, "A.ROOT-SERVERS.NET.", "198.41.0.4")) goto failed; + if(!ah(dp, "B.ROOT-SERVERS.NET.", "192.228.79.201")) goto failed; + if(!ah(dp, "C.ROOT-SERVERS.NET.", "192.33.4.12")) goto failed; + if(!ah(dp, "D.ROOT-SERVERS.NET.", "128.8.10.90")) goto failed; + if(!ah(dp, "E.ROOT-SERVERS.NET.", "192.203.230.10")) goto failed; + if(!ah(dp, "F.ROOT-SERVERS.NET.", "192.5.5.241")) goto failed; + if(!ah(dp, "G.ROOT-SERVERS.NET.", "192.112.36.4")) goto failed; + if(!ah(dp, "H.ROOT-SERVERS.NET.", "128.63.2.53")) goto failed; + if(!ah(dp, "I.ROOT-SERVERS.NET.", "192.36.148.17")) goto failed; + if(!ah(dp, "J.ROOT-SERVERS.NET.", "192.58.128.30")) goto failed; + if(!ah(dp, "K.ROOT-SERVERS.NET.", "193.0.14.129")) goto failed; + if(!ah(dp, "L.ROOT-SERVERS.NET.", "199.7.83.42")) goto failed; + if(!ah(dp, "M.ROOT-SERVERS.NET.", "202.12.27.33")) goto failed; } if(do_ip6) { - if(!ah(dp, "A.ROOT-SERVERS.NET.", "2001:503:ba3e::2:30")) return 0; - if(!ah(dp, "D.ROOT-SERVERS.NET.", "2001:500:2d::d")) return 0; - if(!ah(dp, "F.ROOT-SERVERS.NET.", "2001:500:2f::f")) return 0; - if(!ah(dp, "H.ROOT-SERVERS.NET.", "2001:500:1::803f:235")) return 0; - if(!ah(dp, "I.ROOT-SERVERS.NET.", "2001:7fe::53")) return 0; - if(!ah(dp, "J.ROOT-SERVERS.NET.", "2001:503:c27::2:30")) return 0; - if(!ah(dp, "K.ROOT-SERVERS.NET.", "2001:7fd::1")) return 0; - if(!ah(dp, "L.ROOT-SERVERS.NET.", "2001:500:3::42")) return 0; - if(!ah(dp, "M.ROOT-SERVERS.NET.", "2001:dc3::35")) return 0; + if(!ah(dp, "A.ROOT-SERVERS.NET.", "2001:503:ba3e::2:30")) goto failed; + if(!ah(dp, "D.ROOT-SERVERS.NET.", "2001:500:2d::d")) goto failed; + if(!ah(dp, "F.ROOT-SERVERS.NET.", "2001:500:2f::f")) goto failed; + if(!ah(dp, "H.ROOT-SERVERS.NET.", "2001:500:1::803f:235")) goto failed; + if(!ah(dp, "I.ROOT-SERVERS.NET.", "2001:7fe::53")) goto failed; + if(!ah(dp, "J.ROOT-SERVERS.NET.", "2001:503:c27::2:30")) goto failed; + if(!ah(dp, "K.ROOT-SERVERS.NET.", "2001:7fd::1")) goto failed; + if(!ah(dp, "L.ROOT-SERVERS.NET.", "2001:500:3::42")) goto failed; + if(!ah(dp, "M.ROOT-SERVERS.NET.", "2001:dc3::35")) goto failed; } return dp; +failed: + delegpt_free_mlc(dp); + return 0; } /** insert new hint info into hint structure */ @@ -253,17 +256,19 @@ read_stubs(struct iter_hints* hints, struct config_file* cfg) struct config_stub* s; struct delegpt* dp; for(s = cfg->stubs; s; s = s->next) { - if(!(dp=read_stubs_name(s)) || - !read_stubs_host(s, dp) || - !read_stubs_addr(s, dp)) + if(!(dp=read_stubs_name(s))) + return 0; + if(!read_stubs_host(s, dp) || !read_stubs_addr(s, dp)) { + delegpt_free_mlc(dp); return 0; + } /* the flag is turned off for 'stub-first' so that the * last resort will ask for parent-side NS record and thus * fallback to the internet name servers on a failure */ dp->has_parent_side_NS = (uint8_t)!s->isfirst; + delegpt_log(VERB_QUERY, dp); if(!hints_insert(hints, LDNS_RR_CLASS_IN, dp, !s->isprime)) return 0; - delegpt_log(VERB_QUERY, dp); } return 1; } diff --git a/services/listen_dnsport.c b/services/listen_dnsport.c index 59ca1991e..647cbe07e 100644 --- a/services/listen_dnsport.c +++ b/services/listen_dnsport.c @@ -323,6 +323,11 @@ create_udp_sock(int family, int socktype, struct sockaddr* addr, log_err("setsockopt(..., IP_MTU_DISCOVER, " "IP_PMTUDISC_DONT...) failed: %s", strerror(errno)); +# ifndef USE_WINSOCK + close(s); +# else + closesocket(s); +# endif return -1; } # elif defined(IP_DONTFRAG) @@ -331,6 +336,11 @@ create_udp_sock(int family, int socktype, struct sockaddr* addr, &off, (socklen_t)sizeof(off)) < 0) { log_err("setsockopt(..., IP_DONTFRAG, ...) failed: %s", strerror(errno)); +# ifndef USE_WINSOCK + close(s); +# else + closesocket(s); +# endif return -1; } # endif /* IPv4 MTU */ @@ -408,9 +418,11 @@ create_tcp_accept_sock(struct addrinfo *addr, int v6only, int* noproto) #ifndef USE_WINSOCK log_err("setsockopt(.. SO_REUSEADDR ..) failed: %s", strerror(errno)); + close(s); #else log_err("setsockopt(.. SO_REUSEADDR ..) failed: %s", wsa_strerror(WSAGetLastError())); + closesocket(s); #endif return -1; } @@ -422,9 +434,11 @@ create_tcp_accept_sock(struct addrinfo *addr, int v6only, int* noproto) #ifndef USE_WINSOCK log_err("setsockopt(..., IPV6_V6ONLY, ...) failed: %s", strerror(errno)); + close(s); #else log_err("setsockopt(..., IPV6_V6ONLY, ...) failed: %s", wsa_strerror(WSAGetLastError())); + closesocket(s); #endif return -1; } @@ -443,23 +457,32 @@ create_tcp_accept_sock(struct addrinfo *addr, int v6only, int* noproto) (struct sockaddr_storage*)addr->ai_addr, addr->ai_addrlen); } + close(s); #else log_err("can't bind socket: %s", wsa_strerror(WSAGetLastError())); log_addr(0, "failed address", (struct sockaddr_storage*)addr->ai_addr, addr->ai_addrlen); + closesocket(s); #endif return -1; } if(!fd_set_nonblock(s)) { +#ifndef USE_WINSOCK + close(s); +#else + closesocket(s); +#endif return -1; } if(listen(s, TCP_BACKLOG) == -1) { #ifndef USE_WINSOCK log_err("can't listen: %s", strerror(errno)); + close(s); #else log_err("can't listen: %s", wsa_strerror(WSAGetLastError())); + closesocket(s); #endif return -1; } @@ -653,8 +676,14 @@ ports_create_if(const char* ifname, int do_auto, int do_udp, int do_tcp, return 0; } /* getting source addr packet info is highly non-portable */ - if(!set_recvpktinfo(s, hints->ai_family)) + if(!set_recvpktinfo(s, hints->ai_family)) { +#ifndef USE_WINSOCK + close(s); +#else + closesocket(s); +#endif return 0; + } if(!port_insert(list, s, listen_type_udpancil)) { #ifndef USE_WINSOCK close(s); diff --git a/smallapp/unbound-control.c b/smallapp/unbound-control.c index 58be7b7ab..a5aa92bf9 100644 --- a/smallapp/unbound-control.c +++ b/smallapp/unbound-control.c @@ -379,7 +379,8 @@ int main(int argc, char* argv[]) if(!RAND_status()) { /* try to seed it */ unsigned char buf[256]; - unsigned int v, seed=(unsigned)time(NULL) ^ (unsigned)getpid(); + unsigned int seed=(unsigned)time(NULL) ^ (unsigned)getpid(); + unsigned int v = seed; size_t i; for(i=0; i<256/sizeof(v); i++) { memmove(buf+i*sizeof(v), &v, sizeof(v)); diff --git a/util/config_file.c b/util/config_file.c index 8ba79d2a2..2f5e988cb 100644 --- a/util/config_file.c +++ b/util/config_file.c @@ -1038,9 +1038,9 @@ static int isalldigit(const char* str, size_t l) int cfg_parse_memsize(const char* str, size_t* res) { - size_t len = (size_t)strlen(str); + size_t len; size_t mult = 1; - if(!str || len == 0) { + if(!str || (len=(size_t)strlen(str)) == 0) { log_err("not a size: '%s'", str); return 0; } diff --git a/util/data/msgparse.c b/util/data/msgparse.c index a03f543e8..c51c4406d 100644 --- a/util/data/msgparse.c +++ b/util/data/msgparse.c @@ -655,8 +655,10 @@ calc_size(ldns_buffer* pkt, uint16_t type, struct rr_parse* rr) len = 0; break; case LDNS_RDF_TYPE_STR: - if(pkt_len < 1) + if(pkt_len < 1) { + /* NOTREACHED, due to 'while(>0)' */ return 0; /* len byte exceeds rdata */ + } len = ldns_buffer_current(pkt)[0] + 1; break; default: diff --git a/util/tube.c b/util/tube.c index 67294e056..28c51d79d 100644 --- a/util/tube.c +++ b/util/tube.c @@ -360,6 +360,7 @@ int tube_read_msg(struct tube* tube, uint8_t** buf, uint32_t* len, } d += r; } + log_assert(*len < 65536*2); *buf = (uint8_t*)malloc(*len); if(!*buf) { log_err("tube read out of memory"); diff --git a/validator/autotrust.c b/validator/autotrust.c index 989694324..a361ab04b 100644 --- a/validator/autotrust.c +++ b/validator/autotrust.c @@ -1851,6 +1851,7 @@ static void autr_tp_remove(struct module_env* env, struct trust_anchor* tp, struct ub_packed_rrset_key* dnskey_rrset) { + struct trust_anchor* del_tp; struct trust_anchor key; struct autr_point_data pd; time_t mold, mnew; @@ -1876,19 +1877,24 @@ autr_tp_remove(struct module_env* env, struct trust_anchor* tp, /* take from tree. It could be deleted by someone else,hence (void). */ lock_basic_lock(&env->anchors->lock); - (void)rbtree_delete(env->anchors->tree, &key); + del_tp = (struct trust_anchor*)rbtree_delete(env->anchors->tree, &key); mold = wait_probe_time(env->anchors); (void)rbtree_delete(&env->anchors->autr->probe, &key); mnew = wait_probe_time(env->anchors); anchors_init_parents_locked(env->anchors); lock_basic_unlock(&env->anchors->lock); - /* save on disk */ - tp->autr->next_probe_time = 0; /* no more probing for it */ - autr_write_file(env, tp); + /* if !del_tp then the trust point is no longer present in the tree, + * it was deleted by someone else, who will write the zonefile and + * clean up the structure */ + if(del_tp) { + /* save on disk */ + del_tp->autr->next_probe_time = 0; /* no more probing for it */ + autr_write_file(env, del_tp); - /* delete */ - autr_point_delete(tp); + /* delete */ + autr_point_delete(del_tp); + } if(mold != mnew) { reset_worker_timer(env); } diff --git a/validator/val_anchor.c b/validator/val_anchor.c index 200bf5d97..fa69df52d 100644 --- a/validator/val_anchor.c +++ b/validator/val_anchor.c @@ -1246,6 +1246,7 @@ anchors_delete_insecure(struct val_anchors* anchors, uint16_t c, lock_basic_lock(&ta->lock); /* see if its really an insecure point */ if(ta->keylist || ta->autr || ta->numDS || ta->numDNSKEY) { + lock_basic_unlock(&anchors->lock); lock_basic_unlock(&ta->lock); /* its not an insecure point, do not remove it */ return; diff --git a/validator/val_sigcrypt.c b/validator/val_sigcrypt.c index 4c79c004d..32dbc0bd7 100644 --- a/validator/val_sigcrypt.c +++ b/validator/val_sigcrypt.c @@ -606,10 +606,14 @@ dnskeyset_verify_rrset(struct module_env* env, struct val_env* ve, (uint8_t)rrset_get_sig_algo(rrset, i)); } } - verbose(VERB_ALGO, "rrset failed to verify: no valid signatures for " - "%d algorithms", (int)algo_needs_num_missing(&needs)); if(sigalg && (alg=algo_needs_missing(&needs)) != 0) { + verbose(VERB_ALGO, "rrset failed to verify: " + "no valid signatures for %d algorithms", + (int)algo_needs_num_missing(&needs)); algo_needs_reason(env, alg, reason, "no signatures"); + } else { + verbose(VERB_ALGO, "rrset failed to verify: " + "no valid signatures"); } return sec_status_bogus; }