From 11b9c839f0c04c943a1e5f587dafb704e068b1aa Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Sun, 17 May 2009 01:55:02 -0400 Subject: [PATCH 1/3] Stop using malloc_usable_size(): valgrind hates it. --- ChangeLog | 3 +++ src/common/util.c | 4 +++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index b7a4db1404..05d5719465 100644 --- a/ChangeLog +++ b/ChangeLog @@ -18,6 +18,9 @@ Changes in version 0.2.1.15??? - ????-??-?? memory right up to the end of a memarea, then realigned the memory one step beyond the end. Fixes a possible cause of bug 930. + - Stop using malloc_usable_size() to use more area than we had + actually allocated: it was safe, but made valgrind really + unhappy. Bugfix on 0.2.0.x. Changes in version 0.2.1.14-rc - 2009-04-12 diff --git a/src/common/util.c b/src/common/util.c index d153df8cf1..13c55b2888 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -258,7 +258,9 @@ _tor_malloc_roundup(size_t *sizep DMALLOC_PARAMS) #ifdef HAVE_MALLOC_GOOD_SIZE *sizep = malloc_good_size(*sizep); return _tor_malloc(*sizep DMALLOC_FN_ARGS); -#elif defined(HAVE_MALLOC_USABLE_SIZE) && !defined(USE_DMALLOC) +#elif 0 && defined(HAVE_MALLOC_USABLE_SIZE) && !defined(USE_DMALLOC) + /* Never use malloc_usable_size(); it makes valgrind really unhappy, + * and doesn't win much in terms of usable space where it exists. */ void *result = _tor_malloc(*sizep DMALLOC_FN_ARGS); *sizep = malloc_usable_size(result); return result; From c0515b307c4c60d4a17158e47db1033540019b28 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Sun, 17 May 2009 02:01:09 -0400 Subject: [PATCH 2/3] Fix valgrind error when marking a descriptor as never-downloadable. When we got a descriptor that we (as an authority) rejected as totally bad, we were freeing it, then using the digest in its RAM to look up its download status. Caught by arma with valgrind. Bugfix on 0.2.1.9-alpha. --- ChangeLog | 2 ++ src/or/routerlist.c | 7 ++++--- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/ChangeLog b/ChangeLog index 05d5719465..b84da4ceeb 100644 --- a/ChangeLog +++ b/ChangeLog @@ -21,6 +21,8 @@ Changes in version 0.2.1.15??? - ????-??-?? - Stop using malloc_usable_size() to use more area than we had actually allocated: it was safe, but made valgrind really unhappy. Bugfix on 0.2.0.x. + - Fix use of freed memory when deciding to mark a non-addable + descriptor as never-downloadable. Bugfix on 0.2.1.9-alpha. Changes in version 0.2.1.14-rc - 2009-04-12 diff --git a/src/or/routerlist.c b/src/or/routerlist.c index c6bfca4546..de38e354e0 100644 --- a/src/or/routerlist.c +++ b/src/or/routerlist.c @@ -3513,6 +3513,7 @@ router_load_routers_from_string(const char *s, const char *eos, SMARTLIST_FOREACH_BEGIN(routers, routerinfo_t *, ri) { was_router_added_t r; + char d[DIGEST_LEN]; if (requested_fingerprints) { base16_encode(fp, sizeof(fp), descriptor_digests ? ri->cache_info.signed_descriptor_digest : @@ -3533,6 +3534,7 @@ router_load_routers_from_string(const char *s, const char *eos, } } + memcpy(d, ri->cache_info.signed_descriptor_digest, DIGEST_LEN); r = router_add_to_routerlist(ri, &msg, from_cache, !from_cache); if (WRA_WAS_ADDED(r)) { any_changed++; @@ -3541,11 +3543,10 @@ router_load_routers_from_string(const char *s, const char *eos, smartlist_clear(changed); } else if (WRA_WAS_REJECTED(r)) { download_status_t *dl_status; - dl_status = router_get_dl_status_by_descriptor_digest( - ri->cache_info.signed_descriptor_digest); + dl_status = router_get_dl_status_by_descriptor_digest(d); if (dl_status) { log_info(LD_GENERAL, "Marking router %s as never downloadable", - hex_str(ri->cache_info.signed_descriptor_digest, DIGEST_LEN)); + hex_str(d, DIGEST_LEN)); download_status_mark_impossible(dl_status); } } From 2fa54102188b70b9899035250a9f7c26b843fc91 Mon Sep 17 00:00:00 2001 From: Roger Dingledine Date: Sun, 17 May 2009 01:22:29 -0400 Subject: [PATCH 3/3] Fix a memory leak when v3 directory authorities load their keys and cert from disk. Bugfix on 0.2.0.1-alpha. --- ChangeLog | 2 ++ src/or/router.c | 3 --- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/ChangeLog b/ChangeLog index b84da4ceeb..21569d0999 100644 --- a/ChangeLog +++ b/ChangeLog @@ -23,6 +23,8 @@ Changes in version 0.2.1.15??? - ????-??-?? unhappy. Bugfix on 0.2.0.x. - Fix use of freed memory when deciding to mark a non-addable descriptor as never-downloadable. Bugfix on 0.2.1.9-alpha. + - Fix a memory leak when v3 directory authorities load their keys + and cert from disk. Bugfix on 0.2.0.1-alpha. Changes in version 0.2.1.14-rc - 2009-04-12 diff --git a/src/or/router.c b/src/or/router.c index 45ea18b14f..da922b7508 100644 --- a/src/or/router.c +++ b/src/or/router.c @@ -330,9 +330,6 @@ load_authority_keyset(int legacy, crypto_pk_env_t **key_out, "certificate"); goto done; } - parsed->cache_info.signed_descriptor_body = cert; - parsed->cache_info.signed_descriptor_len = eos-cert; - cert = NULL; if (*key_out) crypto_free_pk_env(*key_out);