evdns: fix a crash when evdns_base with waiting requests is freed

Fix undefined behaviour and application crash that might take
place in some rare cases after calling evdns_base_free when
there are requests in the waiting queue.

Current cleanup procedure in evdns_base_free_and_unlock
function includes 2 steps:
1. Finish all inflight requests.
2. Finish all waiting requests.
During the first step we iterate over each list in req_heads
structure and finish all requests in these lists. With current
logic finishing an inflight request (function request_finished)
removes it from the inflight requests container and forces
a wating connection to be sent (by calling
evdns_requests_pump_waiting_queue). When these new requests are
sent it is possible that they will be inserted to the list in
req_heads that we've already cleaned.
So in some cases container of the inflight requests is not empty
after this procedure and some requests are not finished and
deleted. When timeouts for these requests expire
evdns_request_timeout_callback is called but corresponding
evdns_base has been already deleted which causes undefined
behaviour and possible applicaton crash.

It is interesting to note that in old versions of libevent such
situation was not possible. This bug was introduced by the commit
14f84bbdc77d90b1d936076661443cdbf516c593. Before this commit
nameservers were deleted before finishing the requests. Therefore
it was not possible that requests from the waiting queue be sent
while we finish the inflight requests.

1 files changed, 5 insertions, 5 deletions

diff --git a/evdns.c b/evdns.c
index 0e8db53a..d820aa70 100644
--- a/evdns.c
+++ b/evdns.c
@@ -4106,6 +4106,11 @@ evdns_base_free_and_unlock(struct evdns_base *base, int fail_requests)
 
 	/* TODO(nickm) we might need to refcount here. */
 
+	while (base->req_waiting_head) {
+		if (fail_requests)
+			reply_schedule_callback(base->req_waiting_head, 0, DNS_ERR_SHUTDOWN, NULL);
+		request_finished(base->req_waiting_head, &base->req_waiting_head, 1);
+	}
 	for (i = 0; i < base->n_req_heads; ++i) {
 		while (base->req_heads[i]) {
 			if (fail_requests)
@@ -4113,11 +4118,6 @@ evdns_base_free_and_unlock(struct evdns_base *base, int fail_requests)
 			request_finished(base->req_heads[i], &REQ_HEAD(base, base->req_heads[i]->trans_id), 1);
 		}
 	}
-	while (base->req_waiting_head) {
-		if (fail_requests)
-			reply_schedule_callback(base->req_waiting_head, 0, DNS_ERR_SHUTDOWN, NULL);
-		request_finished(base->req_waiting_head, &base->req_waiting_head, 1);
-	}
 	base->global_requests_inflight = base->global_requests_waiting = 0;
 
 	for (server = base->server_head; server; server = server_next) {