commit 8c29729916be9cd9692657096babb838fc2e7a4c
Author: David Goulet <dgoulet(a)torproject.org>
Date: Tue Apr 20 13:13:54 2021 -0400
hs: Fix memory leak in client cache
Fixes #40356
Signed-off-by: David Goulet <dgoulet(a)torproject.org>
---
changes/ticket40356 | 3 +++
src/feature/hs/hs_cache.c | 61 ++++++++++++++++++++++++++++-------------------
2 files changed, 39 insertions(+), 25 deletions(-)
diff --git a/changes/ticket40356 b/changes/ticket40356
new file mode 100644
index 0000000000..59c32ce0cc
--- /dev/null
+++ b/changes/ticket40356
@@ -0,0 +1,3 @@
+ o Minor bugfix (onion service, client, memory leak):
+ - An expired cached descriptor could have been overwritten with a new one
+ leading to a memory leak. Fixes bug 40356; bugfix on 0.3.5.1-alpha.
diff --git a/src/feature/hs/hs_cache.c b/src/feature/hs/hs_cache.c
index c1334a7d27..9c35936748 100644
--- a/src/feature/hs/hs_cache.c
+++ b/src/feature/hs/hs_cache.c
@@ -353,6 +353,31 @@ static digest256map_t *hs_cache_v3_client;
* objects all related to a specific service. */
static digest256map_t *hs_cache_client_intro_state;
+#define cache_client_desc_free(val) \
+ FREE_AND_NULL(hs_cache_client_descriptor_t, cache_client_desc_free_, (val))
+
+/** Free memory allocated by <b>desc</b>. */
+static void
+cache_client_desc_free_(hs_cache_client_descriptor_t *desc)
+{
+ if (desc == NULL) {
+ return;
+ }
+ hs_descriptor_free(desc->desc);
+ memwipe(&desc->key, 0, sizeof(desc->key));
+ memwipe(desc->encoded_desc, 0, strlen(desc->encoded_desc));
+ tor_free(desc->encoded_desc);
+ tor_free(desc);
+}
+
+/** Helper function: Use by the free all function to clear the client cache */
+static void
+cache_client_desc_free_void(void *ptr)
+{
+ hs_cache_client_descriptor_t *desc = ptr;
+ cache_client_desc_free(desc);
+}
+
/** Return the size of a client cache entry in bytes. */
static size_t
cache_get_client_entry_size(const hs_cache_client_descriptor_t *entry)
@@ -390,7 +415,18 @@ remove_v3_desc_as_client(const hs_cache_client_descriptor_t *desc)
static void
store_v3_desc_as_client(hs_cache_client_descriptor_t *desc)
{
+ hs_cache_client_descriptor_t *cached_desc;
+
tor_assert(desc);
+
+ /* Because the lookup function doesn't return an expired entry, it can linger
+ * in the cache until we clean it up or a new descriptor is stored. So,
+ * before adding, we'll make sure we are not overwriting an old descriptor
+ * (which is OK in terms of semantic) but leads to memory leak. */
+ cached_desc = digest256map_get(hs_cache_v3_client, desc->key.pubkey);
+ if (cached_desc) {
+ cache_client_desc_free(cached_desc);
+ }
digest256map_set(hs_cache_v3_client, desc->key.pubkey, desc);
/* Update cache size with this entry for the OOM handler. */
rend_cache_increment_allocation(cache_get_client_entry_size(desc));
@@ -473,31 +509,6 @@ cache_client_desc_new(const char *desc_str,
return client_desc;
}
-#define cache_client_desc_free(val) \
- FREE_AND_NULL(hs_cache_client_descriptor_t, cache_client_desc_free_, (val))
-
-/** Free memory allocated by <b>desc</b>. */
-static void
-cache_client_desc_free_(hs_cache_client_descriptor_t *desc)
-{
- if (desc == NULL) {
- return;
- }
- hs_descriptor_free(desc->desc);
- memwipe(&desc->key, 0, sizeof(desc->key));
- memwipe(desc->encoded_desc, 0, strlen(desc->encoded_desc));
- tor_free(desc->encoded_desc);
- tor_free(desc);
-}
-
-/** Helper function: Use by the free all function to clear the client cache */
-static void
-cache_client_desc_free_void(void *ptr)
-{
- hs_cache_client_descriptor_t *desc = ptr;
- cache_client_desc_free(desc);
-}
-
/** Return a newly allocated and initialized hs_cache_intro_state_t object. */
static hs_cache_intro_state_t *
cache_intro_state_new(void)