From bbe90e3ec1b308e32f83e9f30f3706645e2b8e36 Mon Sep 17 00:00:00 2001 From: David Goulet <dgoulet@torproject.org> Date: Mon, 13 Jan 2020 09:25:26 -0500 Subject: [PATCH] hs-v2: Always check rend_cache validity before using it When looking up an entry in the rend_cache, stop asserting that it exists but rather confirm it exists and if not, return that no entry was found. The reason for that is because the hs_circ_cleanup_on_free() function (which can end up looking at the rend_cache) can be called from the circuit_free_all() function that is called _after_ the rend cache is cleaned up in tor_free_all(). We could fix the free all ordering but then it will just hide a future bug. Instead, handle a missing rend_cache as a valid use case as in while we are in the cleanup process. As Tor becomes more modular, it is getting more and more difficult to ensure subsystem callstack ordering thus this fix aims at making the HSv2 subsystem more robust at being called while tor is pretty much in any kind of state. Fixes #32847. Signed-off-by: David Goulet <dgoulet@torproject.org> --- src/feature/rend/rendcache.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/feature/rend/rendcache.c b/src/feature/rend/rendcache.c index 04748edbd5..0890a81d8f 100644 --- a/src/feature/rend/rendcache.c +++ b/src/feature/rend/rendcache.c @@ -526,9 +526,16 @@ rend_cache_lookup_entry(const char *query, int version, rend_cache_entry_t **e) rend_cache_entry_t *entry = NULL; static const int default_version = 2; - tor_assert(rend_cache); tor_assert(query); + /* This is possible if we are in the shutdown process and the cache was + * freed while some other subsystem might do a lookup to the cache for + * cleanup reasons such HS circuit cleanup for instance. */ + if (!rend_cache) { + ret = -ENOENT; + goto end; + } + if (!rend_valid_v2_service_id(query)) { ret = -EINVAL; goto end; -- GitLab