[tor-commits] [tor/master] Fix a crash bug in tor_assert(md->held_by_node)

[nickm at torproject.org]

commit a4b7525c3ce596d4221575806d44652aaa9047a9
Author: Nick Mathewson <nickm at torproject.org>
Date:   Wed Sep 28 13:29:01 2011 -0400

    Fix a crash bug in tor_assert(md->held_by_node)
    
    The fix is to turn held_by_node into a reference count.
    
    Fixes bug 4118; bugfix on 0.2.3.1-alpha.
---
 changes/bug4118    |    5 +++++
 src/or/microdesc.c |    8 ++++----
 src/or/nodelist.c  |   16 ++++++++--------
 src/or/or.h        |    4 ++--
 4 files changed, 19 insertions(+), 14 deletions(-)

diff --git a/changes/bug4118 b/changes/bug4118
new file mode 100644
index 0000000..e56ea70
--- /dev/null
+++ b/changes/bug4118
@@ -0,0 +1,5 @@
+  o Major bugfixes:
+    - Fix a crash bug that could occur when the same microdescriptor was
+      referenced by two node_t objects at once. Fix for bug 4118; bugfix
+      on Tor 0.2.3.1-alpha.
+
diff --git a/src/or/microdesc.c b/src/or/microdesc.c
index 510b2f4..0517e47 100644
--- a/src/or/microdesc.c
+++ b/src/or/microdesc.c
@@ -522,7 +522,7 @@ microdesc_free(microdesc_t *md)
     }
     tor_fragile_assert();
   }
-  if (md->held_by_node) {
+  if (md->held_by_nodes) {
     int found=0;
     const smartlist_t *nodes = nodelist_get_list();
     SMARTLIST_FOREACH(nodes, node_t *, node, {
@@ -533,15 +533,15 @@ microdesc_free(microdesc_t *md)
       });
     if (found) {
       log_warn(LD_BUG, "microdesc_free() called, but md was still referenced "
-               "%d node(s)", found);
+               "%d node(s); held_by_nodes == %u", found, md->held_by_nodes);
     } else {
-      log_warn(LD_BUG, "microdesc_free() called with held_by_node set, but "
+      log_warn(LD_BUG, "microdesc_free() called with held_by_nodes set, but "
                "md was not refrenced by any nodes");
     }
     tor_fragile_assert();
   }
   //tor_assert(md->held_in_map == 0);
-  //tor_assert(md->held_by_node == 0);
+  //tor_assert(md->held_by_nodes == 0);
 
   if (md->onion_pkey)
     crypto_free_pk_env(md->onion_pkey);
diff --git a/src/or/nodelist.c b/src/or/nodelist.c
index 308aaa8..39bc082 100644
--- a/src/or/nodelist.c
+++ b/src/or/nodelist.c
@@ -160,9 +160,9 @@ nodelist_add_microdesc(microdesc_t *md)
   node = node_get_mutable_by_id(rs->identity_digest);
   if (node) {
     if (node->md)
-      node->md->held_by_node = 0;
+      node->md->held_by_nodes--;
     node->md = md;
-    md->held_by_node = 1;
+    md->held_by_nodes++;
   }
   return node;
 }
@@ -189,11 +189,11 @@ nodelist_set_consensus(networkstatus_t *ns)
       if (node->md == NULL ||
           tor_memneq(node->md->digest,rs->descriptor_digest,DIGEST256_LEN)) {
         if (node->md)
-          node->md->held_by_node = 0;
+          node->md->held_by_nodes--;
         node->md = microdesc_cache_lookup_by_digest256(NULL,
                                                        rs->descriptor_digest);
         if (node->md)
-          node->md->held_by_node = 1;
+          node->md->held_by_nodes++;
       }
     }
 
@@ -250,7 +250,7 @@ nodelist_remove_microdesc(const char *identity_digest, microdesc_t *md)
   node_t *node = node_get_mutable_by_id(identity_digest);
   if (node && node->md == md) {
     node->md = NULL;
-    md->held_by_node = 0;
+    md->held_by_nodes--;
   }
 }
 
@@ -299,7 +299,7 @@ node_free(node_t *node)
   if (!node)
     return;
   if (node->md)
-    node->md->held_by_node = 0;
+    node->md->held_by_nodes--;
   tor_assert(node->nodelist_idx == -1);
   tor_free(node);
 }
@@ -319,7 +319,7 @@ nodelist_purge(void)
 
     if (node->md && !node->rs) {
       /* An md is only useful if there is an rs. */
-      node->md->held_by_node = 0;
+      node->md->held_by_nodes--;
       node->md = NULL;
     }
 
@@ -394,7 +394,7 @@ nodelist_assert_ok(void)
           microdesc_cache_lookup_by_digest256(NULL, rs->descriptor_digest);
         tor_assert(md == node->md);
         if (md)
-          tor_assert(md->held_by_node == 1);
+          tor_assert(md->held_by_nodes >= 1);
       }
     } SMARTLIST_FOREACH_END(rs);
   }
diff --git a/src/or/or.h b/src/or/or.h
index b05c767..a1a0790 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -1830,10 +1830,10 @@ typedef struct microdesc_t {
   saved_location_t saved_location : 3;
   /** If true, do not attempt to cache this microdescriptor on disk. */
   unsigned int no_save : 1;
-  /** If true, this microdesc is attached to a node_t. */
-  unsigned int held_by_node : 1;
   /** If true, this microdesc has an entry in the microdesc_map */
   unsigned int held_in_map : 1;
+  /** Reference count: how many node_ts have a reference to this microdesc? */
+  unsigned int held_by_nodes;
 
   /** If saved_location == SAVED_IN_CACHE, this field holds the offset of the
    * microdescriptor in the cache. */