[or-cvs] r18306: {tor} Fix a race condition on nameserver reconfiguration. This res (in tor/trunk: . src/or)

[nickm at seul.org]

Author: nickm
Date: 2009-01-28 13:26:20 -0500 (Wed, 28 Jan 2009)
New Revision: 18306

Modified:
   tor/trunk/ChangeLog
   tor/trunk/src/or/eventdns.c
Log:
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.

Modified: tor/trunk/ChangeLog
===================================================================
--- tor/trunk/ChangeLog	2009-01-28 18:21:02 UTC (rev 18305)
+++ tor/trunk/ChangeLog	2009-01-28 18:26:20 UTC (rev 18306)
@@ -20,6 +20,9 @@
       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,

Modified: tor/trunk/src/or/eventdns.c
===================================================================
--- tor/trunk/src/or/eventdns.c	2009-01-28 18:21:02 UTC (rev 18305)
+++ tor/trunk/src/or/eventdns.c	2009-01-28 18:26:20 UTC (rev 18306)
@@ -2083,28 +2083,48 @@
 
 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;