commit 1cf0377057a97d6d944dee153c2ca0dc1c7897de
parent 5dbb8ea475061fd15bfd76056fd354178f965381
Author: David Goulet <dgoulet@torproject.org>
Date: Mon, 17 Nov 2025 12:35:56 -0500
Merge branch 'maint-0.4.8'
Diffstat:
9 files changed, 64 insertions(+), 27 deletions(-)
diff --git a/changes/ticket41162 b/changes/ticket41162
@@ -0,0 +1,5 @@
+ o Major bugfixes (conflux, exit):
+ - When dequeuing out-of-order conflux cells, the circuit could be close in
+ between two dequeue which could lead to a mishandling of a NULL pointer.
+ Fixes bug 41162; bugfix on 0.4.8.4.
+
diff --git a/src/core/mainloop/connection.c b/src/core/mainloop/connection.c
@@ -971,7 +971,7 @@ connection_free_,(connection_t *conn))
return;
tor_assert(!connection_is_on_closeable_list(conn));
tor_assert(!connection_in_array(conn));
- if (BUG(conn->linked_conn)) {
+ if (conn->linked_conn) {
conn->linked_conn->linked_conn = NULL;
if (! conn->linked_conn->marked_for_close &&
conn->linked_conn->reading_from_linked_conn)
diff --git a/src/core/or/circuitlist.c b/src/core/or/circuitlist.c
@@ -2722,8 +2722,12 @@ circuits_handle_oom(size_t current_allocation)
* outbuf due to a malicious destination holding off the read on us. */
if ((conn->type == CONN_TYPE_DIR && conn->linked_conn == NULL) ||
CONN_IS_EDGE(conn)) {
- if (!conn->marked_for_close)
+ if (!conn->marked_for_close) {
+ if (CONN_IS_EDGE(conn)) {
+ TO_EDGE_CONN(conn)->end_reason = END_STREAM_REASON_RESOURCELIMIT;
+ }
connection_mark_for_close(conn);
+ }
mem_recovered += single_conn_free_bytes(conn);
if (conn->type == CONN_TYPE_DIR) {
diff --git a/src/core/or/conflux.c b/src/core/or/conflux.c
@@ -891,9 +891,11 @@ conflux_process_relay_msg(conflux_t *cfx, circuit_t *in_circ,
* now be checked for remaining elements */
cfx->last_seq_delivered++;
return true;
- } else if (BUG(leg->last_seq_recv <= cfx->last_seq_delivered)) {
- log_warn(LD_BUG, "Got a conflux cell with a sequence number "
- "less than the last delivered. Closing circuit.");
+ } else if (leg->last_seq_recv <= cfx->last_seq_delivered) {
+ /* Anyone can mangle these sequence number. */
+ log_fn(LOG_PROTOCOL_WARN, LD_BUG,
+ "Got a conflux cell with a sequence number "
+ "less than the last delivered. Closing circuit.");
circuit_mark_for_close(in_circ, END_CIRC_REASON_INTERNAL);
return false;
} else {
@@ -939,9 +941,37 @@ conflux_process_relay_msg(conflux_t *cfx, circuit_t *in_circ,
* or has a hole.
*/
conflux_msg_t *
-conflux_dequeue_relay_msg(conflux_t *cfx)
+conflux_dequeue_relay_msg(circuit_t *circ)
{
conflux_msg_t *top = NULL;
+ /* Related to #41162. This is really a consequence of the C-tor maze.
+ * The function above can close a circuit without returning an error
+ * due to several return code ignored. Auditting all of the cell code
+ * path and fixing them to not ignore errors could bring many more
+ * issues as this behavior has been in tor forever. So do the bandaid
+ * fix of bailing if the circuit is closed. */
+ if (circ->marked_for_close) {
+ static ratelim_t rlim = RATELIM_INIT(60 * 60);
+ log_fn_ratelim(&rlim, (circ->conflux == NULL) ? LOG_WARN : LOG_NOTICE,
+ LD_CIRC,
+ "Circuit was closed at %s:%u when dequeuing from OOO",
+ circ->marked_for_close_file, circ->marked_for_close);
+ return NULL;
+ }
+ conflux_t *cfx = circ->conflux;
+ if (cfx == NULL) {
+ static ratelim_t rlim = RATELIM_INIT(60 * 60);
+ log_fn_ratelim(&rlim, LOG_WARN, LD_CIRC,
+ "Bug: Non marked for close circuit with NULL conflux");
+ return NULL;
+ }
+ if (cfx->ooo_q == NULL) {
+ static ratelim_t rlim = RATELIM_INIT(60 * 60);
+ log_fn_ratelim(&rlim, LOG_WARN, LD_CIRC,
+ "Bug: Non marked for close circuit with NULL OOO queue");
+ return NULL;
+ }
+
if (smartlist_len(cfx->ooo_q) == 0)
return NULL;
diff --git a/src/core/or/conflux.h b/src/core/or/conflux.h
@@ -60,7 +60,7 @@ bool conflux_should_multiplex(int relay_command);
bool conflux_process_relay_msg(conflux_t *cfx, circuit_t *in_circ,
crypt_path_t *layer_hint,
const relay_msg_t *msg);
-conflux_msg_t *conflux_dequeue_relay_msg(conflux_t *cfx);
+conflux_msg_t *conflux_dequeue_relay_msg(circuit_t *circ);
void conflux_note_cell_sent(conflux_t *cfx, circuit_t *circ,
uint8_t relay_command);
void conflux_relay_msg_free_(conflux_msg_t *msg);
diff --git a/src/core/or/conflux_pool.c b/src/core/or/conflux_pool.c
@@ -2127,7 +2127,10 @@ conflux_pool_init(void)
void
conflux_log_set(int loglevel, const conflux_t *cfx, bool is_client)
{
- tor_assert(cfx);
+ /* This could be called on a closed circuit. */
+ if (cfx == NULL) {
+ return;
+ }
log_fn(loglevel,
LD_BUG,
diff --git a/src/core/or/connection_edge.c b/src/core/or/connection_edge.c
@@ -1080,7 +1080,9 @@ static mainloop_event_t *attach_pending_entry_connections_ev = NULL;
static void
connection_edge_about_to_close(edge_connection_t *edge_conn)
{
- if (!edge_conn->edge_has_sent_end) {
+ /* Under memory pressure, the OOM handler can close connections without
+ * sending END cell. If we are NOT in that scenario, log loudly. */
+ if (!edge_conn->edge_has_sent_end && !have_been_under_memory_pressure()) {
connection_t *conn = TO_CONN(edge_conn);
log_warn(LD_BUG, "(Harmless.) Edge connection (marked at %s:%d) "
"hasn't sent end yet?",
diff --git a/src/core/or/relay.c b/src/core/or/relay.c
@@ -589,17 +589,13 @@ relay_send_command_from_edge_,(streamid_t stream_id, circuit_t *orig_circ,
circ = conflux_decide_circ_for_send(orig_circ->conflux, orig_circ,
relay_command);
if (!circ) {
- log_warn(LD_BUG, "No circuit to send for conflux for relay command %d, "
- "called from %s:%d", relay_command, filename, lineno);
- conflux_log_set(LOG_WARN, orig_circ->conflux,
- CIRCUIT_IS_ORIGIN(orig_circ));
- circ = orig_circ;
- } else {
- /* Conflux circuits always send multiplexed relay commands to
- * to the last hop. (Non-multiplexed commands go on their
- * original circuit and hop). */
- cpath_layer = conflux_get_destination_hop(circ);
+ /* Something is wrong with the conflux set. We are done. */
+ return -1;
}
+ /* Conflux circuits always send multiplexed relay commands to
+ * to the last hop. (Non-multiplexed commands go on their
+ * original circuit and hop). */
+ cpath_layer = conflux_get_destination_hop(circ);
}
/* This is possible because we have protocol error paths when deciding the
@@ -2118,7 +2114,7 @@ connection_edge_process_relay_cell(const relay_msg_t *msg, circuit_t *circ,
}
/* Now, check queue for more */
- while ((c_msg = conflux_dequeue_relay_msg(circ->conflux))) {
+ while ((c_msg = conflux_dequeue_relay_msg(circ))) {
conn = relay_lookup_conn(circ, c_msg->msg, CELL_DIRECTION_OUT,
layer_hint);
ret = connection_edge_process_ordered_relay_cell(c_msg->msg, circ,
@@ -2930,14 +2926,11 @@ cell_queues_check_size(void)
/** Return true if we've been under memory pressure in the last
* MEMORY_PRESSURE_INTERVAL seconds. */
-int
+bool
have_been_under_memory_pressure(void)
{
- if (last_time_under_memory_pressure == 0) {
- return false;
- }
- return last_time_under_memory_pressure + MEMORY_PRESSURE_INTERVAL
- < approx_time();
+ return approx_time() <
+ last_time_under_memory_pressure + MEMORY_PRESSURE_INTERVAL;
}
/**
diff --git a/src/core/or/relay.h b/src/core/or/relay.h
@@ -67,7 +67,7 @@ extern uint64_t oom_stats_n_bytes_removed_hsdir;
void dump_cell_pool_usage(int severity);
size_t packed_cell_mem_cost(void);
-int have_been_under_memory_pressure(void);
+bool have_been_under_memory_pressure(void);
/* For channeltls.c */
void packed_cell_free_(packed_cell_t *cell);
