From ac466a22195f8d550a8612bb89583c5e58eadb1a Mon Sep 17 00:00:00 2001 From: Micah Elizabeth Scott Date: Mon, 27 Mar 2023 16:18:26 -0700 Subject: [PATCH] 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 --- src/feature/hs/hs_circuit.c | 11 +++++++++++ src/feature/hs/hs_circuit.h | 1 + src/feature/hs/hs_pow.c | 2 ++ src/test/test_hs_pow.c | 3 ++- src/test/test_hs_pow_slow.c | 1 + 5 files changed, 17 insertions(+), 1 deletion(-) diff --git a/src/feature/hs/hs_circuit.c b/src/feature/hs/hs_circuit.c index 82c77fcfcb..92217d760a 100644 --- 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 da4eb9aa0b..8b0692d76b 100644 --- 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 08981699df..199c290004 100644 --- 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 ca470e2b5d..909a2b569e 100644 --- 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 690ce0cf78..fdade2d3fa 100644 --- 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);