From: Nick Mathewson <nickm@torproject.org>
Date: Wed, 28 Jan 2009 18:26:20 +0000 (+0000)
Subject: Fix a race condition on nameserver reconfiguration.
X-Git-Tag: tor-0.2.1.12-alpha~29
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=f78793879d0f8cda54303f40337a7f439423b54a;p=thirdparty%2Ftor.git

Fix a race condition on nameserver reconfiguration.

This resolves bug 526, wherein we would crash if the following
events occurred in this order:
  A: We're an OR, and one of our nameservers goes down.
  B: We launch a probe to it to see if it's up again. (We do this hourly
     in steady-state.)
  C: Before the probe finishes, we reconfigure our nameservers,
     usually because we got a SIGHUP and the resolve.conf file changed.
  D: The probe reply comes back, or times out. (There is a five-second
     window for this, after B has happens).

IOW, if one of our nameservers is down and our nameserver
configuration has changed, there were 5 seconds per hour where HUPing
the server was unsafe.

Bugfix on 0.1.2.1-alpha.  Too obscure to backport.

svn:r18306
---

diff --git a/ChangeLog b/ChangeLog
index c8512c9eab..aac98ff998 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -20,6 +20,9 @@ Changes in version 0.2.1.12-alpha - 2009-02-??
       of them created a new connection with just the wrong timing, the other
       might decide to deprecate the new connection erroneously.  Bugfix on
       0.1.1.13-alpha.
+    - Resolve a very rare crash bug that could occur when the user forced
+      a nameserver reconfiguration during the middle of a nameserver
+      probe.  Fixes bug 526.  Bugfix on 0.1.2.1-alpha.
 
   o Minor features:
     - Support platforms where time_t is 64 bits long. (Congratulations,
diff --git a/src/or/eventdns.c b/src/or/eventdns.c
index 005abc4425..edb934f8f3 100644
--- a/src/or/eventdns.c
+++ b/src/or/eventdns.c
@@ -2083,28 +2083,48 @@ evdns_request_transmit(struct request *req) {
 
 static void
 nameserver_probe_callback(int result, char type, int count, int ttl, void *addresses, void *arg) {
-	struct nameserver *const ns = (struct nameserver *) arg;
+	struct sockaddr *addr = arg;
+	struct nameserver *server;
 	(void) type;
 	(void) count;
 	(void) ttl;
 	(void) addresses;
 
-	if (result == DNS_ERR_NONE || result == DNS_ERR_NOTEXIST) {
-		/* this is a good reply */
-		nameserver_up(ns);
-	} else nameserver_probe_failed(ns);
+	for (server = server_head; server; server = server->next) {
+		if (sockaddr_eq(addr, (struct sockaddr*) &server->address, 1)) {
+			if (result == DNS_ERR_NONE || result == DNS_ERR_NOTEXIST) {
+				/* this is a good reply */
+				nameserver_up(server);
+			} else {
+				nameserver_probe_failed(server);
+			}
+		}
+		if (server->next == server_head)
+			break;
+	}
+
+	free(addr);
 }
 
 static void
 nameserver_send_probe(struct nameserver *const ns) {
 	struct request *req;
+	struct sockaddr_storage *addr;
 	/* here we need to send a probe to a given nameserver */
 	/* in the hope that it is up now. */
 
+	/* We identify the nameserver by its address, in case it is removed before
+	 * our probe comes back. */
+	addr = malloc(sizeof(struct sockaddr_storage));
+	memcpy(addr, &ns->address, sizeof(struct sockaddr_storage));
+
 	log(EVDNS_LOG_DEBUG, "Sending probe to %s", debug_ntop((struct sockaddr *)&ns->address));
 
-	req = request_new(TYPE_A, "www.google.com", DNS_QUERY_NO_SEARCH, nameserver_probe_callback, ns);
-	if (!req) return;
+	req = request_new(TYPE_A, "www.google.com", DNS_QUERY_NO_SEARCH, nameserver_probe_callback, addr);
+	if (!req) {
+		free(addr);
+		return;
+	}
 	/* we force this into the inflight queue no matter what */
 	request_trans_id_set(req, transaction_id_pick());
 	req->ns = ns;