commit a4b7525c3ce596d4221575806d44652aaa9047a9 Author: Nick Mathewson nickm@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. */