From: Waldemar Zimpel Date: Sun, 1 Jun 2025 12:43:03 +0000 (+0200) Subject: Fix: "Bug: Duplicate call to circuit_mark_for_close()" X-Git-Tag: tor-0.4.8.17~5^2^2 X-Git-Url: http://git.ipfire.org/gitweb/gitweb.cgi?a=commitdiff_plain;h=159f0c08c5cc9b7b2be502010e37d6266357fbfb;p=thirdparty%2Ftor.git Fix: "Bug: Duplicate call to circuit_mark_for_close()" Closes issue #40951 --- diff --git a/changes/bug40951 b/changes/bug40951 new file mode 100644 index 0000000000..8ef87f342d --- /dev/null +++ b/changes/bug40951 @@ -0,0 +1,4 @@ + o Minor bugfixes (circuit handling): + - Prevent circuit_mark_for_close() from + being called twice on the same circuit. + Fixes bug 40951; bugfix on 0.4.8.16-dev. \ No newline at end of file diff --git a/src/core/or/command.c b/src/core/or/command.c index cad7a173b6..6e85311c86 100644 --- a/src/core/or/command.c +++ b/src/core/or/command.c @@ -476,7 +476,7 @@ command_process_relay_cell(cell_t *cell, channel_t *chan) { const or_options_t *options = get_options(); circuit_t *circ; - int reason, direction; + int direction; uint32_t orig_delivered_bw = 0; uint32_t orig_overhead_bw = 0; @@ -566,7 +566,7 @@ command_process_relay_cell(cell_t *cell, channel_t *chan) } } - if ((reason = circuit_receive_relay_cell(cell, circ, direction)) < 0) { + if (circuit_receive_relay_cell(cell, circ, direction) < 0) { log_fn(LOG_DEBUG,LD_PROTOCOL,"circuit_receive_relay_cell " "(%s) failed. Closing.", direction==CELL_DIRECTION_OUT?"forward":"backward"); @@ -574,7 +574,6 @@ command_process_relay_cell(cell_t *cell, channel_t *chan) if (CIRCUIT_IS_ORIGIN(circ)) { control_event_circ_bandwidth_used_for_circ(TO_ORIGIN_CIRCUIT(circ)); } - circuit_mark_for_close(circ, -reason); } if (CIRCUIT_IS_ORIGIN(circ)) { diff --git a/src/core/or/relay.c b/src/core/or/relay.c index 005a597cf8..a6321f180f 100644 --- a/src/core/or/relay.c +++ b/src/core/or/relay.c @@ -227,7 +227,10 @@ circuit_update_channel_usage(circuit_t *circ, cell_t *cell) * - If not recognized, then we need to relay it: append it to the appropriate * cell_queue on circ. * - * Return -reason on failure. + * If a reason exists to close circ, circuit_mark_for_close() + * is called in this function, so the caller doesn't have to do it. + * + * Return -reason on failure, else 0. */ int circuit_receive_relay_cell(cell_t *cell, circuit_t *circ, @@ -249,7 +252,8 @@ circuit_receive_relay_cell(cell_t *cell, circuit_t *circ, < 0) { log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, "relay crypt failed. Dropping connection."); - return -END_CIRC_REASON_INTERNAL; + reason = -END_CIRC_REASON_INTERNAL; + goto error; } circuit_update_channel_usage(circ, cell); @@ -280,10 +284,9 @@ circuit_receive_relay_cell(cell_t *cell, circuit_t *circ, log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, "connection_edge_process_relay_cell (away from origin) " "failed."); - return reason; + goto error; } - } - if (cell_direction == CELL_DIRECTION_IN) { + } else if (cell_direction == CELL_DIRECTION_IN) { ++stats_n_relay_cells_delivered; log_debug(LD_OR,"Sending to origin."); reason = connection_edge_process_relay_cell(cell, circ, conn, @@ -296,7 +299,7 @@ circuit_receive_relay_cell(cell_t *cell, circuit_t *circ, log_warn(LD_OR, "connection_edge_process_relay_cell (at origin) failed."); } - return reason; + goto error; } } return 0; @@ -319,7 +322,8 @@ circuit_receive_relay_cell(cell_t *cell, circuit_t *circ, * XXX: Shouldn't they always die? */ if (circ->purpose == CIRCUIT_PURPOSE_PATH_BIAS_TESTING) { TO_ORIGIN_CIRCUIT(circ)->path_state = PATH_STATE_USE_FAILED; - return -END_CIRC_REASON_TORPROTOCOL; + reason = -END_CIRC_REASON_TORPROTOCOL; + goto error; } else { return 0; } @@ -339,15 +343,14 @@ circuit_receive_relay_cell(cell_t *cell, circuit_t *circ, CELL_DIRECTION_IN)) < 0) { log_warn(LD_REND, "Error relaying cell across rendezvous; closing " "circuits"); - /* XXXX Do this here, or just return -1? */ - circuit_mark_for_close(circ, -reason); - return reason; + goto error; } return 0; } if (BUG(CIRCUIT_IS_ORIGIN(circ))) { /* Should be impossible at this point. */ - return -END_CIRC_REASON_TORPROTOCOL; + reason = -END_CIRC_REASON_TORPROTOCOL; + goto error; } or_circuit_t *or_circ = TO_OR_CIRCUIT(circ); if (++or_circ->n_cells_discarded_at_end == 1) { @@ -356,7 +359,8 @@ circuit_receive_relay_cell(cell_t *cell, circuit_t *circ, "Didn't recognize a cell, but circ stops here! Closing circuit. " "It was created %ld seconds ago.", (long)seconds_open); } - return -END_CIRC_REASON_TORPROTOCOL; + reason = -END_CIRC_REASON_TORPROTOCOL; + goto error; } log_debug(LD_OR,"Passing on unrecognized cell."); @@ -366,9 +370,14 @@ circuit_receive_relay_cell(cell_t *cell, circuit_t *circ, * the cells. */ if (append_cell_to_circuit_queue(circ, chan, cell, cell_direction, 0) < 0) { - return -END_CIRC_REASON_RESOURCELIMIT; + reason = -END_CIRC_REASON_RESOURCELIMIT; + goto error; } return 0; + +error: + circuit_mark_for_close(circ, -reason); + return reason; } /** Package a relay cell from an edge: