hs_pow: leak fix, free the contents of pqueue entries in hs_pow_free_service_state

Asan catches this pretty readily when ending a service gracefully while
a DoS is in progress and the queue is full of items that haven't yet
timed out.

The module boundaries in hs_circuit are quite fuzzy here, but I'm trying
to follow the vibe of the existing hs_pow code.

Signed-off-by: Micah Elizabeth Scott <beth@torproject.org>

diff --git a/src/feature/hs/hs_circuit.c b/src/feature/hs/hs_circuit.c
index 82c77fcfcb6fcad2d81f8bff785287a0b1ba94c1..92217d760a8207d0b54f452440766a0381867c39 100644 (file)
--- a/src/feature/hs/hs_circuit.c
+++ b/src/feature/hs/hs_circuit.c
@@ -741,6 +741,17 @@ top_of_rend_pqueue_is_worthwhile(hs_pow_service_state_t *pow_state)
   return 0;
 }
 
+/** Abandon and free all pending rend requests, leaving the pqueue empty. */
+void
+rend_pqueue_clear(hs_pow_service_state_t *pow_state)
+{
+  tor_assert(pow_state->rend_request_pqueue);
+  while (smartlist_len(pow_state->rend_request_pqueue)) {
+    pending_rend_t *req = smartlist_pop_last(pow_state->rend_request_pqueue);
+    free_pending_rend(req);
+  }
+}
+
 /** How many rendezvous request we handle per mainloop event. Per prop327,
  * handling an INTRODUCE2 cell takes on average 5.56msec on an average CPU and
  * so it means that launching this max amount of circuits is well below 0.08

diff --git a/src/feature/hs/hs_circuit.h b/src/feature/hs/hs_circuit.h
index da4eb9aa0b8529e1abb133f226977408a100c8e1..8b0692d76b395a3137dbac5521e18a2bb19959c4 100644 (file)
--- a/src/feature/hs/hs_circuit.h
+++ b/src/feature/hs/hs_circuit.h
@@ -33,6 +33,7 @@ typedef struct pending_rend_t {
 } pending_rend_t;
 
 int top_of_rend_pqueue_is_worthwhile(hs_pow_service_state_t *pow_state);
+void rend_pqueue_clear(hs_pow_service_state_t *pow_state);
 
 /* Cleanup function when the circuit is closed or freed. */
 void hs_circ_cleanup_on_close(circuit_t *circ);

diff --git a/src/feature/hs/hs_pow.c b/src/feature/hs/hs_pow.c
index 08981699df01d82d782ae9a92500ec785b11999d..199c290004cd60aaea66cd63a4ccaa0a933749b5 100644 (file)
--- a/src/feature/hs/hs_pow.c
+++ b/src/feature/hs/hs_pow.c
@@ -337,6 +337,8 @@ hs_pow_free_service_state(hs_pow_service_state_t *state)
   if (state == NULL) {
     return;
   }
+  rend_pqueue_clear(state);
+  tor_assert(smartlist_len(state->rend_request_pqueue) == 0);
   smartlist_free(state->rend_request_pqueue);
   mainloop_event_free(state->pop_pqueue_ev);
   tor_free(state);

diff --git a/src/test/test_hs_pow.c b/src/test/test_hs_pow.c
index ca470e2b5da8bac916b9badcca3644d025af95f7..909a2b569ef399ed454da186f3e3a700713c043c 100644 (file)
--- a/src/test/test_hs_pow.c
+++ b/src/test/test_hs_pow.c
@@ -194,7 +194,7 @@ testing_hs_pow_service_free(testing_hs_pow_service_t *tsvc)
   hs_metrics_service_free(&tsvc->service);
   service_intro_point_free(tsvc->service_ip);
   hs_desc_intro_point_free(tsvc->desc_ip);
-  tor_free(tsvc->service.state.pow_state);
+  hs_pow_free_service_state(tsvc->service.state.pow_state);
   tor_free(tsvc);
 
   if (fake_node) {
@@ -343,6 +343,7 @@ test_hs_pow_vectors(void *arg)
   testing_hs_pow_service_t *tsvc = testing_hs_pow_service_new();
   hs_pow_service_state_t *pow_state = tor_malloc_zero(sizeof *pow_state);
   tsvc->service.state.pow_state = pow_state;
+  pow_state->rend_request_pqueue = smartlist_new();
 
   char *mem_op_hex_tmp = NULL;
   uint8_t *decrypted = NULL;

diff --git a/src/test/test_hs_pow_slow.c b/src/test/test_hs_pow_slow.c
index 690ce0cf7849dcba86229d8870bd38c2d6c7e72b..fdade2d3fabdb3ffcadd89a01485fa4f82931619 100644 (file)
--- a/src/test/test_hs_pow_slow.c
+++ b/src/test/test_hs_pow_slow.c
@@ -26,6 +26,7 @@ testing_one_hs_pow_solution(const hs_pow_solution_t *ref_solution,
   int retval = -1;
   hs_pow_solution_t sol_buffer;
   hs_pow_service_state_t *s = tor_malloc_zero(sizeof(hs_pow_service_state_t));
+  s->rend_request_pqueue = smartlist_new();
 
   memcpy(s->seed_previous, seed, HS_POW_SEED_LEN);