From 9d0f6eb4e2a0d6d3d4c799d6a1a92bb732b9ced6 Mon Sep 17 00:00:00 2001
From: Nick Mathewson <nickm@torproject.org>
Date: Fri, 22 May 2009 18:32:09 +0000
Subject: [PATCH] Fix a potentially very annoying evdns bug that we found in
 Tor.

Generally speaking, it way better to event_assign() an event when you
allocate it than to assign it before every time you event_add it: if
it is already event_add()ed, the assign will mess it up so that it
doesn't _look_ added, and event_add() will insert a second copy.
Later, event_del() will only delete the second copy.  Eventually, the
event_base will have a dangling pointer to freed memory.  Ouch!

svn:r1308
---
 ChangeLog | 1 +
 evdns.c   | 7 ++++---
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 24737e40..c0f59a26 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,6 @@
 Changes in 1.4.12-stable:
  o Try to contain degree of failure when running on a win32 version so heavily firewalled that we can't fake a socketpair.
+ o Fix an obscure timing-dependent, allocator-dependent crash in the evdns code.
 
 Changes in 1.4.11-stable:
  o Fix a bug when removing a timeout from the heap. [Patch from Marko Kreen]
diff --git a/evdns.c b/evdns.c
index b8c02507..b9f6004e 100644
--- a/evdns.c
+++ b/evdns.c
@@ -469,7 +469,6 @@ nameserver_probe_failed(struct nameserver *const ns) {
 					  global_nameserver_timeouts_length - 1)];
 	ns->failed_times++;
 
-	evtimer_set(&ns->timeout_event, nameserver_prod_callback, ns);
 	if (evtimer_add(&ns->timeout_event, (struct timeval *) timeout) < 0) {
           log(EVDNS_LOG_WARN,
               "Error from libevent when adding timer event for %s",
@@ -498,7 +497,6 @@ nameserver_failed(struct nameserver *const ns, const char *msg) {
 	ns->state = 0;
 	ns->failed_times = 1;
 
-	evtimer_set(&ns->timeout_event, nameserver_prod_callback, ns);
 	if (evtimer_add(&ns->timeout_event, (struct timeval *) &global_nameserver_timeouts[0]) < 0) {
 		log(EVDNS_LOG_WARN,
 		    "Error from libevent when adding timer event for %s",
@@ -1941,7 +1939,6 @@ evdns_request_transmit(struct request *req) {
 		/* all ok */
 		log(EVDNS_LOG_DEBUG,
 		    "Setting timeout for request %lx", (unsigned long) req);
-		evtimer_set(&req->timeout_event, evdns_request_timeout_callback, req);
 		if (evtimer_add(&req->timeout_event, &global_timeout) < 0) {
                   log(EVDNS_LOG_WARN,
 		      "Error from libevent when adding timer for request %lx",
@@ -2102,6 +2099,8 @@ _evdns_nameserver_add_impl(unsigned long int address, int port) {
 
 	memset(ns, 0, sizeof(struct nameserver));
 
+	evtimer_set(&ns->timeout_event, nameserver_prod_callback, ns);
+
 	ns->socket = socket(PF_INET, SOCK_DGRAM, 0);
 	if (ns->socket < 0) { err = 1; goto out1; }
         evutil_make_socket_nonblocking(ns->socket);
@@ -2226,6 +2225,8 @@ request_new(int type, const char *name, int flags,
         if (!req) return NULL;
 	memset(req, 0, sizeof(struct request));
 
+	evtimer_set(&req->timeout_event, evdns_request_timeout_callback, req);
+
 	/* request data lives just after the header */
 	req->request = ((u8 *) req) + sizeof(struct request);
 	/* denotes that the request data shouldn't be free()ed */