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 c8512c9eab80366683910ccbd63fccabe04050f1..aac98ff998343b6dff9128e0076cee765b19ab3b 100644 (file)
--- 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 005abc4425a88d254c3394c0387619ba48bc288c..edb934f8f31d6a090c18195bdf9af3e4b456e858 100644 (file)
--- 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;