From 114a371c0ea43aae667b71b2ba1e0a7754d08ea5 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 2 Nov 2010 11:20:09 -0400 Subject: [PATCH] Fix the assert in bug 1776 In the case where old_router == NULL but sdmap has an entry for the router, we can currently safely infer that the old_router was not a bridge. Add an assert to ensure that this remains true, and fix the logic not to die with the tor_assert(old_router) call. --- changes/bug1776_redux | 5 +++++ src/or/routerlist.c | 17 ++++++++++++----- 2 files changed, 17 insertions(+), 5 deletions(-) create mode 100644 changes/bug1776_redux diff --git a/changes/bug1776_redux b/changes/bug1776_redux new file mode 100644 index 0000000000..0e2e5cd57c --- /dev/null +++ b/changes/bug1776_redux @@ -0,0 +1,5 @@ + o Major bugfixes + - Avoid an assertion failure when we as an authority receive a + duplicate upload of a router descriptor that we already have, + but which we previously considered an obsolete descriptor. + Fixes another case of bug 1776. Bugfix on 0.2.2.16-alpha. diff --git a/src/or/routerlist.c b/src/or/routerlist.c index 8ed9a7f5e6..94f57cc39f 100644 --- a/src/or/routerlist.c +++ b/src/or/routerlist.c @@ -2661,12 +2661,15 @@ signed_descriptor_free(signed_descriptor_t *sd) tor_free(sd); } -/** Extract a signed_descriptor_t from a routerinfo, and free the routerinfo. +/** Extract a signed_descriptor_t from a general routerinfo, and free the + * routerinfo. */ static signed_descriptor_t * signed_descriptor_from_routerinfo(routerinfo_t *ri) { - signed_descriptor_t *sd = tor_malloc_zero(sizeof(signed_descriptor_t)); + signed_descriptor_t *sd; + tor_assert(ri->purpose == ROUTER_PURPOSE_GENERAL); + sd = tor_malloc_zero(sizeof(signed_descriptor_t)); memcpy(sd, &(ri->cache_info), sizeof(signed_descriptor_t)); sd->routerlist_index = -1; ri->cache_info.signed_descriptor_body = NULL; @@ -3209,10 +3212,14 @@ router_add_to_routerlist(routerinfo_t *router, const char **msg, /* If we have this descriptor already and the new descriptor is a bridge * descriptor, replace it. If we had a bridge descriptor before and the * new one is not a bridge descriptor, don't replace it. */ - tor_assert(old_router); + + /* Only members of routerlist->identity_map can be bridges; we don't + * put bridges in old_routers. */ + const int was_bridge = old_router && + old_router->purpose == ROUTER_PURPOSE_BRIDGE; + if (! (routerinfo_is_a_configured_bridge(router) && - (router->purpose == ROUTER_PURPOSE_BRIDGE || - old_router->purpose != ROUTER_PURPOSE_BRIDGE))) { + (router->purpose == ROUTER_PURPOSE_BRIDGE || !was_bridge))) { log_info(LD_DIR, "Dropping descriptor that we already have for router '%s'", router->nickname);