Backport candidate:

Fix a potential crash bug when we load many server descriptors at
once and some of them make others of them obsolete. Fixes bug
458. [Bugfix on 0.1.2.x]


svn:r10832

3 files changed, 12 insertions, 6 deletions

diff --git a/ChangeLog b/ChangeLog
index ebdbba1774..16046c2d53 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -49,6 +49,9 @@ Changes in version 0.2.0.3-alpha - 2007-??-??
   o Minor bugfixes (directory):
     - Fix another crash bug related to extra-info caching.  (Bug found by
       Peter Palfrader.) [Bugfix on 0.2.0.2-alpha]
+    - Fix a potential crash bug when we load many server descriptors at
+      once and some of them make others of them obsolete. Fixes bug
+      458. [Bugfix on 0.1.2.x]
     - Directories no longer return a "304 not modified" when they don't
       have the networkstatus the client asked for. Also fix a memory
       leak when returning 304 not modified. [Bugfixes on 0.2.0.2-alpha]
diff --git a/src/or/control.c b/src/or/control.c
index 3f929ffbb5..f103982e0f 100644
--- a/src/or/control.c
+++ b/src/or/control.c
@@ -3013,7 +3013,7 @@ control_event_logmsg(int severity, uint32_t domain, const char *msg)
 
 /** Called whenever we receive new router descriptors: tell any
  * interested control connections.  <b>routers</b> is a list of
- * DIGEST_LEN-byte identity digests.
+ * routerinfo_t's.
  */
 int
 control_event_descriptors_changed(smartlist_t *routers)
diff --git a/src/or/routerlist.c b/src/or/routerlist.c
index 113b063b78..c75c024dc4 100644
--- a/src/or/routerlist.c
+++ b/src/or/routerlist.c
@@ -2337,7 +2337,10 @@ router_set_status(const char *digest, int up)
 /** Add <b>router</b> to the routerlist, if we don't already have it.  Replace
  * older entries (if any) with the same key.  Note: Callers should not hold
  * their pointers to <b>router</b> if this function fails; <b>router</b>
- * will either be inserted into the routerlist or freed.
+ * will either be inserted into the routerlist or freed. Similarly, even
+ * if this call succeeds, they should not hold their pointers to
+ * <b>router</b> after subsequent calls with other routerinfo's -- they
+ * might cause the original routerinfo to get freed.
  *
  * Returns >= 0 if the router was added; less than 0 if it was not.
  *
@@ -2863,13 +2866,13 @@ router_load_routers_from_string(const char *s, const char *eos,
     if (purpose != ROUTER_PURPOSE_GENERAL)
       ri->cache_info.do_not_cache = 1;
 
-    if (router_add_to_routerlist(ri, &msg, from_cache, !from_cache) >= 0)
+    if (router_add_to_routerlist(ri, &msg, from_cache, !from_cache) >= 0) {
       smartlist_add(changed, ri);
+      routerlist_descriptors_added(changed);
+      smartlist_clear(changed);
+    }
   });
 
-  if (smartlist_len(changed))
-    routerlist_descriptors_added(changed);
-
   routerlist_assert_ok(routerlist);
   router_rebuild_store(0, 0);