From 6355b2a4caeacf25cab5f790ae7e163b1fe0d1f0 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 26 Jul 2012 10:16:47 -0400 Subject: [PATCH 1/6] Remove unused variable; spotted by coverity --- test/regress_dns.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/test/regress_dns.c b/test/regress_dns.c index 557a63d6..9a2e4d76 100644 --- a/test/regress_dns.c +++ b/test/regress_dns.c @@ -1048,7 +1048,6 @@ test_bufferevent_connect_hostname(void *arg) int expect_err5; struct evdns_base *dns=NULL; struct evdns_server_port *port=NULL; - evutil_socket_t server_fd=-1; struct sockaddr_in sin; int listener_port=-1; ev_uint16_t dns_port=0; @@ -1150,8 +1149,6 @@ test_bufferevent_connect_hostname(void *arg) end: if (listener) evconnlistener_free(listener); - if (server_fd>=0) - evutil_closesocket(server_fd); if (port) evdns_close_server_port(port); if (dns) From 6a4ec5c2b580a6174f2713ba1743e76851e1ca3c Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 26 Jul 2012 10:34:06 -0400 Subject: [PATCH 2/6] Avoid possible needless call to writev. Found by coverity. --- buffer.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/buffer.c b/buffer.c index ff64aede..5fecb25b 100644 --- a/buffer.c +++ b/buffer.c @@ -2263,6 +2263,8 @@ evbuffer_write_iovec(struct evbuffer *buffer, evutil_socket_t fd, } chain = chain->next; } + if (! i) + return 0; #ifdef WIN32 { DWORD bytesSent; From b9e7329751e9adf97839833c77d5b11648b408d0 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 26 Jul 2012 10:37:13 -0400 Subject: [PATCH 3/6] Add checks to various return values in unit tests. Found by coverity --- test/bench_cascade.c | 9 ++++++--- test/bench_httpclient.c | 5 +++-- test/regress_et.c | 2 +- test/regress_listener.c | 2 +- test/regress_testutils.c | 8 ++++++-- test/test-eof.c | 3 ++- 6 files changed, 19 insertions(+), 10 deletions(-) diff --git a/test/bench_cascade.c b/test/bench_cascade.c index 08b7f945..a614156e 100644 --- a/test/bench_cascade.c +++ b/test/bench_cascade.c @@ -70,8 +70,10 @@ read_cb(evutil_socket_t fd, short which, void *arg) long idx = (long) arg; recv(fd, &ch, sizeof(ch), 0); - if (idx >= 0) - send(idx, "e", 1, 0); + if (idx >= 0) { + if (send(idx, "e", 1, 0) < 0) + perror("send"); + } fired++; } @@ -112,7 +114,8 @@ run_once(int num_pipes) fired = 0; /* kick everything off with a single write */ - send(pipes[1], "e", 1, 0); + if (send(pipes[1], "e", 1, 0) < 0) + perror("send"); event_dispatch(); diff --git a/test/bench_httpclient.c b/test/bench_httpclient.c index cf667534..581b23da 100644 --- a/test/bench_httpclient.c +++ b/test/bench_httpclient.c @@ -115,11 +115,12 @@ frob_socket(evutil_socket_t sock) { struct linger l; int one = 1; - setsockopt(sock, SOL_SOCKET, SO_REUSEADDR, (void*)&one, sizeof(one)); + if (setsockopt(sock, SOL_SOCKET, SO_REUSEADDR, (void*)&one, sizeof(one))<0) + perror("setsockopt(SO_REUSEADDR)"); l.l_onoff = 1; l.l_linger = 0; if (setsockopt(sock, SOL_SOCKET, SO_LINGER, (void*)&l, sizeof(l))<0) - perror("setsockopt"); + perror("setsockopt(SO_LINGER)"); } static int diff --git a/test/regress_et.c b/test/regress_et.c index 54ee9326..3ddd9edd 100644 --- a/test/regress_et.c +++ b/test/regress_et.c @@ -104,7 +104,7 @@ test_edgetriggered(void *et) called = was_et = 0; - send(pair[0], test, (int)strlen(test)+1, 0); + tt_int_op(send(pair[0], test, (int)strlen(test)+1, 0), >, 0); shutdown(pair[0], SHUT_WR); /* Initalize the event library */ diff --git a/test/regress_listener.c b/test/regress_listener.c index f6c22cb0..8938546d 100644 --- a/test/regress_listener.c +++ b/test/regress_listener.c @@ -162,7 +162,7 @@ regress_listener_error(void *arg) } /* send, so that pair[0] will look 'readable'*/ - send(data->pair[1], "hello", 5, 0); + tt_int_op(send(data->pair[1], "hello", 5, 0), >, 0); /* Start a listener with a bogus socket. */ listener = evconnlistener_new(base, acceptcb, &count, diff --git a/test/regress_testutils.c b/test/regress_testutils.c index aedaef40..c70c9a17 100644 --- a/test/regress_testutils.c +++ b/test/regress_testutils.c @@ -179,12 +179,16 @@ regress_dns_server_cb(struct evdns_server_request *req, void *data) return; } else if (!strcmp(tab->anstype, "A")) { struct in_addr in; - evutil_inet_pton(AF_INET, tab->ans, &in); + if (!evutil_inet_pton(AF_INET, tab->ans, &in)) { + TT_DIE(("Bad A value %s in table", tab->ans)); + } evdns_server_request_add_a_reply(req, question, 1, &in.s_addr, 100); } else if (!strcmp(tab->anstype, "AAAA")) { struct in6_addr in6; - evutil_inet_pton(AF_INET6, tab->ans, &in6); + if (!evutil_inet_pton(AF_INET6, tab->ans, &in6)) { + TT_DIE(("Bad AAAA value %s in table", tab->ans)); + } evdns_server_request_add_aaaa_reply(req, question, 1, &in6.s6_addr, 100); } else { diff --git a/test/test-eof.c b/test/test-eof.c index 3f470ce2..47da694b 100644 --- a/test/test-eof.c +++ b/test/test-eof.c @@ -106,7 +106,8 @@ main(int argc, char **argv) return (1); - send(pair[0], test, (int)strlen(test)+1, 0); + if (send(pair[0], test, (int)strlen(test)+1, 0) < 0) + return (1); shutdown(pair[0], SHUT_WR); /* Initalize the event library */ From a2006c008740dfc6b4260d8848f80d76b192f75a Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 26 Jul 2012 10:37:47 -0400 Subject: [PATCH 4/6] Move assignment outside tt_assert in ssl unit tests. Appeases coverity. --- test/regress_ssl.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/regress_ssl.c b/test/regress_ssl.c index a2b7f05b..8e0087aa 100644 --- a/test/regress_ssl.c +++ b/test/regress_ssl.c @@ -108,7 +108,8 @@ getcert(void) name = X509_NAME_new(); tt_assert(name); - tt_assert(NID_undef != (nid = OBJ_txt2nid("commonName"))); + nid = OBJ_txt2nid("commonName"); + tt_assert(NID_undef != nid); tt_assert(0 != X509_NAME_add_entry_by_NID( name, nid, MBSTRING_ASC, (unsigned char*)"example.com", -1, -1, 0)); From a1a0e675318da89f2b13eb65a1f8cab601123bf9 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 26 Jul 2012 10:38:29 -0400 Subject: [PATCH 5/6] memset sockaddr_in before using it. Found by coverity. --- evdns.c | 1 + 1 file changed, 1 insertion(+) diff --git a/evdns.c b/evdns.c index 416137ba..d648c936 100644 --- a/evdns.c +++ b/evdns.c @@ -2529,6 +2529,7 @@ evdns_base_nameserver_add(struct evdns_base *base, unsigned long int address) { struct sockaddr_in sin; int res; + memset(&sin, 0, sizeof(sin)); sin.sin_addr.s_addr = address; sin.sin_port = htons(53); sin.sin_family = AF_INET; From a0912e32068621eb776d678224e4108511d281e3 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 26 Jul 2012 10:39:05 -0400 Subject: [PATCH 6/6] Check more setsockopt return values when binding sockets. Found by coverity --- http.c | 9 ++++++--- listener.c | 10 ++++++++-- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/http.c b/http.c index b9714d33..00a3499b 100644 --- a/http.c +++ b/http.c @@ -3825,9 +3825,12 @@ bind_socket_ai(struct evutil_addrinfo *ai, int reuse) if (evutil_make_socket_closeonexec(fd) < 0) goto out; - setsockopt(fd, SOL_SOCKET, SO_KEEPALIVE, (void *)&on, sizeof(on)); - if (reuse) - evutil_make_listen_socket_reuseable(fd); + if (setsockopt(fd, SOL_SOCKET, SO_KEEPALIVE, (void *)&on, sizeof(on))<0) + goto out; + if (reuse) { + if (evutil_make_listen_socket_reuseable(fd) < 0) + goto out; + } if (ai != NULL) { r = bind(fd, ai->ai_addr, (ev_socklen_t)ai->ai_addrlen); diff --git a/listener.c b/listener.c index 9941e1d8..a78ca0d6 100644 --- a/listener.c +++ b/listener.c @@ -226,9 +226,15 @@ evconnlistener_new_bind(struct event_base *base, evconnlistener_cb cb, } } - setsockopt(fd, SOL_SOCKET, SO_KEEPALIVE, (void*)&on, sizeof(on)); + if (setsockopt(fd, SOL_SOCKET, SO_KEEPALIVE, (void*)&on, sizeof(on))<0) { + evutil_closesocket(fd); + return NULL; + } if (flags & LEV_OPT_REUSEABLE) { - evutil_make_listen_socket_reuseable(fd); + if (evutil_make_listen_socket_reuseable(fd) < 0) { + evutil_closesocket(fd); + return NULL; + } } if (sa) {