From a7e75ff7966d7d3d83d41a47c5b9fa5e3daa2465 Mon Sep 17 00:00:00 2001 From: Roger Dingledine Date: Wed, 24 May 2017 23:14:23 -0400 Subject: [PATCH 1/3] don't free the values in options->MyFamily when we make a descriptor If we free them here, we will still attempt to access the freed memory later on, and also we will double-free when we are freeing the config. Fixes part of bug 22368. --- src/or/router.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/or/router.c b/src/or/router.c index 642f415a38..b43742aa83 100644 --- a/src/or/router.c +++ b/src/or/router.c @@ -2289,7 +2289,7 @@ router_build_fresh_descriptor(routerinfo_t **r, extrainfo_t **e) char *name = family->value; const node_t *member; if (!strcasecmp(name, options->Nickname)) - goto skip; /* Don't list ourself, that's redundant */ + continue; /* Don't list ourself, that's redundant */ else member = node_get_by_nickname(name, 1); if (!member) { @@ -2323,8 +2323,6 @@ router_build_fresh_descriptor(routerinfo_t **r, extrainfo_t **e) if (smartlist_contains_string(warned_nonexistent_family, name)) smartlist_string_remove(warned_nonexistent_family, name); } - skip: - tor_free(name); } /* remove duplicates from the list */ From d22d565331c7f8e02abb7720afd447c6b8760f21 Mon Sep 17 00:00:00 2001 From: Roger Dingledine Date: Wed, 24 May 2017 23:23:12 -0400 Subject: [PATCH 2/3] add copy of MyFamily element to the descriptor, not the element itself If we add the element itself, we will later free it when we free the descriptor, and the next time we go to look at MyFamily, things will go badly. Fixes the rest of bug 22368; bugfix on 0.3.1.1-alpha. --- changes/bug22368 | 6 ++++++ src/or/router.c | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) create mode 100644 changes/bug22368 diff --git a/changes/bug22368 b/changes/bug22368 new file mode 100644 index 0000000000..eb445d0e43 --- /dev/null +++ b/changes/bug22368 @@ -0,0 +1,6 @@ + o Major bugfixes: + - Relays that set MyFamily no longer free the elements of + options->MyFamily while making their descriptor. They tried to + access the freed elements, and then double-free them, when making + the next descriptor or when changing the config. Fixes bug 22368; + bugfix on 0.3.1.1-alpha. diff --git a/src/or/router.c b/src/or/router.c index b43742aa83..f2741b70af 100644 --- a/src/or/router.c +++ b/src/or/router.c @@ -2308,7 +2308,7 @@ router_build_fresh_descriptor(routerinfo_t **r, extrainfo_t **e) smartlist_add_strdup(warned_nonexistent_family, name); } if (is_legal) { - smartlist_add(ri->declared_family, name); + smartlist_add_strdup(ri->declared_family, name); name = NULL; } } else if (router_digest_is_me(member->identity)) { From 5f74749fbabe1122cbcd812c56ab0dc77c358f9d Mon Sep 17 00:00:00 2001 From: Roger Dingledine Date: Wed, 24 May 2017 23:29:30 -0400 Subject: [PATCH 3/3] get rid of some dead code (leftover from commit fa04fe1) --- src/or/router.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/or/router.c b/src/or/router.c index f2741b70af..9b4c2445f4 100644 --- a/src/or/router.c +++ b/src/or/router.c @@ -2309,7 +2309,6 @@ router_build_fresh_descriptor(routerinfo_t **r, extrainfo_t **e) } if (is_legal) { smartlist_add_strdup(ri->declared_family, name); - name = NULL; } } else if (router_digest_is_me(member->identity)) { /* Don't list ourself in our own family; that's redundant */